From e58f56b46fb84895c1ef60f23dd4cdfdf90cff1e Mon Sep 17 00:00:00 2001 From: sudacode Date: Sun, 3 May 2026 20:29:06 -0700 Subject: [PATCH] Fix Yomitan popup shortcut precedence in keyboard-only mode - In keyboard-driven mode, popup keys (e.g. `j`/`k`) now forward to the Yomitan popup instead of firing configured session bindings like `cycle sid` - Non-keyboard-only mode behavior is unchanged - Add regression test covering the `KeyJ`/`cycle sid` conflict --- ...-only-Yomitan-popup-shortcut-precedence.md | 55 +++++++++++++++++++ changes/325-keyboard-popup-precedence.md | 4 ++ src/renderer/handlers/keyboard.test.ts | 34 ++++++++++++ src/renderer/handlers/keyboard.ts | 2 +- 4 files changed, 94 insertions(+), 1 deletion(-) create mode 100644 backlog/tasks/task-325 - Fix-keyboard-only-Yomitan-popup-shortcut-precedence.md create mode 100644 changes/325-keyboard-popup-precedence.md diff --git a/backlog/tasks/task-325 - Fix-keyboard-only-Yomitan-popup-shortcut-precedence.md b/backlog/tasks/task-325 - Fix-keyboard-only-Yomitan-popup-shortcut-precedence.md new file mode 100644 index 00000000..0e66bcb4 --- /dev/null +++ b/backlog/tasks/task-325 - Fix-keyboard-only-Yomitan-popup-shortcut-precedence.md @@ -0,0 +1,55 @@ +--- +id: TASK-325 +title: Fix keyboard-only Yomitan popup shortcut precedence +status: Done +assignee: + - codex +created_date: '2026-05-04 01:19' +updated_date: '2026-05-04 01:22' +labels: + - bug + - keyboard + - yomitan +dependencies: [] +priority: high +--- + +## Description + + +When keyboard-only mode is active and a Yomitan popup is visible, popup keyboard controls must win over overlay/mpv/session keybindings. Currently default overlay bindings such as bare `j` can fire instead of scrolling/navigating the Yomitan popup. + + +## Acceptance Criteria + +- [x] #1 With keyboard-only mode active and a Yomitan popup visible, pressing `j`/`k` forwards to the Yomitan popup instead of dispatching default session bindings such as primary subtitle track cycling. +- [x] #2 With keyboard-only mode inactive, existing popup-visible session binding behavior remains unchanged for bound keys. +- [x] #3 Regression coverage captures the keyboard-only popup precedence behavior. + + +## Implementation Plan + + +1. Add a focused regression in `src/renderer/handlers/keyboard.test.ts`: keyboard-only mode + visible Yomitan popup + bare `KeyJ` session binding should forward `KeyJ` to the popup and not dispatch the mpv/session binding. +2. Verify the new test fails before production changes. +3. Patch `src/renderer/handlers/keyboard.ts` so popup key handling ignores session-binding precedence only while keyboard-driven mode is enabled. +4. Run targeted renderer keyboard tests, then update acceptance criteria and final notes. + + +## Implementation Notes + + +Implementation: `handleYomitanPopupKeybind` now only lets configured session bindings take precedence when keyboard-driven mode is not enabled. In keyboard-only mode with a visible Yomitan popup, bare popup keys such as `KeyJ` forward to the popup instead of dispatching overlay/mpv keybindings. Added a regression covering `KeyJ` bound to `cycle sid`. + +Verification: targeted test failed before the production change, then passed after the fix. Full local gates run: `bun test src/renderer/handlers/keyboard.test.ts --test-name-pattern "keyboard mode: popup keybinds take precedence"`, `bun test src/renderer/handlers/keyboard.test.ts`, `bun run changelog:lint`, `bun run typecheck`, `bun run test:fast`, `bun run test:env`, `bun run build`, `bun run test:smoke:dist`. Build initially required `bun install --frozen-lockfile`, submodule init, and `stats/` locked deps install because this worktree had no dependencies/submodules checked out. + + +## Final Summary + + +Fixed keyboard-only Yomitan popup shortcut precedence by allowing popup key forwarding to bypass configured session bindings only while keyboard-driven mode is active. This makes popup controls such as `j`/`k` win over default overlay/mpv bindings like primary subtitle track cycling, while preserving existing non-keyboard-only popup behavior where configured bindings still fire. + +Added renderer keyboard regression coverage for the reported `KeyJ`/`cycle sid` conflict and added a changelog fragment for the user-visible overlay fix. + +Verification passed: targeted red/green regression, full renderer keyboard test file, changelog lint, typecheck, `test:fast`, `test:env`, build, and dist smoke tests. + diff --git a/changes/325-keyboard-popup-precedence.md b/changes/325-keyboard-popup-precedence.md new file mode 100644 index 00000000..85e10210 --- /dev/null +++ b/changes/325-keyboard-popup-precedence.md @@ -0,0 +1,4 @@ +type: fixed +area: overlay + +- Fixed keyboard-only Yomitan popup controls so popup shortcuts take precedence over overlay keybindings like `j`. diff --git a/src/renderer/handlers/keyboard.test.ts b/src/renderer/handlers/keyboard.test.ts index 13addd2b..6560ed43 100644 --- a/src/renderer/handlers/keyboard.test.ts +++ b/src/renderer/handlers/keyboard.test.ts @@ -613,6 +613,40 @@ test('keyboard mode: up/down/j/k forward keydown to yomitan popup when open', as } }); +test('keyboard mode: popup keybinds take precedence over configured session bindings', async () => { + const { ctx, handlers, testGlobals } = createKeyboardHandlerHarness(); + + try { + await handlers.setupMpvInputForwarding(); + handlers.updateSessionBindings([ + { + sourcePath: 'keybindings[0].key', + originalKey: 'KeyJ', + key: { code: 'KeyJ', modifiers: [] }, + actionType: 'mpv-command', + command: ['cycle', 'sid'], + }, + ] as never); + handlers.handleKeyboardModeToggleRequested(); + + ctx.state.yomitanPopupVisible = true; + testGlobals.setPopupVisible(true); + + testGlobals.dispatchKeydown({ key: 'j', code: 'KeyJ' }); + + assert.deepEqual(testGlobals.mpvCommands, []); + assert.deepEqual( + testGlobals.commandEvents + .filter((event) => event.type === 'forwardKeyDown') + .map((event) => event.code), + ['KeyJ'], + ); + } finally { + ctx.state.keyboardDrivenModeEnabled = false; + testGlobals.restore(); + } +}); + test('keyboard mode: repeated popup navigation keys are forwarded while popup is open', async () => { const { ctx, handlers, testGlobals } = createKeyboardHandlerHarness(); diff --git a/src/renderer/handlers/keyboard.ts b/src/renderer/handlers/keyboard.ts index 4558a0d7..12ccdd6b 100644 --- a/src/renderer/handlers/keyboard.ts +++ b/src/renderer/handlers/keyboard.ts @@ -865,7 +865,7 @@ export function createKeyboardHandlers( if (modifierOnlyCodes.has(e.code)) return false; const keyString = keyEventToString(e); - if (ctx.state.sessionBindingMap.has(keyString)) { + if (!ctx.state.keyboardDrivenModeEnabled && ctx.state.sessionBindingMap.has(keyString)) { return false; }