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

Fix #283 #419

Merged
merged 7 commits into from
Mar 9, 2012
Merged

Fix #283 #419

merged 7 commits into from
Mar 9, 2012

Conversation

jasonsanjose
Copy link
Member

Fix for #283.

  • Refactor modal dialogs to Dialogs module.
  • Add keyboard shortcuts for Don't Save (CMD+D on mac, N on win) and Cancel (ESC) buttons.
  • Add focus to primary button when shown
  • Add spacebar to click on focused button (only works for primary button)

Does not address #414 for tabbing between dialog controls.

Refactor modal dialogs to Dialogs module.
Add keyboard shortcuts for Don't Save and Cancel buttons.
Add ESC key support.
Remove keyup+keydown handling. Use keydown only to match native behavior.
@gruehle
Copy link
Member

gruehle commented Mar 8, 2012

Reviewing

/*global define: false, $: false, brackets: false */

/**
* Utilities for working with Deferred, Promise, and other asynchronous processes.
Copy link
Member

Choose a reason for hiding this comment

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

Comment is incorrect

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

if (buttonId) {
_dismissDialog(this, buttonId);
} else {
// Stop the event if not handled by this dialog
Copy link
Member

Choose a reason for hiding this comment

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

We only want to stop the event if it is not targeted for the dialog. This keydown handler is called in the capture phase, so if we stop all events here, we would not be able to have editable text inside a dialog.

@gruehle
Copy link
Member

gruehle commented Mar 8, 2012

Initial review complete

@jasonsanjose
Copy link
Member Author

Fixed Glenn's comment, but does not address #427.

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