Feat/respect keyboard layout in code fallback#53
Conversation
…youts The `matchesKeyboardEvent` function falls back to `event.code` (physical key position) when `event.key` doesn't match the hotkey. This fallback exists to handle cases like macOS Option+T producing '†' instead of 'T'. However, when `event.key` is already a standard ASCII letter, this fallback incorrectly matches based on physical key position, breaking all non-QWERTY keyboard layouts (Dvorak, Colemak, AZERTY, etc.). For example, on Dvorak with macOS "Use keyboard layout for shortcuts": - Pressing Cmd + physical B key produces event.key='x', event.code='KeyB' - A hotkey registered as 'Mod+B' would incorrectly match via the code fallback, even though the user pressed Cmd+X in their layout The fix: when event.key is a standard ASCII letter (a-z), trust the keyboard layout mapping and skip the event.code fallback. The fallback is still used when event.key is a non-letter character (like '†') or a dead key. Fixes TanStack#17
@tanstack/hotkeys
@tanstack/hotkeys-devtools
@tanstack/preact-hotkeys
@tanstack/preact-hotkeys-devtools
@tanstack/react-hotkeys
@tanstack/react-hotkeys-devtools
@tanstack/solid-hotkeys
@tanstack/solid-hotkeys-devtools
commit: |
KevinVandy
force-pushed
the
feat/respect-keyboard-layout-in-code-fallback
branch
from
df20bf1 to
467f93c
Compare
|
@dmontagu I think I was able to solve non-ascii too by using regex |
|
@KevinVandy I can see why you might like this behavior if you are typically typing custom characters, but I believe this change may be too broad and may have broken a number of reasonable keyboard shortcuts involving the option key on MacOS. The characters in the tables below all match I can see how if you meant to type these characters it might be annoying, but my suspicion is that it will be more common to want to use some of these combinations as a shortcut than it will be for them to be intentionally typed. I'm not sure where to draw the line but you might want to be aware of this, in particular I would expect some issues to be reported as a result of this change given it will break so many previously-working shortcuts. I think in the case of an ASCII letter being typed it's less controversial to not fall back to key code, as basically the only way to get an ASCII letter as the key but not the key code is to have a difference of keyboard layout. But for the characters, I think the probably they were intended to be typed when there is a compatible keyboard shortcut is fairly low, and handling this properly is perhaps better handled for most users by disabling global hotkeys (or at least the expanded set of letters?) when entering a textbox through some API for managing the hotkeys, rather than making these unusable. Note that the below key combinations are on QWERTY, they will be register differently on other keyboard layouts (as it is based on the layout-determined letter, not the physical key, but the physical key won't change), so it may be hard to just whitelist based on key combinations. I would suggest if you want to be conservative to go with my original suggestion of just skipping the fallback on ASCII letters to have less impact on existing hotkeys. |
|
Hmm, ok, we may need to patch again then |
|
Would you want to follow up with another pr? |
2 participants
🎯 Changes
Fixes #17
The
matchesKeyboardEventfunction falls back toevent.code(physical key position) whenevent.keydoesn't match the registered hotkey. This fallback exists to handle cases where modifier keys cause the OS to report a special character instead of the expected letter (e.g., macOS Option+T producing†instead ofT).However, the fallback currently activates even when
event.keyis a standard ASCII letter — which means it matches based on physical key position rather than the keyboard layout's logical mapping. This breaks all non-QWERTY keyboard layouts (Dvorak, Colemak, AZERTY, etc.):Example on Dvorak (macOS with "Use keyboard layout for shortcuts" enabled):
event.key='x',event.code='KeyB'Mod+Bchecks:'x' === 'b'? No.event.code = 'KeyB'→'B' === 'B'? Yes → hotkey fires incorrectlyThe fix: When
event.keyis already a standard ASCII letter (a-z), trust the keyboard layout and returnfalseimmediately. Theevent.codefallback is still used whenevent.keyproduces a non-letter character (e.g.,†,´, dead keys).(To be clear, this "example" was a real issue in our Pydantic Logfire app, and is precisely how I noticed this bug, as I am a Dvorak user myself. I have applied the diff from this PR in our own frontend build via pnpm patches, and the problem is now fixed for me.)
✅ Checklist
pnpm run test:pr.🚀 Release Impact