Allow Escape in modifier combinations - #43
Open
ubf-hunter wants to merge 1 commit into
Open
Conversation
Only bare Escape is reserved system-wide (it's fixed to panel:restore and owns App.tsx's five-rung dismiss chain). A modified Escape (Ctrl+Escape, Alt+Shift+Escape, ...) is a different chord and had no reason to be unbindable — it was swept up because App.tsx's dismiss handler checked event.key alone, with no modifier guard. - checkAccelerator / checkBinding (shared/accelerator.ts) now only treat Escape as reserved when it carries zero modifiers. - App.tsx's dismiss-chain handler bails out early when any modifier is held, so a modified Escape falls through to whatever the user bound it to instead of closing dialogs / leaving fullscreen. - Updated the accelerator.test.ts case that pinned the old blanket reservation, and added coverage for a modified Escape passing both checkAccelerator and checkBinding (including mid-sequence). The dismiss chain itself is intentionally untouched, per the issue: it is a five-rung priority chain, not a single action, so it can't move onto the regular keymap. Closes Travus#37
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
checkAccelerator/checkBinding(shared/accelerator.ts) now only treatEscapeas reserved when it carries zero modifiers, instead of unconditionally.App.tsx's dismiss-chain handler bails out early when any modifier is held, soCtrl+Escapeetc. fall through to whatever the user bound them to instead of closing dialogs / leaving fullscreen.accelerator.test.tscase that pinned the old blanket reservation, and added coverage for a modified Escape passing bothcheckAcceleratorandcheckBinding(including mid-sequence, e.g.CmdOrCtrl+K CmdOrCtrl+Escape).Per the issue, the dismiss chain itself is intentionally untouched — it's a five-rung priority chain, not a single action, so it can't move onto the regular keymap.
Closes #37
Test plan
npm run typecheck— cleannpm run lint— cleannpx vitest run— 127 passed