From dc0612fc86a52df8397197cf1fb524ae59e97a67 Mon Sep 17 00:00:00 2001 From: Phil Haack Date: Thu, 16 Jul 2026 12:49:06 -0700 Subject: [PATCH 1/2] =?UTF-8?q?fix(shortcuts):=20match=20=E2=8C=98?= =?UTF-8?q?=E2=8C=A5=20shortcuts=20by=20layout-aware=20key,=20not=20physic?= =?UTF-8?q?al=20position?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The keyboard shortcut engine resolved the letter of a ⌘⌥ (command+option) chord from event.code (physical key position) whenever Option/Alt was held. Since every app shortcut uses command+option, this broke every ⌘⌥ shortcut on non-QWERTY layouts (Dvorak, AZERTY, etc.): the physical key produces a different letter than the one printed on the key. Prefer the layout-aware event.key, which is already correct on every layout. Only fall back to the physical event.code for the macOS quirk it was written for — where holding Option turns event.key into a special glyph or dead key (e.g. ⌥K becomes "˚") — and never when event.key already gives a usable letter or digit. The older useKeyboardHotkeys hook already matches on event.key; this brings the newer engine in line. Generated-By: PostHog Code Task-Id: 570b6a35-794b-45bb-933f-7b54751b014a --- .../Shortcuts/shortcutLogic.test.ts | 60 +++++++++++++++++++ .../components/Shortcuts/shortcutLogic.tsx | 9 ++- 2 files changed, 67 insertions(+), 2 deletions(-) create mode 100644 frontend/src/lib/components/Shortcuts/shortcutLogic.test.ts diff --git a/frontend/src/lib/components/Shortcuts/shortcutLogic.test.ts b/frontend/src/lib/components/Shortcuts/shortcutLogic.test.ts new file mode 100644 index 000000000000..707269d4ad5d --- /dev/null +++ b/frontend/src/lib/components/Shortcuts/shortcutLogic.test.ts @@ -0,0 +1,60 @@ +import { initKeaTests } from '~/test/init' + +import { shortcutLogic } from './shortcutLogic' + +jest.mock('lib/posthog-typed', () => ({ + __esModule: true, + default: { __loaded: true, capture: jest.fn() }, +})) + +// The ⌘⌥ matching path branches on isMac(), which is captured once at module load. +jest.mock('lib/utils/dom', () => ({ + ...jest.requireActual('lib/utils/dom'), + isMac: () => true, +})) + +describe('shortcutLogic', () => { + let logic: ReturnType + + beforeEach(() => { + initKeaTests() + logic = shortcutLogic() + logic.mount() + }) + + afterEach(() => { + logic.unmount() + }) + + function register(name: string, keybind: string[][]): jest.Mock { + const callback = jest.fn() + logic.actions.registerShortcut({ name, keybind, intent: name, interaction: 'function', callback }) + return callback + } + + function press(init: KeyboardEventInit): void { + window.dispatchEvent(new KeyboardEvent('keydown', { bubbles: true, ...init })) + } + + it('matches ⌘⌥ shortcuts by the layout-aware key, not the physical key position', () => { + const onC = register('c-action', [['command', 'option', 'c']]) + const onI = register('i-action', [['command', 'option', 'i']]) + + // On Dvorak the key labelled "c" sits where QWERTY has "i": event.code is "KeyI" + // but event.key is the true letter "c". Matching must follow the letter, not the position. + press({ key: 'c', code: 'KeyI', metaKey: true, altKey: true }) + + expect(onC).toHaveBeenCalledTimes(1) + expect(onI).not.toHaveBeenCalled() + }) + + it('falls back to the physical key only when Option turns event.key into a non-letter glyph', () => { + const onK = register('k-action', [['command', 'option', 'k']]) + + // macOS US layout: ⌥K produces the glyph "˚" as event.key, so the physical code is the + // only reliable source of the intended letter. + press({ key: '˚', code: 'KeyK', metaKey: true, altKey: true }) + + expect(onK).toHaveBeenCalledTimes(1) + }) +}) diff --git a/frontend/src/lib/components/Shortcuts/shortcutLogic.tsx b/frontend/src/lib/components/Shortcuts/shortcutLogic.tsx index a6c40ce162fd..353ff1aa99bb 100644 --- a/frontend/src/lib/components/Shortcuts/shortcutLogic.tsx +++ b/frontend/src/lib/components/Shortcuts/shortcutLogic.tsx @@ -187,9 +187,14 @@ export const shortcutLogic = kea([ pressedKeys.push('option') } - // Handle Alt key combinations - event.key can change with Alt held + // event.key is layout-aware, so it stays correct on non-QWERTY layouts (Dvorak, + // AZERTY, etc.) where the physical event.code position does not match the letter. + // Prefer it. The only reason to consult the physical event.code is the macOS quirk + // where holding Option turns event.key into a special glyph or dead key (e.g. ⌥K + // becomes "˚"); fall back to event.code only in that case, never when event.key + // already gives us a usable letter or digit. let keyToAdd = event.key.toLowerCase() - if (event.altKey) { + if (event.altKey && !/^[a-z0-9]$/.test(keyToAdd)) { const codeMatch = event.code.match(/^Key([A-Z])$/) if (codeMatch) { keyToAdd = codeMatch[1].toLowerCase() From 20ac6a6645401a89005a34704a5cb1a0d330fb99 Mon Sep 17 00:00:00 2001 From: Phil Haack Date: Thu, 16 Jul 2026 13:18:53 -0700 Subject: [PATCH 2/2] test(shortcuts): stabilize shortcutLogic test setup The partial lib/posthog-typed mock replaced the real posthog-js re-export, which broke the common logics initKeaTests mounts, failing both tests in setup. shortcutLogic is standalone, so skip common-logic mounting (initKeaTests(false)) and drop the posthog mock; keep only the isMac mock so the modifier deterministically resolves to command. Generated-By: PostHog Code Task-Id: 570b6a35-794b-45bb-933f-7b54751b014a --- .../src/lib/components/Shortcuts/shortcutLogic.test.ts | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/frontend/src/lib/components/Shortcuts/shortcutLogic.test.ts b/frontend/src/lib/components/Shortcuts/shortcutLogic.test.ts index 707269d4ad5d..ccccdcf0cddd 100644 --- a/frontend/src/lib/components/Shortcuts/shortcutLogic.test.ts +++ b/frontend/src/lib/components/Shortcuts/shortcutLogic.test.ts @@ -2,12 +2,8 @@ import { initKeaTests } from '~/test/init' import { shortcutLogic } from './shortcutLogic' -jest.mock('lib/posthog-typed', () => ({ - __esModule: true, - default: { __loaded: true, capture: jest.fn() }, -})) - -// The ⌘⌥ matching path branches on isMac(), which is captured once at module load. +// The ⌘⌥ matching path branches on isMac(), captured once at module load. Force it so the +// modifier resolves to 'command' regardless of the host platform running the test. jest.mock('lib/utils/dom', () => ({ ...jest.requireActual('lib/utils/dom'), isMac: () => true, @@ -17,7 +13,7 @@ describe('shortcutLogic', () => { let logic: ReturnType beforeEach(() => { - initKeaTests() + initKeaTests(false) logic = shortcutLogic() logic.mount() })