Dismiss popups and context menus with Escape#22
Conversation
Robin-Reiche
left a comment
Robin-Reiche
left a comment
There was a problem hiding this comment.
Nice, this is the cleanest of the three and I am happy to merge. I checked the AG Grid path: hidePopupMenu() is a real public GridApi method in 32.3.3 and it does close the filter popup. And since you only preventDefault and never stopPropagation, AG Grid's native Escape close still runs too, so the branch is correct and robust. Just one comment-clarity nit and a trailing newline, inline.
|
|
||
| // Sets up a global Esc key listener to dismiss all popups (wired once at startup). | ||
| // - Uses the `capture phase` (true) to intercept the event before AG Grid consumes it. | ||
| // - Does NOT call `stopPropagation`, allowing individual popups to run their own cleanup logic (e.g., resetting state). |
There was a problem hiding this comment.
Tiny clarity nit: this says individual popups run their own cleanup on Escape, but of the seven POPUP_IDS only rename and go-to-row have an Escape handler. Both are bound to their input element, so they only fire while that input has focus. For the context menus and dropdowns closeAllPopups() just toggles .hidden and nothing resets. Worth narrowing the comment to those two input-bound cases so a future reader does not assume Escape resets state for every popup.
There was a problem hiding this comment.
Narrowed the comment, only rename and go-to-row have an input-bound Esc handler, the rest just hide.
| closeAllPopups(); | ||
| e.preventDefault(); | ||
| }, true); | ||
| } No newline at end of file |
There was a problem hiding this comment.
Nit: missing trailing newline at end of file (the diff shows 'No newline at end of file'). The other webview files end with one, so adding it back avoids a noisy diff later.
There was a problem hiding this comment.
Added the trailing newline back.
2 participants
Adds a global Escape listener via
closeAllPopupsto dismiss context menus and popovers. It only intercepts when a popup is actually open, leavingEscfree to cancel cell edits otherwise.