Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Keyboard capture of Space and ESC is super aggressive #585

Closed
feross opened this issue May 25, 2016 · 7 comments
Closed

Keyboard capture of Space and ESC is super aggressive #585

feross opened this issue May 25, 2016 · 7 comments
Labels
Milestone

Comments

@feross
Copy link
Member

feross commented May 25, 2016

gWhat version of WebTorrent Desktop? (See the 'About WebTorrent' menu)

0.6.0

What operating system and version?

OS X 10.11.5

What did you do?

Try to use of an OS shortcut with ESC or Space while WebTorrent Desktop is running.

What did you expect to happen?

OS shortcuts should work.

What actually happened?

Any system-wide OS shortcut that makes use of ESC or Space does not work if WebTorrent Desktop is the active window, because it is captured by the app and the electron-localshortcut package.

@feross feross added the bug label May 25, 2016
@dcposch
Copy link
Contributor

dcposch commented May 26, 2016

I assume the fix is to capture those keys only when we're in the player view and it's the active window

@dcposch dcposch self-assigned this May 26, 2016
@feross
Copy link
Member Author

feross commented May 26, 2016

Nah, that's not the solution. The solution is to get make ESC and Space into menu item accelerators (shortcuts).

For Play, let's get rid of CMD+P and just have Space.

For ESC let's just add an item called "Go Back" to the View menu.

@dcposch
Copy link
Contributor

dcposch commented May 26, 2016

Sure, but the Play one should still be disabled unless you're in the player. The Go Back will be enabled/disabled the same as the back button

@feross
Copy link
Member Author

feross commented May 26, 2016

The Play/Pause menu item that's already there already gets enabled/disabled as the user enters/leaves the player.

For the back button, up to you. But I think it's fine to just leave it always enabled and it can just no-op. Otherwise we need to add events to let the main process know about the state of LocationHistory which IMO is more trouble than it's worth.

@dcposch
Copy link
Contributor

dcposch commented May 26, 2016

While I'm at it, do you want to get rid of this as well?

localShortcut.register('CmdOrCtrl+Shift+F', menu.toggleFullScreen)

@feross
Copy link
Member Author

feross commented May 26, 2016

@dcposch Sure. It was a shortcut I was familiar with from Chrome, but I just checked and it does something different now.

I say we remove it. The standard OS fullscreen shortcut should be sufficient! Now we can axe that dependency!

@dcposch
Copy link
Contributor

dcposch commented May 26, 2016

Done

dcposch added a commit that referenced this issue May 26, 2016
@feross feross modified the milestone: v0.6.1 May 26, 2016
@lock lock bot locked as resolved and limited conversation to collaborators May 10, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants