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

Detect menu items invoked via keybinding and dispatch keybinding on renderer #64228

Merged
merged 6 commits into from
Dec 19, 2018

Conversation

alexdima
Copy link
Member

@alexdima alexdima commented Dec 3, 2018

Fixes #35225

This PR uses the observation from #35225 (comment) , that Electron gives an event when a menu item is invoked which contains the information about modifier key status. Through a heuristic (that the modifiers must match) we execute, only on mac, a separate code path where the keybinding is sent to the renderer and it is there interpreted using the keybinding service.

@bpasero bpasero added this to the December 2018 milestone Dec 3, 2018
@bpasero bpasero removed their assignment Dec 10, 2018
@bpasero
Copy link
Member

bpasero commented Dec 10, 2018

I resolved the merge conflict but let @sbatten handle this PR for inclusion as the owner of menus.

@sbatten
Copy link
Member

sbatten commented Dec 11, 2018

I think this will work well for the resolution of keybindings when not using the menu, but the menu itself will still show the keybinding it resolves against the global context and the enablement of that item will also be resolved against the global context.

@alexdima
Copy link
Member Author

Thanks @sbatten for the review. I've addressed your comments.


I think this will work well for the resolution of keybindings when not using the menu, but the menu itself will still show the keybinding it resolves against the global context and the enablement of that item will also be resolved against the global context.

Sure. I suggest we create another issue to track that. The original issue #35225 complains about the key bindings and not about the menu displaying the keybindings. I think they are different problems with different severity and I think they should be tackled independently.

@alexdima
Copy link
Member Author

@sbatten Is that a 👍 to merge ?

@sbatten
Copy link
Member

sbatten commented Dec 19, 2018

@alexandrudima yes, this can be merged

@jesaerys
Copy link

@alexdima I'm seeing a regression on this, but #35225 is locked and I can't comment. Should I open a new issue?

@alexdima
Copy link
Member Author

@jesaerys Yes, please open a new issue.

@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

'When' clauses are ignored for all commands in global menu
4 participants