diff --git a/internal/design/ui/hotkey.md b/internal/design/ui/hotkey.md index 9f6bf351..adc536ff 100644 --- a/internal/design/ui/hotkey.md +++ b/internal/design/ui/hotkey.md @@ -166,7 +166,7 @@ Only `0-9` is supported. Arbitrary ranges (e.g., `a-z`) are not — there's no u - Patterns are parsed into `{ modifiers, key }` at registration time — no parsing per event. - `event.key` is compared case-insensitively. - **Exact modifier matching** — all specified modifiers must be active, all unspecified must be inactive. `k` does not fire when Ctrl+K is pressed. -- **Implicit Shift** — For single non-letter character keys (`>`, `<`, `?`, `!`, etc.), `shiftKey` is ignored during matching. These characters require Shift on some layouts but not others. Explicitly specifying `Shift+>` still requires Shift to be held. +- **Implicit modifiers (Shift, Alt)** — For single non-letter character keys (`>`, `<`, `?`, `!`, etc.), `shiftKey` and `altKey` are ignored during matching. These characters require Shift on some layouts (US: `Shift+.` produces `>`), Alt/Option on others (some Mac layouts: `Option+Shift` produces `>`), or neither (some European layouts: `>` is an unshifted key). Explicitly specifying `Shift+>` or `Alt+>` still requires that modifier to be held. `ctrlKey` and `metaKey` remain strict — they are intentional modifier shortcuts, never layout artifacts. - `Mod` resolves at parse time based on platform detection (`navigator.userAgentData?.platform` with `navigator.platform` fallback). - IME composition input is filtered — events where `event.key === 'Unidentified'` are skipped. diff --git a/packages/core/src/dom/hotkey/hotkey.ts b/packages/core/src/dom/hotkey/hotkey.ts index c2662393..d8e2a7d3 100644 --- a/packages/core/src/dom/hotkey/hotkey.ts +++ b/packages/core/src/dom/hotkey/hotkey.ts @@ -71,12 +71,13 @@ export function parseHotkeyPattern(pattern: string): ParsedHotkeyBinding[] { } /** - * Single non-letter character — Shift was used to produce the character itself, - * not as a deliberate modifier (e.g. Shift+. → ">", Shift+/ → "?"). + * Single non-letter character — layout-dependent modifiers (Shift, Alt/Option) + * were used to produce the character itself, not as deliberate modifiers + * (e.g. Shift+. → ">", Option+Shift → ">" on some Mac layouts). * Letters excluded because Shift changes case intentionally (k vs K). * Named keys excluded because event.key.length > 1 (ArrowLeft, Tab, etc.). */ -function isImplicitShift(key: string): boolean { +function isImplicitModifierKey(key: string): boolean { return key.length === 1 && !/[a-z]/i.test(key); } @@ -88,15 +89,17 @@ export function matchesHotkeyEvent(binding: ParsedHotkeyBinding, event: Keyboard // Case-insensitive key comparison. if (event.key.toLowerCase() !== binding.key) return false; - // Implicit Shift: non-letter character keys (>, <, ?, !) may require Shift to produce - // on some layouts but not others. Treat Shift as present only when the event has it, - // but ignore extra shiftKey when the binding doesn't ask for it. - const shiftKey = isImplicitShift(event.key) ? event.shiftKey && binding.modifiers.has('shift') : event.shiftKey; + // Implicit modifiers: non-letter character keys (>, <, ?, !) may require Shift or + // Alt (Option) to produce on some layouts but not others. Treat these modifiers as + // present only when the binding explicitly asks for them. + const implicit = isImplicitModifierKey(event.key); + const shiftKey = implicit ? event.shiftKey && binding.modifiers.has('shift') : event.shiftKey; + const altKey = implicit ? event.altKey && binding.modifiers.has('alt') : event.altKey; // Exact modifier matching — all four must agree. if (shiftKey !== binding.modifiers.has('shift')) return false; if (event.ctrlKey !== binding.modifiers.has('ctrl')) return false; - if (event.altKey !== binding.modifiers.has('alt')) return false; + if (altKey !== binding.modifiers.has('alt')) return false; if (event.metaKey !== binding.modifiers.has('meta')) return false; return true; diff --git a/packages/core/src/dom/hotkey/tests/hotkey.test.ts b/packages/core/src/dom/hotkey/tests/hotkey.test.ts index fa5cfc49..cb0b17cd 100644 --- a/packages/core/src/dom/hotkey/tests/hotkey.test.ts +++ b/packages/core/src/dom/hotkey/tests/hotkey.test.ts @@ -121,7 +121,7 @@ describe('matchesHotkeyEvent', () => { expect(matchesHotkeyEvent(binding, createEvent('j'))).toBe(false); }); - describe('implicit shift for non-letter characters', () => { + describe('implicit modifiers for non-letter characters', () => { it('matches > when Shift is held (US keyboard: Shift+. produces >)', () => { const binding = parseHotkeyPattern('>')[0]!; expect(matchesHotkeyEvent(binding, createEvent('>', { shiftKey: true }))).toBe(true); @@ -173,6 +173,47 @@ describe('matchesHotkeyEvent', () => { expect(matchesHotkeyEvent(binding, createEvent('>', { shiftKey: true }))).toBe(true); expect(matchesHotkeyEvent(binding, createEvent('>'))).toBe(false); }); + + it('matches > when Alt is held (Mac Option key produces >)', () => { + const binding = parseHotkeyPattern('>')[0]!; + expect(matchesHotkeyEvent(binding, createEvent('>', { altKey: true }))).toBe(true); + }); + + it('matches > when Alt and Shift are held (Mac Option+Shift produces >)', () => { + const binding = parseHotkeyPattern('>')[0]!; + expect(matchesHotkeyEvent(binding, createEvent('>', { altKey: true, shiftKey: true }))).toBe(true); + }); + + it('matches < when Alt is held (Mac Option key produces <)', () => { + const binding = parseHotkeyPattern('<')[0]!; + expect(matchesHotkeyEvent(binding, createEvent('<', { altKey: true }))).toBe(true); + }); + + it('Alt+> binding requires Alt held', () => { + const binding = parseHotkeyPattern('Alt+>')[0]!; + expect(matchesHotkeyEvent(binding, createEvent('>', { altKey: true }))).toBe(true); + expect(matchesHotkeyEvent(binding, createEvent('>'))).toBe(false); + }); + + it('rejects letter k when Alt is held (Alt is not implicit for letters)', () => { + const binding = parseHotkeyPattern('k')[0]!; + expect(matchesHotkeyEvent(binding, createEvent('k', { altKey: true }))).toBe(false); + }); + + it('rejects ArrowLeft when Alt is held but binding has no Alt', () => { + const binding = parseHotkeyPattern('ArrowLeft')[0]!; + expect(matchesHotkeyEvent(binding, createEvent('ArrowLeft', { altKey: true }))).toBe(false); + }); + + it('rejects > when Ctrl is held but binding has no Ctrl', () => { + const binding = parseHotkeyPattern('>')[0]!; + expect(matchesHotkeyEvent(binding, createEvent('>', { ctrlKey: true }))).toBe(false); + }); + + it('rejects > when Meta is held but binding has no Meta', () => { + const binding = parseHotkeyPattern('>')[0]!; + expect(matchesHotkeyEvent(binding, createEvent('>', { metaKey: true }))).toBe(false); + }); }); });