diff --git a/internal/design/ui/hotkey.md b/internal/design/ui/hotkey.md index ad94ca42..9f6bf351 100644 --- a/internal/design/ui/hotkey.md +++ b/internal/design/ui/hotkey.md @@ -58,8 +58,8 @@ Each element declares one binding. To bind multiple keys to the same action, use - - + + @@ -75,7 +75,7 @@ Each element declares one binding. To bind multiple keys to the same action, use - + ``` `keys` (not `key`) avoids collision with React's reserved `key` prop. @@ -141,7 +141,7 @@ Key patterns use `KeyboardEvent.key` values — layout-dependent, mnemonic ("K f | `Space` | Space bar | | `ArrowLeft` | Left arrow | | `0-9` | Any digit key (0 through 9) | -| `Shift+>` | Shift + > (the shifted character, not the unshifted key) | +| `>` | > key, no modifiers (on US keyboards, produced by Shift+.) | | `Mod+k` | Cmd+K on macOS, Ctrl+K elsewhere | | `Ctrl+Shift+f` | Ctrl + Shift + F | @@ -166,6 +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. - `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. @@ -312,7 +313,7 @@ Every major video player ships keyboard shortcuts. The bindings are remarkably c | Seek ±10s | `J` / `L` | YouTube, Vimeo, Media Chrome | | Percentage seek | `0`–`9` | YouTube, Netflix, Video.js 7 | | Captions | `C` | Vimeo, Media Chrome, Vidstack | -| Speed | `Shift+>` / `Shift+<` | YouTube, VLC, Media Chrome, Vidstack | +| Speed | `>` / `<` | YouTube, VLC, Media Chrome, Vidstack | | PiP | `I` or `P` | YouTube (`I`), Media Chrome (`P`) | | Help overlay | `Shift+?` | YouTube, Vimeo, Media Chrome | @@ -351,8 +352,8 @@ Not shipped as a preset — documented as the standard set for users to compose: | `0-9` | `seekToPercent` | — | YouTube/Netflix. Each digit × 10%. | | `Home` | `seekToPercent` | `0` | Jump to start. | | `End` | `seekToPercent` | `100` | Jump to end. | -| `Shift+>` | `speedUp` | — | YouTube/VLC. Steps through `playbackRates`. | -| `Shift+<` | `speedDown` | — | YouTube/VLC. Steps through `playbackRates`. | +| `>` | `speedUp` | — | YouTube/VLC. Steps through `playbackRates`. | +| `<` | `speedDown` | — | YouTube/VLC. Steps through `playbackRates`. | ## Edge cases diff --git a/packages/core/src/dom/hotkey/hotkey.ts b/packages/core/src/dom/hotkey/hotkey.ts index 61c3990e..c2662393 100644 --- a/packages/core/src/dom/hotkey/hotkey.ts +++ b/packages/core/src/dom/hotkey/hotkey.ts @@ -31,8 +31,8 @@ const MODIFIER_KEYS = new Set(['shift', 'ctrl', 'alt', 'meta']); * * @example * ```ts - * parseHotkeyPattern('Shift+>'); - * // [{ modifiers: Set('shift'), key: '>', originalKey: '>' }] + * parseHotkeyPattern('>'); + * // [{ modifiers: Set(), key: '>', originalKey: '>' }] * * parseHotkeyPattern('0-9'); * // 10 bindings, one per digit @@ -70,6 +70,16 @@ export function parseHotkeyPattern(pattern: string): ParsedHotkeyBinding[] { return [{ modifiers, key, originalKey: rawKey }]; } +/** + * Single non-letter character — Shift was used to produce the character itself, + * not as a deliberate modifier (e.g. Shift+. → ">", Shift+/ → "?"). + * 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 { + return key.length === 1 && !/[a-z]/i.test(key); +} + /** Whether a parsed binding matches a keyboard event. */ export function matchesHotkeyEvent(binding: ParsedHotkeyBinding, event: KeyboardEvent): boolean { // IME composition filtering. @@ -78,8 +88,13 @@ 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; + // Exact modifier matching — all four must agree. - if (event.shiftKey !== binding.modifiers.has('shift')) return false; + 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 (event.metaKey !== binding.modifiers.has('meta')) return false; diff --git a/packages/core/src/dom/hotkey/tests/aria.test.ts b/packages/core/src/dom/hotkey/tests/aria.test.ts index 3e5bc921..9012faf0 100644 --- a/packages/core/src/dom/hotkey/tests/aria.test.ts +++ b/packages/core/src/dom/hotkey/tests/aria.test.ts @@ -13,7 +13,7 @@ describe('toAriaKeyShortcut', () => { }); it('maps shift to Shift', () => { - expect(toAriaKeyShortcut(parseHotkeyPattern('Shift+>'))).toBe('Shift+>'); + expect(toAriaKeyShortcut(parseHotkeyPattern('Shift+ArrowLeft'))).toBe('Shift+ArrowLeft'); }); it('formats multiple modifiers in consistent order', () => { diff --git a/packages/core/src/dom/hotkey/tests/hotkey.test.ts b/packages/core/src/dom/hotkey/tests/hotkey.test.ts index 6db99b0e..fa5cfc49 100644 --- a/packages/core/src/dom/hotkey/tests/hotkey.test.ts +++ b/packages/core/src/dom/hotkey/tests/hotkey.test.ts @@ -13,10 +13,10 @@ describe('parseHotkeyPattern', () => { }); it('parses Shift modifier', () => { - const result = parseHotkeyPattern('Shift+>'); + const result = parseHotkeyPattern('Shift+ArrowLeft'); expect(result).toHaveLength(1); - expect(result[0]!.key).toBe('>'); + expect(result[0]!.key).toBe('arrowleft'); expect(result[0]!.modifiers.has('shift')).toBe(true); expect(result[0]!.modifiers.size).toBe(1); }); @@ -120,6 +120,60 @@ describe('matchesHotkeyEvent', () => { const binding = parseHotkeyPattern('k')[0]!; expect(matchesHotkeyEvent(binding, createEvent('j'))).toBe(false); }); + + describe('implicit shift 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); + }); + + it('matches < when Shift is held (US keyboard: Shift+, produces <)', () => { + const binding = parseHotkeyPattern('<')[0]!; + expect(matchesHotkeyEvent(binding, createEvent('<', { shiftKey: true }))).toBe(true); + }); + + it('matches > when Shift is NOT held (European keyboard: > is unshifted)', () => { + const binding = parseHotkeyPattern('>')[0]!; + expect(matchesHotkeyEvent(binding, createEvent('>'))).toBe(true); + }); + + it('matches ? when Shift is held', () => { + const binding = parseHotkeyPattern('?')[0]!; + expect(matchesHotkeyEvent(binding, createEvent('?', { shiftKey: true }))).toBe(true); + }); + + it('matches Ctrl+> with Ctrl and Shift held', () => { + const binding = parseHotkeyPattern('Ctrl+>')[0]!; + expect(matchesHotkeyEvent(binding, createEvent('>', { ctrlKey: true, shiftKey: true }))).toBe(true); + }); + + it('rejects Ctrl+> when only Shift is held (no Ctrl)', () => { + const binding = parseHotkeyPattern('Ctrl+>')[0]!; + expect(matchesHotkeyEvent(binding, createEvent('>', { shiftKey: true }))).toBe(false); + }); + + it('rejects letter k when Shift is held (Shift changes letter case)', () => { + const binding = parseHotkeyPattern('k')[0]!; + expect(matchesHotkeyEvent(binding, createEvent('K', { shiftKey: true }))).toBe(false); + }); + + it('requires exact Shift for named keys', () => { + const binding = parseHotkeyPattern('Shift+ArrowLeft')[0]!; + expect(matchesHotkeyEvent(binding, createEvent('ArrowLeft', { shiftKey: true }))).toBe(true); + expect(matchesHotkeyEvent(binding, createEvent('ArrowLeft'))).toBe(false); + }); + + it('rejects ArrowLeft when Shift is held but binding has no Shift', () => { + const binding = parseHotkeyPattern('ArrowLeft')[0]!; + expect(matchesHotkeyEvent(binding, createEvent('ArrowLeft', { shiftKey: true }))).toBe(false); + }); + + it('Shift+> binding requires Shift held', () => { + const binding = parseHotkeyPattern('Shift+>')[0]!; + expect(matchesHotkeyEvent(binding, createEvent('>', { shiftKey: true }))).toBe(true); + expect(matchesHotkeyEvent(binding, createEvent('>'))).toBe(false); + }); + }); }); describe('createHotkey', () => { diff --git a/packages/html/src/define/audio/skin.ts b/packages/html/src/define/audio/skin.ts index 50a3fc9d..fa1da084 100644 --- a/packages/html/src/define/audio/skin.ts +++ b/packages/html/src/define/audio/skin.ts @@ -108,8 +108,8 @@ function getTemplateHTML() { - - + + `; } diff --git a/packages/html/src/define/video/skin.ts b/packages/html/src/define/video/skin.ts index 8b830cb6..96d643f0 100644 --- a/packages/html/src/define/video/skin.ts +++ b/packages/html/src/define/video/skin.ts @@ -147,8 +147,8 @@ function getTemplateHTML() { - - + + `; } diff --git a/packages/react/src/presets/audio/skin.tsx b/packages/react/src/presets/audio/skin.tsx index d3ee198a..783abead 100644 --- a/packages/react/src/presets/audio/skin.tsx +++ b/packages/react/src/presets/audio/skin.tsx @@ -170,8 +170,8 @@ export function AudioSkin(props: AudioSkinProps): ReactNode { - - + + ); } diff --git a/packages/react/src/presets/video/skin.tsx b/packages/react/src/presets/video/skin.tsx index 8c45610a..e02b978c 100644 --- a/packages/react/src/presets/video/skin.tsx +++ b/packages/react/src/presets/video/skin.tsx @@ -247,8 +247,8 @@ export function VideoSkin(props: VideoSkinProps): ReactNode { - - + + ); }