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

Fix keyboard event handling for stacked modal dialogs#3785

Merged
njx merged 3 commits into
masterfrom
iwehrman/multiple-modal-dialogs
May 14, 2013
Merged

Fix keyboard event handling for stacked modal dialogs#3785
njx merged 3 commits into
masterfrom
iwehrman/multiple-modal-dialogs

Conversation

@iwehrman
Copy link
Copy Markdown
Contributor

This pull attempts to fix keyboard event handling in case multiple modal dialogs are shown at once by maintaining a stack of key-down handler functions that corresponds to the stack of dialogs, and enabling only the handler for the top dialog. Escape-key dismissal is also now handled explicitly in the key-down handler.

Lightly tested with the Extension Manager -> Install Extension dialog stack .

This pull addresses issue #3505 and supersedes pull #3522.

Ian Wehrman added 2 commits May 10, 2013 13:06
… keydownHandlers, only the top-most of which are enabled.
@ghost ghost assigned njx May 13, 2013
Comment thread src/widgets/Dialogs.js Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think we need to do this (and the call to KeyBindingManager.setEnabled(false) below) only if we're at the outermost level of the stack.

@njx
Copy link
Copy Markdown

njx commented May 14, 2013

Done with review. Overall, CURSE YOU FOR COMING UP WITH A MORE ELEGANT FIX THAN MINE. I'll get the hang of closures one of these days...

… the "stack", even if it isn't on top, and also only fiddling with the KeyBindingManager when there is a single open dialog.
@iwehrman
Copy link
Copy Markdown
Contributor Author

Comments addressed?

  1. restores keyboard: false comment;
  2. removes the current dialog's handler from the stack even if it isn't on top; and
  3. fiddle with the KeyBindingManager only when there is a single open dialog.

@njx
Copy link
Copy Markdown

njx commented May 14, 2013

Great! Merging.

njx pushed a commit that referenced this pull request May 14, 2013
Fix keyboard event handling for stacked modal dialogs
@njx njx merged commit a3ffd16 into master May 14, 2013
@njx njx deleted the iwehrman/multiple-modal-dialogs branch May 14, 2013 23:36
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.

2 participants