From 2525a122640c01a7f098fa6f38abcad63e187ca4 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Sun, 27 Sep 2026 17:28:10 +0200 Subject: [PATCH] fix(keys): a key a focused widget already handled fires no shortcut The player's settings tabs switched tab on an arrow and prevented its default, yet the player's window listener still sought the video 10 s. dispatch() now skips a defaultPrevented key for every scope; listNav's private copy of the same guard goes. --- frontend/src/lib/keymap.test.ts | 15 +++++++++++++++ frontend/src/lib/keymap.ts | 6 +++++- frontend/src/lib/listNav.ts | 6 ++---- 3 files changed, 22 insertions(+), 5 deletions(-) diff --git a/frontend/src/lib/keymap.test.ts b/frontend/src/lib/keymap.test.ts index 49bc8d04..3118eb2a 100644 --- a/frontend/src/lib/keymap.test.ts +++ b/frontend/src/lib/keymap.test.ts @@ -154,6 +154,21 @@ describe("dispatch", () => { expect(calls).toEqual([]); }); + // E28 UAT: an arrow the player's settings tabs had already taken (they switch tab and prevent its + // default) ALSO sought the video 10 s. A key a focused widget consumed is spent, for every scope. + it("ignores a key a focused widget already handled", () => { + const calls: string[] = []; + const tab = document.createElement("button"); + tab.setAttribute("role", "tab"); + document.body.append(tab); + tab.addEventListener("keydown", (e) => e.preventDefault()); + expect(dispatch(press({ key: "ArrowRight" }, tab), "plexPlayer", plexHandlers(calls))).toBe( + false, + ); + expect(calls).toEqual([]); + tab.remove(); + }); + // A `handledLocally` row is documentation + a shared chord, matched on the focused element by its // own component (Shift+F10 belongs to whichever row has focus). `dispatch` must not claim it, and // the mapped handler type must not demand one — this test is the runtime half of that. diff --git a/frontend/src/lib/keymap.ts b/frontend/src/lib/keymap.ts index 604d3826..3372b9c3 100644 --- a/frontend/src/lib/keymap.ts +++ b/frontend/src/lib/keymap.ts @@ -322,7 +322,7 @@ const stepsOf = (chord: string): string[] => chord.split(" "); * whether to `preventDefault`, because only it knows if it actually acted) or a sequence's first * step was taken. `handlers` is a mapped type over the scope's ids, so the table and the * implementation cannot drift without tsc saying so. Skipped while typing unless the row opts in - * with `allowInInput` (Escape, and only Escape), and — for a page-level scope — while a layer that + * with `allowInInput` (Escape, and only Escape), on a key a focused widget already handled, and — for a page-level scope — while a layer that * owns the keyboard is open. The rule lives here rather than in each listener so a new page-level * surface cannot forget it. */ export function dispatch( @@ -331,6 +331,10 @@ export function dispatch( handlers: { readonly [K in ShortcutId]: (e: KeyboardEvent) => void }, ): boolean { if (sequenceEvents.has(e)) return false; + // A key a focused widget already handled is not a shortcut: every listener here is on `window`, + // so a tablist or a keyboard drag (dnd-kit, on the document) has acted on it and prevented its + // default first. The player's settings tabs switched tab AND sought the video 10 s (E28 UAT). + if (e.defaultPrevented) return false; // An Escape a layer already took (a menu that just closed) is spent: a page or a player reacting to // it too is the double close the layer stack exists to prevent. if (e.key === "Escape" && escapeHandled(e)) return false; diff --git a/frontend/src/lib/listNav.ts b/frontend/src/lib/listNav.ts index 415c5e43..b0a94845 100644 --- a/frontend/src/lib/listNav.ts +++ b/frontend/src/lib/listNav.ts @@ -320,10 +320,8 @@ function crossRow(from: NavList, item: HTMLElement, dy: -1 | 1): void { } function onKey(e: KeyboardEvent): void { - // A key a widget inside the item already handled is not ours: during a keyboard drag a playlist - // row's grip moves the row with the arrows (dnd-kit listens on the document, before this window - // listener, and prevents the default of every key it acts on). - if (e.defaultPrevented) return; + // A key a widget inside the item already handled is not ours (a keyboard drag's arrows): `dispatch` + // skips every such key, for every scope. const arrow = (dx: -1 | 0 | 1, dy: -1 | 0 | 1) => (ev: KeyboardEvent) => { if (move(dx, dy)) ev.preventDefault(); };