Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

[HACK] [WHAT] [NO] Fix Mac command-key shortcuts in modal dialogs #4315

Closed
wants to merge 3 commits into from

Conversation

iwehrman
Copy link
Contributor

This terrible, horrible no-good pull request fixes Mac command-key shortcuts (like cmd-D to dismiss the save dialog) in modal dialogs. The problem arises from the operating system capturing keyboard events that correspond to shortcuts for menu commands. The shell handles these keyboard events and translates them into command names, which are translated into commands and executed at the JavaScript level. This hack intercepts these command names and, when a modal dialog is open, forwards them as shellCommand events which are handled by open modal dialogs. The shellCommand events are then translated into their corresponding key bindings (e.g., edit.duplicate --> Cmd-D), which are used to manufacture a corresponding keydown event, and which are immediately handed off to the modal dialog's keydown handler.

(You may wonder why the command name forwarded from the shell isn't immediately translated into a keyboard event. To do so requires importing KeyBindingManager into ShellAPI, which seems to cause a circular dependency that makes Brackets sad.)

So, yes, this is very ugly, but, hey, YOU CAN DISMISS SAVE DIALOGS WITH THE KEYBOARD!

Ian Wehrman added 3 commits June 20, 2013 21:19
…og is present, which is later handled by the modal dialog and re-interpreted as a keyboard event
@iwehrman
Copy link
Contributor Author

thereifixedit

@iwehrman
Copy link
Contributor Author

See also #418.

@ghost ghost assigned RaymondLim Jun 24, 2013
@RaymondLim
Copy link
Contributor

@iwehrman
Your changes work and we can now use Cmd-D and Cmd-. on Mac. But as you have said this hack is quite ugly and we may need to do a thorough clean up later on. So instead of landing this, I did implement the JS dialog modality with these two pull requests -- adobe/brackets-shell#268 and #4341

@iwehrman
Copy link
Contributor Author

iwehrman commented Jul 8, 2013

Closing this out in favor of of pull #4341 from @RaymondLim, which is, in contrast, not insane.

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.

None yet

2 participants