-
Notifications
You must be signed in to change notification settings - Fork 7.6k
Implement esc key to close pop-ups and inline widgets #1262
Conversation
The change in focus management breaks all commands that apply to inline editors. Now they all get redirected to the main editor instead. We could try to add a real focus manager, but that feels like scope creep for what was supposed to be a smallish story. Would it be simpler to just remove the part of this diff that tries to put focus on open menus? (Making Esc close menus too wasn't officially part of the story's requirements). |
if (!event.isDefaultPrevented()) { | ||
keyEvent.stopImmediatePropagation(); | ||
|
||
_removePopUp($popUp, true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like calling the "_"-prefixed one here means we'll leak by leaving the entry in _popUps...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
It turns out, the ESC key stuff still work with Menus even after undoing my fix. I threw it in there since it seemed like a bug to type while the menu was open. |
@tvoliter or @peterflynn ... ready to review |
When working on the unit tests, I notice that the contextMenuClose event does not fire when closing via Esc key. |
Ooops, github did not update the page. I'll open an issue. |
Renames and indents fix modified README
Renames and indents fix modified README fixed typo in readme
Renames and indents fix modified README fixed typo in readme Make things work Modified README
Renames and indents fix modified README fixed typo in readme Make things work Modified README Added setObject wrapper function
Renames and indents fix modified README fixed typo in readme Make things work Modified README Added setObject wrapper function changes
Renames and indents fix modified README fixed typo in readme Make things work Modified README Added setObject wrapper function changes merge conflict
Renames and indents fix modified README fixed typo in readme Make things work Modified README Added setObject wrapper function changes merge conflict small fix
Renames and indents fix modified README fixed typo in readme Make things work Modified README Added setObject wrapper function changes merge conflict small fix Fixed issue where the initial state of auto close tags (true) did not reflect in the editor statemanager mistake clean up Fixed error with editor not reflecting auto close initially Modified README and StateManager Typo in README fixed Modified UI.js and RemoteCommanderHandler.js as requested
Renames and indents fix modified README fixed typo in readme Make things work Modified README Added setObject wrapper function changes merge conflict small fix Fixed issue where the initial state of auto close tags (true) did not reflect in the editor statemanager mistake clean up Fixed error with editor not reflecting auto close initially Modified README and StateManager Typo in README fixed Modified UI.js and RemoteCommanderHandler.js as requested
Fix adobe#1262 - Option to disable Auto-Enclosing Tags
Renames and indents fix modified README fixed typo in readme Make things work Modified README Added setObject wrapper function changes merge conflict small fix Fixed issue where the initial state of auto close tags (true) did not reflect in the editor statemanager mistake clean up Fixed error with editor not reflecting auto close initially Modified README and StateManager Typo in README fixed Modified UI.js and RemoteCommanderHandler.js as requested
Assigned to @tvoliter
Note: I noticed that
Menus
andCodeHintManager
share positioning logic. It makes sense to refactor this intoPopUpManager
in the future.