fix(hotkey): migrate useHotkeys to event.key|code#114192
Merged
natemoo-re merged 8 commits intomasterfrom Apr 29, 2026
Merged
Conversation
github-actions
Bot
added
the
Scope: Frontend
natemoo-re
force-pushed
the
nm/hotkey-i18n
branch
from
7abb250 to
f367f77
Compare
natemoo-re
force-pushed
the
nm/hotkey-i18n
branch
from
9208fdb to
1bda131
Compare
natemoo-re
changed the title
useHotkeys to event.key|code
natemoo-re
force-pushed
the
nm/hotkey-i18n
branch
from
4535e6b to
13983f3
Compare
natemoo-re
commented
Apr 29, 2026
| useHotkeys([ | ||
| { | ||
| match: 'Escape', | ||
| enabled: isDrawerOpen, |
Member
Author
There was a problem hiding this comment.
Big refactoring win! Added an enabled option to useHotkeys because the migration to key uncovered a latent bug here that we couldn't detect before.
33452cf migrated the hook from event.keyCode to event.key. GlobalDrawer had been registering an unconditional useHotkeys for Escape—this previously failed silently in tests because userEvent.keyboard('{Escape}') doesn't set keyCode in jsdom, but now it fires (and calls preventDefault) on every Escape—even when no drawer was open. The modal's escape handler bails on e.defaultPrevented (added in PR #109556 to let react-aria child overlays absorb Escape), so the global modal stopped closing entirely.
natemoo-re
added a commit
that referenced
this pull request
Apr 29, 2026
Seer currently accepts `Command+/` as a shortcut for opening the drawer. One problem: the `/` key is a secondary key on all European keyboards, and we didn't support those. After a [quite (#114198)](#114198) [productive (#114192)](#114192) deep-dive on our hotkey primitives to fix edge cases, I've circled back to the most robust fix for non-QWERTY keyboards being the most straightforward one—we should just hard-code keybindings for a known set of layouts. This PR adds support for `command+shift+7` for QWERTZ and `command+shift+.` for AZERTY which should cover where the majority of our users see `/` on their keyboards. Regrettably Eurocentric, but that's best effort. ### Alternatives Considered The Chrome-only [`navigator.keyboard`](https://developer.mozilla.org/en-US/docs/Web/API/Keyboard) interface seemed promising, but after quite a bit of experimentation, proved to be a dead-end here. Given that the API is `async`, only supported in secure environments, and seems unlikely to advance further in the spec process, I decided to go with a solution that worked everywhere.
JonasBa
reviewed
Apr 29, 2026
| navigationContext() | ||
| ); | ||
|
|
||
| fireEvent.keyDown(document, {keyCode: 66 /* b */, ctrlKey: true}); |
JonasBa
approved these changes
Apr 29, 2026
3 tasks
cleptric
pushed a commit
that referenced
this pull request
May 5, 2026
Seer currently accepts `Command+/` as a shortcut for opening the drawer. One problem: the `/` key is a secondary key on all European keyboards, and we didn't support those. After a [quite (#114198)](#114198) [productive (#114192)](#114192) deep-dive on our hotkey primitives to fix edge cases, I've circled back to the most robust fix for non-QWERTY keyboards being the most straightforward one—we should just hard-code keybindings for a known set of layouts. This PR adds support for `command+shift+7` for QWERTZ and `command+shift+.` for AZERTY which should cover where the majority of our users see `/` on their keyboards. Regrettably Eurocentric, but that's best effort. ### Alternatives Considered The Chrome-only [`navigator.keyboard`](https://developer.mozilla.org/en-US/docs/Web/API/Keyboard) interface seemed promising, but after quite a bit of experimentation, proved to be a dead-end here. Given that the API is `async`, only supported in secure environments, and seems unlikely to advance further in the spec process, I decided to go with a solution that worked everywhere.
cleptric
pushed a commit
that referenced
this pull request
May 5, 2026
the keyboard detection logic for `useHotkeys` was using [`keyCode`](https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/keyCode) which is deprecated and MDN actively discourages because it is tightly coupled to keyboard layout. this pr migrates our implementation to the modern replacements, which are [`code`](https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/code) (baseline limited availability due to Android compat issues) and [`key`](https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/key) (baseline widely available). by relying on these `KeyboardEvent` properties, we get both the **value** of the key and the **physical location** of the key in a layout-agnostic manner. ~~this _should_ fix `command+/` handling on german keyboard layouts as well as a host of other keyboard problems~~ it did not ([but #114198 does!](#114198)) but this is generally more robust so we should merge anyway
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
2 participants
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.
the keyboard detection logic for
useHotkeyswas usingkeyCodewhich is deprecated and MDN actively discourages because it is tightly coupled to keyboard layout.this pr migrates our implementation to the modern replacements, which are
code(baseline limited availability due to Android compat issues) andkey(baseline widely available).by relying on these
KeyboardEventproperties, we get both the value of the key and the physical location of the key in a layout-agnostic manner.this should fixit did not (but #114198 does!) but this is generally more robust so we should merge anywaycommand+/handling on german keyboard layouts as well as a host of other keyboard problems