Fix: let modifier-key shortcuts (Cmd+-, Ctrl++ etc.) pass through to the browser#124
Open
paperboyo wants to merge 3 commits into
Open
Fix: let modifier-key shortcuts (Cmd+-, Ctrl++ etc.) pass through to the browser#124paperboyo wants to merge 3 commits into
paperboyo wants to merge 3 commits into
Conversation
… the browser handleKeyDown was unconditionally intercepting '+' and '-' keypresses, including when Cmd or Ctrl was held. This caused browser shortcuts like Cmd+- (zoom out) and Cmd+= (zoom in) to be swallowed, inserting a negated/positive chip instead. The fix adds an early return at the top of handleKeyDown when metaKey or ctrlKey is true. None of the keys handled in this function (+, -, :, Escape, Delete, Backspace, Enter, arrows) have legitimate Cmd/Ctrl meanings within CQL — ProseMirror's keymap plugin handles Mod-bindings (Mod-a, Mod-z, Ctrl-a, Ctrl-e) separately and fires before handleKeyDown. Fixes #118.
Contributor
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes browser/OS shortcuts (e.g. Cmd+-, Cmd+=) being swallowed by the CQL editor and inserting polarity chips instead of zooming. The fix is a one-line early return in handleKeyDown when metaKey or ctrlKey is held.
Changes:
- In
handleKeyDown, returnfalseearly when a modifier (Cmd/Ctrl) is pressed so the browser/ProseMirror keymap can handle the event.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The original guard returned false for *any* metaKey or ctrlKey combo, which silently broke Cmd+Backspace (chip deletion) and Cmd+Delete, since those never reached the switch cases that call removeChipAdjacentToSelection. Narrow the early return to only the keys that need protecting: if ((metaKey || ctrlKey) && (key === "+" || key === "-")) return false; This lets Cmd+- and Cmd++ pass through to the browser (zoom shortcuts) while leaving Cmd+Backspace, Cmd+Delete, and all other modifier combos falling through to the existing switch handlers as before.
Contributor
Four tests covering both sides of the fix: Chip creation (modifier keys do not trigger chip creation): - Cmd+- does not create a chip - Cmd++ does not create a chip - Ctrl+- does not create a chip These fail against the original code (no guard) and pass with the fix. Regression guard (deletion): - Cmd+Backspace still removes a chip adjacent to the cursor This fails against the overly-broad guard (metaKey || ctrlKey) that was narrowed in the previous commit, and passes with the correct narrow guard. Together the four tests pin down the exact shape of the fix: only + and - are exempted for modifier combos; all other keys (Backspace, Delete, etc.) continue to be handled as normal. Uses fireEvent.keyDown for the Cmd+Backspace test since editor.shortcut() does not propagate metaKey correctly through ProseMirror's event system in the test environment.
Author
|
Now narrowed and with tests. |
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
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.
[this is not me, this is Claude, I verified in Kupua it fixes the issue affecting Grid and sandbox too, but 🤖s, yeah]
handleKeyDown was unconditionally intercepting '+' and '-' keypresses, including when Cmd or Ctrl was held. This caused browser shortcut Cmd+- (zoom out) to be swallowed, inserting a negated chip instead.
What does this change?
The fix adds an early return in
handleKeyDownfor+and-whenmetaKeyorctrlKeyis held, so Cmd+- / Cmd++ / Ctrl+- pass through to the browser as zoom shortcuts rather than triggering chip creation. Other shortcut combos are unaffected.Fixes #118.
How has this change been tested?
Local build was linked to Kupua and I pressed Cmd+- while in the searchbox. Robot also ran some buns.
How can we measure success?
I know everything about “success”. It’s that thing, which, then… and… driveway. Driveway is a success.
Have we considered potential risks?
No, wait, we didn’t, now, did we? [EDIT: but we have added tests and have tested it in the sandbox too]
Images
Images? Oh, yeah. I liked this one I found today:
Accessibility