fix: close drop down menus with escape key - #6561
Polymorph0us wants to merge 2 commits into
Conversation
|
Thank you for opening your first PR. We appreciate the time and effort you spent in creating this improvement. A maintainer will look at these changes shortly, and conduct a review. Please be patient, as everyone here works on this app in their free time. We will get to your PR as soon as we can.
If any of the above is unclear, or if you want to clarify something before jumping into action, please let us know; either with a comment here under your PR, or on the Community Forum or on Discord. We value upfront communication, since this can reduce misunderstandings and improve the efficiency of the work. |
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved menu behavior, event-phase, cleanup, and pending-response issues remain.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds Escape-key dismissal for custom dropdown menus and synchronizes menubar state.
Changes:
- Adds Escape handling and submenu cleanup.
- Resets
WindowMenubarstate on Escape. - Reuses existing dismissal logic for keyboard events.
File summaries
| File | Summary |
|---|---|
source/common/vue/window/WindowMenubar.vue |
Adds Escape-based state reset handling; requires capture-phase registration and listener cleanup. |
source/common/modules/window-register/application-menu-helper.ts |
Adds keyboard dismissal and submenu tracking; requires fixes to callback handling and event capture. |
Review details
Suppressed comments (3)
source/common/modules/window-register/application-menu-helper.ts:350
- The newly added global keydown listener is not removed when a menu item is selected: the item handlers remove appMenu directly instead of calling closeMenu, so this cleanup block is skipped. Repeatedly opening and selecting from menus accumulates window listeners and their closures; route every menu-removal path through closeMenu (or explicitly remove both global listeners).
window.addEventListener('mousedown', closeMenu)
if (cleanup) {
window.addEventListener('keydown', closeMenu)
source/common/vue/window/WindowMenubar.vue:73
- The menubar's state-reset listener has the same bubbling-phase problem: an Escape handled by FileManager/FileTree or an inline filename input is stopped before this window listener runs, leaving currentSubmenu and menuCloseCallback populated even if the menu is closed by a capture-phase handler. Register this keydown listener in the capture phase and remove it on unmount so the component state is reset for those focused controls.
window.addEventListener('mousedown', resetState)
window.addEventListener('keydown', resetState)
source/common/vue/window/WindowMenubar.vue:64
- Escape only resets state when a menu callback already exists, but
getSubmenusetstargetElementbefore the asynchronous IPC response arrives. If Escape is pressed while a submenu request is pending (especially while switching between top-level items), the stale response still reachesshowSubmenuand opens the menu after it was dismissed. Cancel/mark the pending target on Escape and ignore the corresponding late response.
const resetState = (event?: Event) => {
if (event instanceof KeyboardEvent && event.key !== 'Escape') {
return
- Files reviewed: 2/2 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Description
the dropdown menus couldnt be dismissed with escape so i added escape handling
Changes
showPopupMenuinapplication-menu-helper.tsto attach akeydownlistener (for top-level menus) that cascades closure down to any active submenus via a lifted closure reference.WindowMenubar.vueto listen to thekeydownevent on the global window object to correctly reset its internal tracking state (currentSubmenuandmenuCloseCallback) upon an Escape key press.mousedowndismiss logic to handle the newKeyboardEventwithout relying onstopPropagation.Tested on
Linux
Additional information
Closes #6560
AI Disclosure Statement
-To identify the file responsible for the dropdown as well as a little bit of coding assistance
Declarations
this PR. Usage of AI to generate code has been documented and made
transparent. I understand that AI cannot be an author and the commit messages
do not contain any chatbots or agents as "co-authors". There are no copyright
issues with my code.
not use an LLM to draft this description.
the additional information section.