Skip to content

Conversation

@thomastoledo
Copy link
Collaborator

No description provided.

@thomastoledo thomastoledo requested a review from ImFlog November 14, 2018 15:32
html/popup.html Outdated
</div>
<div>
<i>Tips: You can hide the player with Alt+Maj+T</i>
<i>Tips: You can hide the player with Ctrl+Shift+Z</i>
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you checked if it's not bind to anything already ?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also I kinda liked the usage of T like toogle.
If we change, please also change the readme :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I already checked, and I changed the readme as well. I had to change because the "Alt" key didn't seem to be triggered at all. But I'm open for any change.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CTRL+Shift+Z looks too much like the redo command. Are you sure it's not working with the old binds ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. Totaly sure the old binding does not work. I suspect the problem tu be the "Alt" key. How about "Ctrl+Shift+Space" ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ImFlog Have you had time to take a look at it?

</div>
<div>
<i>Tips: You can hide the player with Alt+Maj+T</i>
<i>Tips: You can hide the player with Ctrl+Shift+Space</i>
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From https://developer.chrome.com/apps/commands : Please note that on Mac 'Ctrl' is automatically converted to 'Command'. If you want 'Ctrl' instead, please specify 'MacCtrl' under "mac". Specifying 'MacCtrl' under "default" will cause the extension to be uninstallable.

We can't use this as CMD + Space triggers the spotlight search on macOs :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine, what do we use, then?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Please note that on Mac 'Ctrl' is automatically converted to 'Command'. If you want 'Ctrl' instead, please specify 'MacCtrl' under "mac". Specifying 'MacCtrl' under "default" will cause the extension to be uninstallable.

We could use ctrl + Shift + space for default and MaCtrl + Shift + Space for mac ?
What bothers me is that there is no easy way to remember this shortcut :(

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ImFlog you're right, let's discuss about it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants