From 92c305bab9281c6ce08b03d10d8adddf0131d244 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Sun, 13 Sep 2026 00:17:09 +0200 Subject: [PATCH 01/16] fix(nav): a navigation closes every overlay before it pushes The overlay history stack let a navigation land ABOVE open overlays and "buried" their entries with a counter. Measured in Chrome: Shift+W over About navigated under the still-open dialog, the next Back both undid the navigation and closed About, and the Back after that did nothing; Shift+W on Home with the player open did nothing at all. The counter could also be spent by a different overlay closing first, and callers that could not report whether they pushed were assumed to have. setPage and openChannel now close every open overlay and push only once the history is back in sync (one entry per live overlay, no programmatic back() in flight, no reconcile queued), so nothing is ever buried and the whole mechanism goes: navigateOverOverlays, closeThenNavigate, buryUnder, setPage's boolean, and the three test files that pinned them. The palette closes only itself before running a row, so a theme picked over the player keeps the player. Shift+W on Home with the player open now closes it (owner decision 2026-09-09). The wait is on state, not on "a popstate": the old 250 ms fallback could run with back()s still in flight. An overlay that refuses to close within 1.5 s drops the command instead of being navigated out from under. --- frontend/src/App.keymap.contract.test.ts | 33 --- frontend/src/App.tsx | 16 +- frontend/src/components/ChannelNameLink.tsx | 2 +- .../NavigationProvider.dom.test.tsx | 153 ++++++---- .../src/components/NavigationProvider.tsx | 101 ++++--- frontend/src/components/PlayerModal.tsx | 14 +- .../components/ui/CommandPalette.dom.test.tsx | 2 +- .../ui/CommandPalette.ordering.dom.test.tsx | 16 +- frontend/src/components/ui/CommandPalette.tsx | 10 +- frontend/src/lib/history.burial.dom.test.ts | 43 --- frontend/src/lib/history.dom.test.ts | 267 +++++++++++------- frontend/src/lib/history.ts | 198 ++++++------- frontend/src/lib/nav.dom.test.ts | 40 --- frontend/src/lib/nav.ts | 19 +- 14 files changed, 449 insertions(+), 465 deletions(-) delete mode 100644 frontend/src/App.keymap.contract.test.ts delete mode 100644 frontend/src/lib/history.burial.dom.test.ts delete mode 100644 frontend/src/lib/nav.dom.test.ts diff --git a/frontend/src/App.keymap.contract.test.ts b/frontend/src/App.keymap.contract.test.ts deleted file mode 100644 index ee26db20..00000000 --- a/frontend/src/App.keymap.contract.test.ts +++ /dev/null @@ -1,33 +0,0 @@ -// The global Shift+W wiring, pinned at its call site. -// -// What this is: a source contract, the same idiom as uiBoundary.contract.test.ts — a structural rule -// asserted directly rather than left to prose. What it is NOT: a behaviour test. The behaviour lives -// in history.dom.test.ts and history.burial.dom.test.ts, which drive `navigateOverOverlays` against -// real overlay entries; the end-to-end gesture (Shift+W over an open player, then Back) is a browser -// check, because jsdom reproduces history ordering only as bookkeeping. -// -// It exists because the composition line itself was unpinned: App.tsx is the one module no test -// renders (main.tsx lazy-imports it and nothing else does), globalKeymap.dom.test.tsx drives the -// hook with its OWN handler, and history.dom.test.ts drives the helper with its own navigate — so -// dropping `navigateOverOverlays` from App left all 818 tests green while the user-visible bug came -// straight back (round 2, verified by running that revert). -import { readFileSync } from "node:fs"; -import { fileURLToPath } from "node:url"; - -import { describe, expect, it } from "vitest"; - -const APP = readFileSync(fileURLToPath(new URL("./App.tsx", import.meta.url)), "utf8"); - -describe("App · global keymap wiring", () => { - it("routes the global home shortcut through navigateOverOverlays", () => { - const call = /useGlobalKeymap\(([\s\S]*?)\);/.exec(APP)?.[1]; - // A rename or a move is a legitimate change — but then this test is the thing that has to be - // updated deliberately, which is the whole point of pinning it. - expect( - call, - "App.tsx no longer calls useGlobalKeymap in a shape this test can read", - ).toBeTypeOf("string"); - expect(call).toContain("navigateOverOverlays"); - expect(call).toContain('setPage("home")'); - }); -}); diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index 707e07c6..869d7533 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -37,7 +37,6 @@ import Toaster from "./components/ui/Toaster"; import KeyboardHelp from "./components/ui/KeyboardHelp"; import AppCommandPalette from "./components/AppCommandPalette"; import { useGlobalKeymap } from "./lib/keymap"; -import { navigateOverOverlays } from "./lib/history"; import { StateMessage } from "./components/ui/QueryState"; import ErrorDialog from "./components/ErrorDialog"; import VersionBanner from "./components/VersionBanner"; @@ -142,13 +141,10 @@ export default function App() { // Home ("mit nézzek?"). Bound only once signed in, because that is where the overlays are mounted // (bottom of this tree) and where the modules exist at all — see useGlobalKeymap on why binding it // above the signed-out branch is worse than not binding it. - // Through `navigateOverOverlays`, because a global key can fire while an overlay is open and does - // not know it: the page change unmounts the player (it lives inside the module), whose reconcile - // then reclaimed the entry this push had just made — Shift+W over a playing video closed the - // player and snapped straight back to the feed. `setPage`'s boolean is load-bearing here, not - // decoration: pressed while ALREADY on Home (the default landing page) it navigates nowhere, and - // burying the open player's entry under a push that never happened orphaned it — see `buryUnder`. - useGlobalKeymap(!!meQuery.data, () => navigateOverOverlays(() => setPage("home"))); + // A global key can fire while an overlay is open; `setPage` closes every overlay before it pushes, + // so Shift+W over the player lands on Home with the player closed — also when Home is already the + // page underneath (owner decision 2026-09-09: the key always ends with the answer on screen). + useGlobalKeymap(!!meQuery.data, () => setPage("home")); // Pin this tab to whatever account it first loaded as. Without this, a tab that never picked // an account (it was riding the session's default) would silently swap identity when ANOTHER @@ -279,8 +275,8 @@ export default function App() { label: t("common.accessRequestsReview"), onClick: () => { focusAccessRequestsTab(); - // Over whatever overlay the toast was clicked in — as navigateTo does (see lib/nav). - navigateOverOverlays(() => setPage("users")); + // Clicked inside whatever overlay hosts the toast; setPage closes it before pushing. + setPage("users"); }, }, }); diff --git a/frontend/src/components/ChannelNameLink.tsx b/frontend/src/components/ChannelNameLink.tsx index f567573c..2be89068 100644 --- a/frontend/src/components/ChannelNameLink.tsx +++ b/frontend/src/components/ChannelNameLink.tsx @@ -11,7 +11,7 @@ import { urlWithParams } from "../lib/urlState"; // The href is the same URL `openChannel` pushes — `?chan=` on the current page — so a new tab // lands where the in-app navigation would have. A plain left click is still handled in-app, because // the SPA path does more than navigate (the player has to close first and reclaim its own -// Back-to-close history entry; see closeThenNavigate). +// Back-to-close history entry; see lib/history.closeOverlaysThenNavigate). type Props = Omit, "href" | "onClick" | "title"> & { channelId: string; title: string | null | undefined; diff --git a/frontend/src/components/NavigationProvider.dom.test.tsx b/frontend/src/components/NavigationProvider.dom.test.tsx index dc6e38a8..34606a34 100644 --- a/frontend/src/components/NavigationProvider.dom.test.tsx +++ b/frontend/src/components/NavigationProvider.dom.test.tsx @@ -1,16 +1,14 @@ // @vitest-environment jsdom // -// `setPage`'s no-op contract — the PRODUCER side of the history-burial fix, which nothing rendered -// before this. `setPage` early-returns when the requested page is already current with no channel -// page over it, and reports that by returning `false`; `lib/history.buryUnder` reads the boolean to -// decide whether the overlay entries beneath a navigation are still reclaimable. Get it wrong and -// the round-2 bug is back: Shift+W over the player on Home buries an entry nothing pushed past, and -// the user's next browser Back does nothing. +// The navigation actions against the overlay history stack: `setPage` and `openChannel` never push +// under an open overlay — they close it and push once its Back-to-close entry is reclaimed — and +// `setPage`'s same-page guard, which decides whether anything is pushed at all. // -// The type does NOT fence this. `NavigationActions` pins the SHAPE (`=> boolean`), so deleting one -// `return` fails tsc — but round 3 measured that swapping the two values (early return `true`, tail -// `false`) passes typecheck and all 822 tests. Hence assertions on the value AND on the effects the -// early return skips, which is what the boolean actually stands for. +// The overlay half is what used to live at each call site (Shift+W in App, a toast's "View", the +// palette, the player's channel link) and could be forgotten at any of them; it now lives in the two +// actions, so this file is where it is pinned. The end-to-end gesture — Shift+W over the player, +// then browser Back — is still checked in a real browser: jsdom reproduces history ordering only as +// bookkeeping. import { describe, it, expect, afterEach, beforeEach, vi } from "vitest"; import { renderHook, act, cleanup } from "@testing-library/react"; import type { ReactNode } from "react"; @@ -27,6 +25,7 @@ vi.mock("react-i18next", async (importOriginal) => { }); import { NavigationProvider, useNavigation, useNavigationActions } from "./NavigationProvider"; +import { useBackToClose } from "../lib/history"; const wrapper = ({ children }: { children: ReactNode }) => ( {children} @@ -34,6 +33,25 @@ const wrapper = ({ children }: { children: ReactNode }) => ( const render = () => renderHook(() => ({ ...useNavigation(), ...useNavigationActions() }), { wrapper }); +const settle = () => + act(async () => { + await new Promise((r) => setTimeout(r, 0)); + }); + +/** An open overlay (the player) whose close really unmounts it; its reclaiming back() is stubbed to + * land its popstate at once. */ +async function openOverlay() { + const back = vi.spyOn(window.history, "back").mockImplementation(() => { + window.dispatchEvent(new PopStateEvent("popstate", { state: window.history.state })); + }); + const holder: { unmount?: () => void } = {}; + const onClose = vi.fn(() => holder.unmount?.()); + const layer = renderHook(() => useBackToClose(onClose)); + holder.unmount = layer.unmount; + await settle(); + return { back, onClose }; +} + beforeEach(() => { localStorage.clear(); sessionStorage.setItem("siftlode.activeAccount", "1"); // account-scoped page memory @@ -41,46 +59,29 @@ beforeEach(() => { }); // Not automatic: vitest runs without `globals`, so RTL never registers its own afterEach — leaving a // provider (and its popstate listener) mounted for the next case. Same reason Home.dom.test.tsx does. -afterEach(cleanup); +afterEach(() => { + cleanup(); + vi.restoreAllMocks(); +}); -describe("NavigationProvider · setPage reports whether it navigated", () => { - it("returns true and pushes an entry for a real page change", () => { +describe("NavigationProvider · setPage's same-page guard", () => { + it("pushes an entry for a real page change", () => { const { result } = render(); const push = vi.spyOn(window.history, "pushState"); - try { - let reported: boolean | undefined; - act(() => { - reported = result.current.setPage("plex"); - }); - expect(reported).toBe(true); - expect(push).toHaveBeenCalledTimes(1); - expect(result.current.page).toBe("plex"); - } finally { - push.mockRestore(); - } + act(() => result.current.setPage("plex")); + expect(push).toHaveBeenCalledTimes(1); + expect(result.current.page).toBe("plex"); }); - it("returns false and pushes nothing when it is already that page", () => { + it("pushes nothing when it is already that page", () => { const { result } = render(); - act(() => { - result.current.setPage("plex"); - }); + act(() => result.current.setPage("plex")); const push = vi.spyOn(window.history, "pushState"); - try { - let reported: boolean | undefined; - act(() => { - reported = result.current.setPage("plex"); - }); - // Both halves matter: `false` is what `buryUnder` reads, and "pushed nothing" is what makes - // that answer true. A value-only assertion would pass against an inverted pair. - expect(reported).toBe(false); - expect(push).not.toHaveBeenCalled(); - } finally { - push.mockRestore(); - } + act(() => result.current.setPage("plex")); + expect(push).not.toHaveBeenCalled(); }); - it("returns true even for the current page when a channel page is over it", () => { + it("still navigates to the current page when a channel page is over it", () => { // The nav rail's own module while a channel page is open over it is a real navigation: it closes // the channel page. The `&& !channelViewRef.current` half of the guard is what makes it one. const { result } = render(); @@ -90,22 +91,64 @@ describe("NavigationProvider · setPage reports whether it navigated", () => { // asking for "feed" was just an ordinary page change and deleting the clause left it green. const current = result.current.page; expect(current).toBe("home"); - act(() => { - result.current.openChannel("UC1", "A channel"); - }); + act(() => result.current.openChannel("UC1", "A channel")); expect(result.current.channelView).not.toBeNull(); const push = vi.spyOn(window.history, "pushState"); - try { - let reported: boolean | undefined; - act(() => { - reported = result.current.setPage(current); - }); - expect(reported).toBe(true); - expect(push).toHaveBeenCalledTimes(1); - expect(result.current.channelView).toBeNull(); - } finally { - push.mockRestore(); - } + act(() => result.current.setPage(current)); + expect(push).toHaveBeenCalledTimes(1); + expect(result.current.channelView).toBeNull(); + }); +}); + +describe("NavigationProvider · never under an open overlay", () => { + it("setPage closes the overlay first and pushes only once its entry is reclaimed", async () => { + const { result } = render(); + const { back, onClose } = await openOverlay(); + const push = vi.spyOn(window.history, "pushState"); + + act(() => result.current.setPage("plex")); + expect(onClose).toHaveBeenCalledTimes(1); + expect(push).not.toHaveBeenCalled(); // not while the player's entry is still current + + await settle(); + await settle(); + expect(back).toHaveBeenCalledTimes(1); + expect(push).toHaveBeenCalledTimes(1); + expect(result.current.page).toBe("plex"); + expect(window.history.state).toMatchObject({ sfPage: "plex" }); + }); + + // Shift+W on Home with the player open (owner decision 2026-09-09): nothing to push, but the player + // closes, and its entry is reclaimed rather than orphaned under a push that never happened. + it("setPage to the current page closes the overlay and pushes nothing", async () => { + const { result } = render(); + const { back, onClose } = await openOverlay(); + const push = vi.spyOn(window.history, "pushState"); + + act(() => result.current.setPage(result.current.page)); + await settle(); + await settle(); + expect(onClose).toHaveBeenCalledTimes(1); + expect(back).toHaveBeenCalledTimes(1); + expect(push).not.toHaveBeenCalled(); + }); + + // The player's channel link and the palette's channel rows. + it("openChannel closes the overlay first and pushes the channel once its entry is reclaimed", async () => { + const { result } = render(); + const { back, onClose } = await openOverlay(); + const push = vi.spyOn(window.history, "pushState"); + + act(() => result.current.openChannel("UC1", "A channel")); + expect(onClose).toHaveBeenCalledTimes(1); + expect(push).not.toHaveBeenCalled(); + + await settle(); + await settle(); + expect(back).toHaveBeenCalledTimes(1); + expect(push).toHaveBeenCalledTimes(1); + expect(result.current.channelView).toEqual({ id: "UC1", name: "A channel" }); + expect(window.history.state).toMatchObject({ _chan: "UC1" }); }); }); diff --git a/frontend/src/components/NavigationProvider.tsx b/frontend/src/components/NavigationProvider.tsx index 2186076f..ec2168e5 100644 --- a/frontend/src/components/NavigationProvider.tsx +++ b/frontend/src/components/NavigationProvider.tsx @@ -14,7 +14,7 @@ import { pageTitleKey } from "../lib/pageMeta"; import { setAccountRaw } from "../lib/storage"; import { useAccountRehydrate } from "../lib/useAccountRehydrate"; import { setNavigator } from "../lib/nav"; -import { nextNavId, notifyNav } from "../lib/history"; +import { closeOverlaysThenNavigate, nextNavId, notifyNav } from "../lib/history"; import { channelViewFrom, loadInitialPage, PAGE_KEY, type ChannelView } from "./navigationInit"; // Owns the app's navigation + browser-history state, lifted out of the App god component so pages @@ -37,10 +37,11 @@ export interface NavigationState { } export interface NavigationActions { - /** Switch modules. Returns whether it actually navigated — it is a no-op when the requested page - * is already current with no channel page over it, and a caller that buries overlay history - * entries under the push has to know that (see `lib/history.navigateOverOverlays`). */ - setPage: (next: Page) => boolean; + /** Switch modules. Closes every open overlay first and pushes only once their history entries are + * back (`lib/history.closeOverlaysThenNavigate`), so over an open player the push lands a moment + * after the call; with nothing open it is synchronous. The requested page being already current + * (with no channel page over it) pushes nothing — but the overlays still close. */ + setPage: (next: Page) => void; openChannel: (id: string, name?: string) => void; closeChannel: () => void; enterYtSearch: (q: string) => void; @@ -53,7 +54,7 @@ const StateContext = createContext({ ytSearch: null, }); const ActionsContext = createContext({ - setPage: () => false, + setPage: () => {}, openChannel: () => {}, closeChannel: () => {}, enterYtSearch: () => {}, @@ -118,53 +119,61 @@ export function NavigationProvider({ children }: { children: ReactNode }) { }); const setPage = useCallback( - (next: Page) => { - // A channel page overlays the content column, so a nav click must close it even when the - // underlying page is unchanged (e.g. "Feed" while a channel opened from the feed). - if (next === pageRef.current && !channelViewRef.current) return false; - markDirty(); - setChannelView(null); // leave any open channel page when navigating via the rail - setPageState(next); - setAccountRaw(PAGE_KEY, next); - // Push an in-app history entry so browser/mouse Back steps through pages instead of leaving the - // app. A fresh { sfPage } (no carried-over _yt/_chan markers) so each page starts at its root, and - // the address bar reflects the page (feed is implicit) while clearing the view-specific params - // (chan/v/pl and the plex content keys pscope/psort/pshow/pf) — a page switch resets to that - // module's root. Feed filters (owned by FeedFiltersProvider) are left intact; the plex/channels - // keys re-mirror from PlexProvider / ChannelsProvider on return to their page. - window.history.pushState( - { sfPage: next, sfScroll: nextNavId() }, - "", - urlWithParams({ - page: next === "feed" ? null : next, - chan: null, - v: null, - pl: null, - pscope: null, - psort: null, - pshow: null, - pf: null, - ctab: null, - csort: null, - cf: null, - }), - ); - notifyNav(); - return true; - }, + (next: Page) => + // Never under an open overlay — see the interface doc. The refs are read when the push + // actually happens, which over an open player is a moment after this call. + closeOverlaysThenNavigate(() => { + // A channel page overlays the content column, so a nav click must close it even when the + // underlying page is unchanged (e.g. "Feed" while a channel opened from the feed). + if (next === pageRef.current && !channelViewRef.current) return; + markDirty(); + setChannelView(null); // leave any open channel page when navigating via the rail + setPageState(next); + setAccountRaw(PAGE_KEY, next); + // Push an in-app history entry so browser/mouse Back steps through pages instead of leaving + // the app. A fresh { sfPage } (no carried-over _yt/_chan markers) so each page starts at its + // root, and the address bar reflects the page (feed is implicit) while clearing the + // view-specific params (chan/v/pl and the plex content keys pscope/psort/pshow/pf) — a page + // switch resets to that module's root. Feed filters (owned by FeedFiltersProvider) are left + // intact; the plex/channels keys re-mirror from PlexProvider / ChannelsProvider on return to + // their page. + window.history.pushState( + { sfPage: next, sfScroll: nextNavId() }, + "", + urlWithParams({ + page: next === "feed" ? null : next, + chan: null, + v: null, + pl: null, + pscope: null, + psort: null, + pshow: null, + pf: null, + ctab: null, + csort: null, + cf: null, + }), + ); + notifyNav(); + }), [markDirty], ); // A sub-view (channel / YT search) is an OVERLAY, not a page change, so it does NOT markDirty: a // pre-pin account re-hydrate should still restore the stored page underneath — the re-hydrate's // replaceState spreads the _chan/_yt markers, so the overlay survives on top of the restored page. + // + // Like setPage it never lands under an open overlay: the player's channel link and the palette's + // channel rows call it from inside one. const openChannel = useCallback((id: string, name?: string) => { - setChannelView({ id, name }); - const st = window.history.state || {}; - const url = urlWithParams({ chan: id }); - if (st._chan) - window.history.replaceState({ ...st, _chan: id, _chanName: name ?? null }, "", url); - else window.history.pushState({ ...st, _chan: id, _chanName: name ?? null }, "", url); + closeOverlaysThenNavigate(() => { + setChannelView({ id, name }); + const st = window.history.state || {}; + const url = urlWithParams({ chan: id }); + if (st._chan) + window.history.replaceState({ ...st, _chan: id, _chanName: name ?? null }, "", url); + else window.history.pushState({ ...st, _chan: id, _chanName: name ?? null }, "", url); + }); }, []); const closeChannel = useCallback(() => { diff --git a/frontend/src/components/PlayerModal.tsx b/frontend/src/components/PlayerModal.tsx index 6b71e37f..15ccd35e 100644 --- a/frontend/src/components/PlayerModal.tsx +++ b/frontend/src/components/PlayerModal.tsx @@ -46,7 +46,7 @@ import { import { renderDescription } from "../lib/descriptionLinks"; import { fullscreenElement } from "../lib/fullscreen"; import { dispatch, isActivationTarget, useKeyScope } from "../lib/keymap"; -import { closeThenNavigate, useBackToClose } from "../lib/history"; +import { useBackToClose } from "../lib/history"; import ChannelNameLink from "./ChannelNameLink"; import Toaster, { useOverlayOrder } from "./ui/Toaster"; import { afterRemoval } from "../lib/playerQueue"; @@ -822,13 +822,11 @@ export default function PlayerModal({ channelId={active.channel_id} title={active.channel_title} onOpenInApp={() => { - const cid = active.channel_id; - const cname = active.channel_title ?? undefined; - // Close first, open the channel only once this player's own Back-to-close - // entry has been reclaimed — otherwise that reclaim undoes the push and - // clicking the name merely closes the player. The rule lives in history.ts - // now; the command palette needs exactly the same one. - closeThenNavigate(onClose, () => openChannel(cid, cname)); + // openChannel closes this player first and pushes only once its + // Back-to-close entry is reclaimed — otherwise that reclaim undoes the push + // and clicking the name merely closes the player + // (lib/history.closeOverlaysThenNavigate). + openChannel(active.channel_id, active.channel_title ?? undefined); }} className="font-medium hover:text-accent truncate text-left" /> diff --git a/frontend/src/components/ui/CommandPalette.dom.test.tsx b/frontend/src/components/ui/CommandPalette.dom.test.tsx index c73afbcb..70bd113b 100644 --- a/frontend/src/components/ui/CommandPalette.dom.test.tsx +++ b/frontend/src/components/ui/CommandPalette.dom.test.tsx @@ -21,7 +21,7 @@ beforeAll(() => { afterEach(cleanup); // `choose` closes and waits for the dialog's own Back-to-close history entry to be reclaimed before -// running the command (see closeThenNavigate), so the command lands a tick later — hence the waits. +// running the command (see closeTopOverlayThenRun), so the command lands a tick later — hence the waits. const ran: string[] = []; const groups = (): CommandGroup[] => [ { diff --git a/frontend/src/components/ui/CommandPalette.ordering.dom.test.tsx b/frontend/src/components/ui/CommandPalette.ordering.dom.test.tsx index 81cf801e..3191b6d1 100644 --- a/frontend/src/components/ui/CommandPalette.ordering.dom.test.tsx +++ b/frontend/src/components/ui/CommandPalette.ordering.dom.test.tsx @@ -1,10 +1,9 @@ // @vitest-environment jsdom // // ONE test, ONE file, on purpose. This is the close→reclaim→run ordering contract of the palette -// (see closeThenNavigate), and it ASSERTS on a real useBackToClose entry driven through a real -// programmatic history.back() — which is why it cannot share module state with anything. (It is not -// the only place in the suite where an unstubbed back() fires; history.burial.dom.test.ts is the -// other file that asserts on one, and any test rendering a real Modal can let one go.) +// (see closeTopOverlayThenRun), and it ASSERTS on a real useBackToClose entry driven through a real +// programmatic history.back() — which is why it cannot share module state with anything. (Any test +// rendering a real Modal can let an unstubbed back() go, too.) // // It cannot live in CommandPalette.dom.test.tsx. jsdom gives a test FILE a single session history, // and history.ts's overlay bookkeeping is module-global, so the sibling tests there leave an @@ -50,7 +49,7 @@ function Harness() { ); } -describe("CommandPalette · closeThenNavigate ordering", () => { +describe("CommandPalette · close-then-run ordering", () => { it("closes, and runs the command only once the close has landed", async () => { render(); // Load-bearing: useBackToClose pushes its entry from a coalesced microtask, so a click in the @@ -62,8 +61,9 @@ describe("CommandPalette · closeThenNavigate ordering", () => { fireEvent.click(screen.getByText("TheVR Podcast")); expect(ran).toEqual(["closed"]); - // Inside closeThenNavigate's 250ms no-popstate fallback, so a pass means the RECLAIM ran the - // command — not the safety net that exists for overlays which never got an entry at all. - await waitFor(() => expect(ran).toEqual(["closed", "ch:1"]), { timeout: 200 }); + // Well inside closeTopOverlayThenRun's 1500 ms give-up timer, so a pass means the RECLAIM ran the + // command — not the late path for a back() that never reports. (The margin used to be 200 ms + // under a 250 ms fallback, which made this load-sensitive by construction.) + await waitFor(() => expect(ran).toEqual(["closed", "ch:1"]), { timeout: 1000 }); }); }); diff --git a/frontend/src/components/ui/CommandPalette.tsx b/frontend/src/components/ui/CommandPalette.tsx index d0f00fca..aff1833a 100644 --- a/frontend/src/components/ui/CommandPalette.tsx +++ b/frontend/src/components/ui/CommandPalette.tsx @@ -2,7 +2,7 @@ import { useEffect, useId, useMemo, useRef, useState, type ReactNode } from "rea import { useTranslation } from "react-i18next"; import { Search } from "lucide-react"; import Modal from "./Modal"; -import { closeThenNavigate } from "../../lib/history"; +import { closeTopOverlayThenRun } from "../../lib/history"; // The app's jump-to bar: one text field over every place you can go and every app-wide thing you can // do, opened with Ctrl/Cmd+K. It is the "one-layer operation" half of E10 — reaching a channel three @@ -92,9 +92,11 @@ export default function CommandPalette({ if (!open) return null; - // Close, wait for this dialog's own Back-to-close entry to be reclaimed, THEN run. Most commands - // navigate, and a pushState issued before that reclaim is undone by it — see closeThenNavigate. - const choose = (item: CommandItem) => closeThenNavigate(onClose, item.run); + // Close, wait for this dialog's own Back-to-close entry to be reclaimed, THEN run: most commands + // navigate, and a pushState issued before that reclaim is undone by it. Only THIS dialog closes — + // a command that navigates closes whatever is underneath itself, one that does not (a theme) must + // leave it standing. See closeTopOverlayThenRun. + const choose = (item: CommandItem) => closeTopOverlayThenRun(onClose, item.run); // These live on the input rather than in the shared keymap: they are the field's own editing keys // (like a and re-render the row as a , ); const btn = screen.getByRole("button", { name: "Save" }) as HTMLButtonElement; - expect(btn.disabled).toBe(true); expect(btn.getAttribute("aria-busy")).toBe("true"); + expect(btn.getAttribute("aria-disabled")).toBe("true"); // the leading icon is swapped for a spinner expect(btn.querySelector("[data-testid=ic]")).toBeNull(); fireEvent.click(btn); expect(onClick).not.toHaveBeenCalled(); }); - it("loading disables even when an explicit disabled={false} is passed", () => { + // E11 S2, measured in Chrome: a focused button that becomes natively disabled drops the focus to + // and never gets it back, so every "Something else" press threw a keyboard user out of Home. + // jsdom has no focus fixup, so what is pinned here is the cause — native `disabled` stays off. + it("loading keeps the button natively enabled, so it keeps the focus", () => { + render(); + const btn = screen.getByRole("button", { name: "Reroll" }) as HTMLButtonElement; + expect(btn.disabled).toBe(false); + }); + + it("loading blocks clicks even when an explicit disabled={false} is passed", () => { // Regression: `disabled ?? loading` let an explicit boolean disabled shadow loading, so a // call-site with `disabled={!valid} loading={pending}` (valid → disabled=false) stayed - // clickable mid-mutation and double-submitted. The contract is: loading always disables. + // clickable mid-mutation and double-submitted. The contract is: loading always blocks. const onClick = vi.fn(); render( , ); - const btn = screen.getByRole("button", { name: "Create" }) as HTMLButtonElement; - expect(btn.disabled).toBe(true); - fireEvent.click(btn); + fireEvent.click(screen.getByRole("button", { name: "Create" })); expect(onClick).not.toHaveBeenCalled(); }); + it("a busy submit button cancels the form's submission", () => { + // Enter in a field submits through a click on the default button; the guard cancels that click. + const onSubmit = vi.fn((e: { preventDefault: () => void }) => e.preventDefault()); + render( +
+ +
, + ); + fireEvent.click(screen.getByRole("button", { name: "Send" })); + expect(onSubmit).not.toHaveBeenCalled(); + }); + + it("a non-busy button still calls onClick", () => { + const onClick = vi.fn(); + render(); + fireEvent.click(screen.getByRole("button", { name: "Go" })); + expect(onClick).toHaveBeenCalledTimes(1); + }); + it("a link gets no button padding, and an explicit size still wins", () => { // The size default is variant-dependent (link → "none"). Locking it because the failure is // silent: a link that quietly inherits px-3 py-2 reads as a button, and nothing errors. diff --git a/frontend/src/components/ui/Button.tsx b/frontend/src/components/ui/Button.tsx index 4db42540..98630d25 100644 --- a/frontend/src/components/ui/Button.tsx +++ b/frontend/src/components/ui/Button.tsx @@ -44,28 +44,32 @@ const VARIANTS: Record = { // and the ~100 hand-rolled ones had drifted into two different looks. That split IS the // inconsistency this epic exists to remove; the primitive follows the majority, not the reverse. outline: - "font-medium border border-border text-muted enabled:hover:text-accent enabled:hover:border-accent", - "outline-accent": "font-medium border border-accent/40 text-accent enabled:hover:bg-accent/10", + "font-medium border border-border text-muted enabled:not-aria-disabled:hover:text-accent enabled:not-aria-disabled:hover:border-accent", + "outline-accent": + "font-medium border border-accent/40 text-accent enabled:not-aria-disabled:hover:bg-accent/10", // Neutral until you reach for it. NOT `danger-soft`, which is already red at rest — the difference // is whether the button should advertise the destruction before the pointer arrives. "outline-danger": - "font-medium border border-border text-muted enabled:hover:text-danger enabled:hover:border-danger/40", - "outline-warn": "font-medium border border-warn/40 text-warn enabled:hover:bg-warn/10", - ghost: "font-medium text-muted enabled:hover:text-fg enabled:hover:bg-card", - glass: "font-medium glass-card glass-hover enabled:hover:text-accent", + "font-medium border border-border text-muted enabled:not-aria-disabled:hover:text-danger enabled:not-aria-disabled:hover:border-danger/40", + "outline-warn": + "font-medium border border-warn/40 text-warn enabled:not-aria-disabled:hover:bg-warn/10", + ghost: + "font-medium text-muted enabled:not-aria-disabled:hover:text-fg enabled:not-aria-disabled:hover:bg-card", + glass: "font-medium glass-card glass-hover enabled:not-aria-disabled:hover:text-accent", "danger-soft": - "font-medium border border-danger/60 text-danger enabled:hover:bg-danger/10 enabled:hover:border-danger", + "font-medium border border-danger/60 text-danger enabled:not-aria-disabled:hover:bg-danger/10 enabled:not-aria-disabled:hover:border-danger", // No font-weight here on purpose: BASE already sets font-medium, and a second font-* utility would // be a coin-flip (both are single-class utilities of equal specificity, and without tailwind-merge // the winner is decided by the generated stylesheet order, not by the class list order). - link: "font-semibold text-accent enabled:hover:underline", + link: "font-semibold text-accent enabled:not-aria-disabled:hover:underline", }; -// Why every hover above is `enabled:hover:` — Tailwind's `:not(:disabled)`. A disabled button that -// still lights up on hover advertises a click that does nothing, and the ONE migrated call site that -// had guarded its own hover (TagManager's Add) silently lost the guard when it moved onto this -// primitive (E10 review round 1). The contract belongs here, not in ~160 call sites: BASE dims a -// disabled button, and the variant must not paint over that dimming. +// Why every hover above is `enabled:not-aria-disabled:hover:` — neither natively disabled nor busy. A +// disabled button that still lights up on hover advertises a click that does nothing, and the ONE +// migrated call site that had guarded its own hover (TagManager's Add) silently lost the guard when it +// moved onto this primitive (E10 review round 1). `not-aria-disabled` is the `loading` half: a busy +// button stays natively enabled (see `loading` below) and must not light up either. The contract +// belongs here, not in ~160 call sites: BASE dims both, and the variant must not paint over it. // The geometry ladder, and the whole of it — a call site picks a rung, it does not invent one. The // rungs were derived from what the app actually used (counted during E10 S1b): `px-3 py-2 text-sm` @@ -86,7 +90,7 @@ const SIZES: Record = { // link looks like everywhere in this app) and the action buttons want medium. Two font-* utilities on // one element is a coin flip without tailwind-merge, so exactly one has to emit it — see the test. const BASE = - "inline-flex items-center justify-center transition disabled:opacity-50 disabled:cursor-not-allowed"; + "inline-flex items-center justify-center transition disabled:opacity-50 disabled:cursor-not-allowed aria-disabled:opacity-50 aria-disabled:cursor-not-allowed"; interface ButtonOwnProps extends ButtonHTMLAttributes { variant?: ButtonVariant; @@ -98,7 +102,8 @@ interface ButtonOwnProps extends ButtonHTMLAttributes { iconOnly?: boolean; /** Accessible name + tooltip — required for `iconOnly`, optional otherwise. */ label?: string; - /** Disable + swap the icon for a spinner + mark aria-busy. */ + /** Busy: swap the icon for a spinner, mark aria-busy + aria-disabled, and swallow activation — but + * NOT native `disabled`, which would throw the focus out of the page (see the render below). */ loading?: boolean; fullWidth?: boolean; } @@ -131,6 +136,7 @@ const ButtonImpl = forwardRef(function Button children, disabled, type, + onClick, ...rest }, ref, @@ -149,11 +155,23 @@ const ButtonImpl = forwardRef(function Button + + ); +} + +describe("useDialogChrome · the page scroll lock", () => { + // Each dialog used to save and restore `overflow` itself, so the one mounted SECOND but unmounted + // LAST put back the "hidden" it had saved — a page left locked with no dialog open. + it("unlocks only when the last dialog closes, in whatever order they close", () => { + document.body.style.overflow = ""; + const player = render(); + const modal = render(); + expect(document.body.style.overflow).toBe("hidden"); + + player.unmount(); // the one underneath goes first + expect(document.body.style.overflow).toBe("hidden"); // the other is still open + + modal.unmount(); + expect(document.body.style.overflow).toBe(""); + }); +}); + +describe("useDialogChrome · initial focus", () => { + it("lands on the first control by default", () => { + // jsdom has no layout, so the visibility filter (offsetParent) would see nothing focusable. + const spy = vi + .spyOn(HTMLElement.prototype, "offsetParent", "get") + .mockReturnValue(document.body); + try { + const { getByText } = render(); + expect(document.activeElement).toBe(getByText("first")); + } finally { + spy.mockRestore(); + } + }); + + it("lands on the scope itself with focusScope", () => { + // Same layout stub as above: without it jsdom sees no focusable child, so the scope would get + // the focus with or without the option and this would pin nothing. + const spy = vi + .spyOn(HTMLElement.prototype, "offsetParent", "get") + .mockReturnValue(document.body); + try { + const { getByTestId } = render(); + expect(document.activeElement).toBe(getByTestId("scope")); + } finally { + spy.mockRestore(); + } + }); +}); diff --git a/frontend/src/components/ui/dialogFocus.ts b/frontend/src/components/ui/dialogFocus.ts index 321ef0be..56d32440 100644 --- a/frontend/src/components/ui/dialogFocus.ts +++ b/frontend/src/components/ui/dialogFocus.ts @@ -46,13 +46,36 @@ export function trapTab( } } +// ONE body scroll lock for every open dialog, counted. Each dialog used to save and restore +// `overflow` on its own, so two unmounting out of order (the player's cleanup running before a Modal +// opened above it) put back a stale "hidden". Invisible today — the app scrolls inside
, not +// the document (measured, E11 S2) — but a count cannot be restored out of order, and PlayerModal's +// second copy of the lock is gone. +let scrollLocks = 0; +let unlockedOverflow = ""; +function lockBodyScroll(): () => void { + if (scrollLocks++ === 0) { + unlockedOverflow = document.body.style.overflow; + document.body.style.overflow = "hidden"; + } + let released = false; + return () => { + if (released) return; + released = true; + if (--scrollLocks === 0) document.body.style.overflow = unlockedOverflow; + }; +} + /** The rest of the contract: lock the page behind, open focused, hand the focus back on close. * `scopeRef` is the panel the focus should land in; `ownerRef`, when the dialog is bigger than its * panel, is that whole dialog — it must be the SAME element `trapTab` is scoped to, or the two - * disagree about what counts as "inside" (see `heldRef`). */ + * disagree about what counts as "inside" (see `heldRef`). `focusScope` opens the dialog focused on + * the scope element itself rather than its first control: the player keeps the focus on its card, so + * Space and the arrows reach its shortcuts instead of activating a button. */ export function useDialogChrome( scopeRef: RefObject, ownerRef?: RefObject, + { focusScope = false }: { focusScope?: boolean } = {}, ): void { // Capture the trigger to restore focus to, during THIS render — i.e. before commit runs the // dialog's autoFocus (or our own initial focus) and moves activeElement into the dialog. Reading it @@ -79,18 +102,17 @@ export function useDialogChrome( // this hook existed the restore was unconditional (deep-review round 6). const heldRef = useRef(true); useEffect(() => { - const prev = document.body.style.overflow; - document.body.style.overflow = "hidden"; + const unlockScroll = lockBodyScroll(); // Initial focus: unless an autoFocus child already placed it inside (e.g. a rename input), move - // focus to where this dialog was focused before — or, the first time, to the first control, so - // the dialog opens focused rather than stranded on the background. + // focus to where this dialog was focused before — or, the first time, to the first control (or + // the scope itself, with `focusScope`), so the dialog opens focused rather than stranded on the + // background. const scope = scopeRef.current; if (scope && !scope.contains(document.activeElement)) { const back = focusedRef.current; const target = (back?.isConnected && scope.contains(back) ? back : null) ?? - focusablesIn(scope)[0] ?? - scope; + (focusScope ? scope : (focusablesIn(scope)[0] ?? scope)); target.focus(); } const onFocusIn = () => { @@ -114,7 +136,7 @@ export function useDialogChrome( document.addEventListener("focusin", onFocusIn); return () => { document.removeEventListener("focusin", onFocusIn); - document.body.style.overflow = prev; + unlockScroll(); // Restore focus to the trigger, if it's still in the document and focusable — and only if the // dialog still held the focus. Once the user has moved it elsewhere, taking it back is the // rudest thing a closing dialog can do. @@ -140,5 +162,5 @@ export function useDialogChrome( if (heldRef.current && !standingAbove && restoreTo && restoreTo.isConnected) restoreTo.focus(); }; - }, [restoreTo, scopeRef, ownerRef]); + }, [restoreTo, scopeRef, ownerRef, focusScope]); } diff --git a/frontend/src/lib/useHoverFocusWithin.dom.test.tsx b/frontend/src/lib/useHoverFocusWithin.dom.test.tsx new file mode 100644 index 00000000..86dddb3c --- /dev/null +++ b/frontend/src/lib/useHoverFocusWithin.dom.test.tsx @@ -0,0 +1,53 @@ +// @vitest-environment jsdom +// +// The focus half of the side panels' peek-open model. What is pinned: a dialog opened from inside +// the panel (rendered at the app root, so outside it in both the DOM and the React tree) does not +// collapse the panel — collapsing unmounted the row the dialog hands the focus back to, stranding a +// keyboard user on after Cancel (measured in Chrome, E11 S2). +import { afterEach, describe, expect, it } from "vitest"; +import { act, cleanup, fireEvent, render, screen } from "@testing-library/react"; +import { useHoverFocusWithin } from "./useHoverFocusWithin"; + +afterEach(cleanup); + +function Panel() { + const peek = useHoverFocusWithin(); + return ( + <> +
+ + {peek.active && expanded} +
+
+ +
+ + + ); +} + +const moveFocus = (from: HTMLElement, to: HTMLElement) => + act(() => { + fireEvent.blur(from, { relatedTarget: to }); + to.focus(); + }); + +describe("useHoverFocusWithin · focus", () => { + it("stays open while the focus is in a dialog opened over the page", () => { + render(); + const row = screen.getByText("row"); + act(() => row.focus()); + expect(screen.queryByText("expanded")).not.toBeNull(); + + moveFocus(row, screen.getByText("cancel")); + expect(screen.queryByText("expanded")).not.toBeNull(); + }); + + it("still closes when the focus leaves for the page itself", () => { + render(); + const row = screen.getByText("row"); + act(() => row.focus()); + moveFocus(row, screen.getByText("elsewhere")); + expect(screen.queryByText("expanded")).toBeNull(); + }); +}); diff --git a/frontend/src/lib/useHoverFocusWithin.ts b/frontend/src/lib/useHoverFocusWithin.ts index 3f27423d..6ecd49f0 100644 --- a/frontend/src/lib/useHoverFocusWithin.ts +++ b/frontend/src/lib/useHoverFocusWithin.ts @@ -58,7 +58,16 @@ export function useHoverFocusWithin() { const focusHandlers = { onFocus: () => setFocused(true), onBlur: (e: FocusEvent) => { - if (!e.currentTarget.contains(e.relatedTarget as Node)) setFocused(false); + const to = e.relatedTarget as Element | null; + if (e.currentTarget.contains(to)) return; + // A dialog opened FROM inside the panel (the rail row's "Delete?" confirm) renders at the app + // root, so neither the DOM nor the React tree sees it as inside — and collapsing the panel under + // it unmounted the very row the dialog hands the focus back to, leaving a keyboard user on + // after Cancel (measured in Chrome, E11 S2). Hold while the focus is in a dialog: when it + // closes, the focus either comes back in here (onFocus) or lands nowhere, which the + // document-level net below clears. + if (to?.closest('[aria-modal="true"]')) return; + setFocused(false); }, }; From 8a50538275ff2afd329516eb871565bd54113a5a Mon Sep 17 00:00:00 2001 From: npeter83 Date: Sun, 13 Sep 2026 01:45:28 +0200 Subject: [PATCH 03/16] fix(ui): a write that fails says what failed and why, wherever it fails `req` opens its blocking dialog for 400/409/422/5xx only. A 403/404/429 was left to the caller and a 502-504 or lost connection got just the self-clearing connection notice, so 34 mutations without a reporting onError and 15 direct api writes with a bare or missing catch failed in silence: a hidden channel that snapped back, a share link that never revoked, a confirmed account delete that ended with nothing said. - lib/queryClient: one MutationCache reporter. react-query awaits it BEFORE the mutation's own onError, so it waits a task and skips a failure the site already reported; the toast title comes from `meta.failTitle`, `meta.errorShownInline` opts out. - notifyActionError: title names WHAT failed, the line under it says WHY (`failureReason`: a structured localized detail, else the status). It marks the error reported (`markReported`), so nothing reports one failure twice; `notifyKnownFailure` for a site's own specific sentence. - Every silent site named (45 new HU/EN titles, 4 reasons). Stale "the global dialog shows it" comments corrected on the job actions, channel block, Plex sync, cookies, download enqueue, account delete and usePref. - Channels passed the tag hook's channel id to the reporter as the error; the tag and priority hooks now report themselves, with the real error. - A contract test keeps every useMutation naming its failure; CLAUDE.md gains the pattern row. - history.dom.test: the "navigates only once the entries are back" assertion raced an async act's macrotask flush (1 in 5 runs); it now asserts from a later popstate listener. --- CLAUDE.md | 1 + frontend/src/App.tsx | 11 +- frontend/src/components/AccountPanel.tsx | 14 +-- .../src/components/AdminUsers.dom.test.tsx | 3 +- frontend/src/components/AdminUsers.tsx | 31 ++---- frontend/src/components/ChannelPage.tsx | 37 ++++--- frontend/src/components/Channels.tsx | 8 +- frontend/src/components/DownloadCenter.tsx | 7 +- frontend/src/components/DownloadDialog.tsx | 8 +- frontend/src/components/Feed.tsx | 32 ++++-- .../src/components/NotificationsPanel.tsx | 24 +++- frontend/src/components/OnboardingWizard.tsx | 2 + .../src/components/Playlists.dom.test.tsx | 2 +- frontend/src/components/Playlists.tsx | 18 +-- frontend/src/components/PlaylistsRail.tsx | 1 + frontend/src/components/PlexSidebar.tsx | 12 +- frontend/src/components/ProfileEditor.tsx | 2 + frontend/src/components/Scheduler.tsx | 4 + frontend/src/components/SettingsPanel.tsx | 18 ++- frontend/src/components/ShareDialog.tsx | 4 + frontend/src/components/Stats.tsx | 5 + frontend/src/components/SyncStatus.tsx | 6 + frontend/src/components/TagManager.tsx | 1 + .../WriteFailureReporting.dom.test.tsx | 24 +++- .../src/components/accountPanel.dom.test.tsx | 6 +- frontend/src/components/downloadAdmin.tsx | 2 + frontend/src/components/downloadModals.tsx | 1 + .../usePlaylistActions.dom.test.tsx | 9 +- frontend/src/i18n/locales/en/errors.json | 52 +++++++++ frontend/src/i18n/locales/hu/errors.json | 52 +++++++++ frontend/src/lib/actionErrors.test.ts | 75 ++++++++++++- frontend/src/lib/actionErrors.ts | 68 +++++++++--- frontend/src/lib/api/core.ts | 8 ++ .../src/lib/channelWriteFailures.dom.test.tsx | 83 ++++++++++++++ frontend/src/lib/history.dom.test.ts | 20 +++- .../src/lib/mutationFailures.contract.test.ts | 77 +++++++++++++ frontend/src/lib/queryClient.dom.test.ts | 104 ++++++++++++++++++ frontend/src/lib/queryClient.ts | 42 +++++++ frontend/src/lib/useChannelPriority.ts | 9 +- frontend/src/lib/useChannelTagActions.ts | 17 ++- frontend/src/lib/usePref.dom.test.ts | 7 +- .../src/lib/usePref.saveFailure.dom.test.ts | 51 +++++++++ frontend/src/lib/usePref.ts | 16 ++- frontend/src/lib/youtubeErrors.ts | 2 + frontend/src/main.tsx | 7 +- 45 files changed, 851 insertions(+), 132 deletions(-) create mode 100644 frontend/src/lib/channelWriteFailures.dom.test.tsx create mode 100644 frontend/src/lib/mutationFailures.contract.test.ts create mode 100644 frontend/src/lib/queryClient.dom.test.ts create mode 100644 frontend/src/lib/queryClient.ts create mode 100644 frontend/src/lib/usePref.saveFailure.dom.test.ts diff --git a/CLAUDE.md b/CLAUDE.md index caca1a13..af4f1665 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -91,6 +91,7 @@ CQRS, a repository/DI framework, a router library, a generic CRUD-router factory | FE keyboard shortcut | a row in `lib/keymap.ts` + `dispatch()` | a private `keydown` listener with its own key literals | | FE control | the chip/segmented primitives (`Select` only for >6 options) | a native `` | | FE loading/empty/error | `QueryState` / `StateMessage` | ad-hoc conditionals | | FE failed write | `meta.failTitle`, else `notifyActionError(err, title)` | a bare `catch {}` or `notify()` with no error attached | diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index 75061a50..cf0401f5 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -23,7 +23,7 @@ import { useNavigation, useNavigationActions } from "./components/NavigationProv import { useFeedFilters, useFeedFiltersActions } from "./components/FeedFiltersProvider"; import { useChannelLayout } from "./components/ChannelsProvider"; import { useFeedView, useFeedViewActions } from "./components/FeedViewProvider"; -import { moduleDef } from "./lib/modules"; +import { moduleDef, moduleOrder } from "./lib/modules"; import { PAGE_CONTENT, PAGE_RAIL } from "./components/moduleRegistry"; // Persistent shell — always mounted, so eagerly imported. import Welcome from "./components/Welcome"; @@ -147,7 +147,13 @@ export default function App() { // A global key can fire while an overlay is open; `setPage` closes every overlay before it pushes, // so Shift+W over the player lands on Home with the player closed — also when Home is already the // page underneath (owner decision 2026-09-09: the key always ends with the answer on screen). - useGlobalKeymap(!!meQuery.data, () => setPage("home")); + // The `page` scope rides the same binding: the `g` chords jump to a module, but only to one this + // account can reach (the nav rail's own list, plus Settings, which is not in the rail's order). + useGlobalKeymap( + !!meQuery.data, + setPage, + meQuery.data ? [...moduleOrder(meQuery.data), "settings"] : [], + ); // Pin this tab to whatever account it first loaded as. Without this, a tab that never picked // an account (it was riding the session's default) would silently swap identity when ANOTHER diff --git a/frontend/src/components/AboutCell.tsx b/frontend/src/components/AboutCell.tsx index 86d9ddf4..3e27a9dc 100644 --- a/frontend/src/components/AboutCell.tsx +++ b/frontend/src/components/AboutCell.tsx @@ -49,7 +49,7 @@ export default function AboutCell({ text, channelId }: { text: string | null; ch ref={pop.refs.setReference} {...pop.getReferenceProps()} tabIndex={0} - className="block max-w-88 truncate text-muted cursor-help rounded-sm outline-hidden focus-visible:outline-2 focus-visible:outline-accent/60 focus-visible:outline-offset-2" + className="block max-w-88 truncate text-muted cursor-help rounded-sm outline-hidden focus-visible:outline-2 focus-visible:outline-accent focus-visible:outline-offset-2" > {preview} diff --git a/frontend/src/components/PlexSearchBox.dom.test.tsx b/frontend/src/components/PlexSearchBox.dom.test.tsx index e7a3163a..0a60e702 100644 --- a/frontend/src/components/PlexSearchBox.dom.test.tsx +++ b/frontend/src/components/PlexSearchBox.dom.test.tsx @@ -180,6 +180,25 @@ describe("PlexSearchBox — typeahead", () => { ); }); + // E11 S4: SearchBar's Escape hands the keyboard back to the page (blur). With the panel open, the + // first Escape belongs to the panel: it must close it and leave the caret in the field; only the + // next Escape leaves. + it("spends the first Escape on the panel and only the second on leaving the field", async () => { + plexState.q = "matrix"; + plexLibrary.mockResolvedValue(result([card("m1", "The Matrix")])); + renderBox(); + const input = screen.getByRole("combobox"); + input.focus(); + await screen.findByRole("listbox"); + + fireEvent.keyDown(input, { key: "Escape" }); + await waitFor(() => expect(screen.queryByRole("listbox")).toBeNull()); + expect(document.activeElement).toBe(input); + + fireEvent.keyDown(input, { key: "Escape" }); + expect(document.activeElement).not.toBe(input); + }); + it("marks the input as a combobox and toggles aria-expanded with the panel", async () => { plexState.q = "matrix"; plexLibrary.mockResolvedValue(result([card("m1", "The Matrix")])); diff --git a/frontend/src/components/PlexSearchBox.tsx b/frontend/src/components/PlexSearchBox.tsx index 03b3dadd..ea1a1d32 100644 --- a/frontend/src/components/PlexSearchBox.tsx +++ b/frontend/src/components/PlexSearchBox.tsx @@ -96,6 +96,7 @@ export default function PlexSearchBox() { e.preventDefault(); // suppress SearchBar's Go; open the highlighted title instead pick(hi); } else if (e.key === "Escape") { + e.preventDefault(); // close the panel only; SearchBar's Escape-to-blur waits for the next press setDismissed(true); setHi(-1); // don't leave a stale highlight to flash on the next reopen } diff --git a/frontend/src/components/SearchBar.keys.dom.test.tsx b/frontend/src/components/SearchBar.keys.dom.test.tsx new file mode 100644 index 00000000..05735421 --- /dev/null +++ b/frontend/src/components/SearchBar.keys.dom.test.tsx @@ -0,0 +1,119 @@ +// @vitest-environment jsdom +// +// E11 S4: `s` puts the caret in the module's search box from anywhere on the page, and Escape hands +// the keyboard back to the page — the way in needs a way out, or `j`/`k` end up typed into the query. +import { afterEach, describe, expect, it, vi } from "vitest"; +import { act, cleanup, render, screen } from "@testing-library/react"; +import SearchBar from "./SearchBar"; +import KeyboardHelp from "./ui/KeyboardHelp"; +import { closeKeyboardHelp, toggleKeyboardHelp } from "../lib/keymap"; +import { pushLayer, removeLayer } from "../lib/layerStack"; + +vi.mock("react-i18next", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, useTranslation: () => ({ t: (key: string) => key }) }; +}); + +afterEach(() => { + cleanup(); + closeKeyboardHelp(); +}); + +function press(init: KeyboardEventInit, on?: Element | null): KeyboardEvent { + const e = new KeyboardEvent("keydown", { bubbles: true, cancelable: true, ...init }); + act(() => { + (on ?? document.activeElement ?? document.body).dispatchEvent(e); + }); + return e; +} + +const field = () => screen.getByPlaceholderText("Search…") as HTMLInputElement; + +describe("SearchBar keys", () => { + it("focuses the box on s, selects the old query, and keeps the s out of it", () => { + render( {}} placeholder="Search…" />); + const e = press({ key: "s" }); + expect(document.activeElement).toBe(field()); + expect(e.defaultPrevented).toBe(true); // the `s` is not typed into the box it just focused + expect([field().selectionStart, field().selectionEnd]).toEqual([0, "old query".length]); + }); + + it("leaves an s typed into another field alone", () => { + render( + <> + + {}} placeholder="Search…" /> + , + ); + const other = screen.getByTestId("other"); + act(() => other.focus()); + expect(press({ key: "s" }).defaultPrevented).toBe(false); + expect(document.activeElement).toBe(other); + }); + + it("stays quiet under a dialog", () => { + render( {}} placeholder="Search…" />); + const id = pushLayer(); + press({ key: "s" }); + removeLayer(id); + expect(document.activeElement).not.toBe(field()); + }); + + it("gives the keyboard back to the page on Escape", () => { + render( {}} placeholder="Search…" />); + press({ key: "s" }); + press({ key: "Escape" }); + expect(document.activeElement).toBe(document.body); + }); + + // Plex's typeahead uses its first Escape to close the result panel; the field keeps focus for that. + it("lets a caller keep its own Escape", () => { + render( + {}} + placeholder="Search…" + onKeyDown={(e) => e.key === "Escape" && e.preventDefault()} + />, + ); + press({ key: "s" }); + press({ key: "Escape" }); + expect(document.activeElement).toBe(field()); + }); + + // Through the real hook: the sheet offers `s` only where a search box is mounted. + it("is listed in the help sheet while a search box is mounted", () => { + const { unmount } = render( + <> + {}} placeholder="Search…" /> + + , + ); + act(() => toggleKeyboardHelp()); + expect(screen.getAllByRole("definition").map((d) => d.textContent)).toContain("keys.search"); + unmount(); + closeKeyboardHelp(); + + render(); + act(() => toggleKeyboardHelp()); + expect(screen.getAllByRole("definition").map((d) => d.textContent)).not.toContain( + "keys.search", + ); + }); + + // Opened over a player, the page keys are quiet — the sheet must not list them there. + it("is left off a sheet opened over a keyboard-owning layer", () => { + render( + <> + {}} placeholder="Search…" /> + + , + ); + const id = pushLayer(); + act(() => toggleKeyboardHelp()); + expect(screen.queryAllByRole("definition").map((d) => d.textContent)).not.toContain( + "keys.search", + ); + removeLayer(id); + }); +}); diff --git a/frontend/src/components/SearchBar.tsx b/frontend/src/components/SearchBar.tsx index dd8b7a0d..11dc408c 100644 --- a/frontend/src/components/SearchBar.tsx +++ b/frontend/src/components/SearchBar.tsx @@ -1,4 +1,5 @@ import { + useRef, type FocusEventHandler, type InputHTMLAttributes, type KeyboardEventHandler, @@ -6,6 +7,7 @@ import { } from "react"; import { useTranslation } from "react-i18next"; import { Search, X } from "lucide-react"; +import { useSearchFocusKey } from "../lib/keymap"; // The floating search pill in the top header. One component for every module that searches // (feed, Plex, channels, playlists): a glass rounded box with a search icon, the input, a @@ -52,6 +54,9 @@ export default function SearchBar({ inputProps?: InputHTMLAttributes; }) { const { t } = useTranslation(); + // `s` puts the caret here from anywhere on the page (E11 S4) — the header mounts exactly one box. + const inputRef = useRef(null); + useSearchFocusKey(inputRef); const fire = () => { if (onGo && !goDisabled) onGo(); }; @@ -67,13 +72,19 @@ export default function SearchBar({ {leading ?? } onChange(e.target.value)} onFocus={onFocus} onBlur={onBlur} onKeyDown={(e) => { onKeyDown?.(e); - if (e.key === "Enter" && !e.defaultPrevented) fire(); + if (e.defaultPrevented) return; + if (e.key === "Enter") fire(); + // Escape hands the keyboard back to the page, where `j`/`k` and the `g` chords live — `s` + // got you in, so there has to be a key that gets you out. The query stays as typed. A caller + // that uses Escape itself (Plex's typeahead closing its panel) preventDefaults first. + else if (e.key === "Escape") e.currentTarget.blur(); }} placeholder={placeholder} className="flex-1 min-w-0 bg-transparent outline-hidden text-sm placeholder:text-muted" diff --git a/frontend/src/components/VideoCard.nav.dom.test.tsx b/frontend/src/components/VideoCard.nav.dom.test.tsx new file mode 100644 index 00000000..158b6979 --- /dev/null +++ b/frontend/src/components/VideoCard.nav.dom.test.tsx @@ -0,0 +1,89 @@ +// @vitest-environment jsdom +// +// E11 S4: the video card's half of keyboard navigation (`lib/listNav`). Every layout's ROOT carries the +// list index — that is the element `j`/`k` focus and ring — and the title link is the card's `open` +// element, so Enter on a selected card runs the very click a mouse would: the in-app player. +import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; +import { cleanup, render, screen } from "@testing-library/react"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; + +vi.mock("react-i18next", async (importOriginal) => ({ + ...(await importOriginal()), + useTranslation: () => ({ t: (key: string) => key, i18n: { language: "en" } }), +})); +vi.mock("./NavigationProvider", () => ({ + useNavigationActions: () => ({ openChannel: vi.fn() }), +})); + +import VideoCard from "./VideoCard"; +import type { Video } from "../lib/api"; +import type { FeedView } from "../lib/feedView"; + +beforeAll(() => { + class Stub { + observe() {} + unobserve() {} + disconnect() {} + takeRecords() { + return []; + } + } + globalThis.ResizeObserver ??= Stub as unknown as typeof ResizeObserver; + globalThis.IntersectionObserver ??= Stub as unknown as typeof IntersectionObserver; +}); + +const video = { + id: "abc123", + title: "Cicada 3301", + channel_id: "UCsXVk37bltHxD1rDPwtNM8Q", + channel_title: "LEMMiNO", + status: "new", + saved: false, + position_seconds: 0, + duration_seconds: 1200, + published_at: "2026-01-01T00:00:00Z", + watch_url: "https://www.youtube.com/watch?v=abc123", + thumbnail_url: "", +} as unknown as Video; + +beforeEach(cleanup); + +function renderCard(view: FeedView, navIndex?: number, onOpen = vi.fn()) { + render( + + {}} + onToggleSave={() => {}} + onOpen={onOpen} + /> + , + ); + return onOpen; +} + +// One per layout FAMILY: the stacked card, the thumbnailed row, the one-line row — three roots. +const VIEWS: FeedView[] = ["cards", "rows", "rowsCompact"]; + +describe("VideoCard — keyboard navigation hooks", () => { + for (const view of VIEWS) { + it(`puts the list index on the ${view} root and opens in-app from the title`, () => { + const onOpen = renderCard(view, 7); + const root = screen.getByTestId("video-card"); + expect(root.getAttribute("data-nav-index")).toBe("7"); + const open = root.querySelectorAll("[data-nav-open]"); + expect(open).toHaveLength(1); + expect(open[0]!.textContent).toBe("Cicada 3301"); + (open[0] as HTMLElement).click(); + expect(onOpen).toHaveBeenCalledWith(video); + }); + } + + it("carries no index outside a navigable list (Home's rails)", () => { + renderCard("cards"); + expect(screen.getByTestId("video-card").hasAttribute("data-nav-index")).toBe(false); + }); +}); diff --git a/frontend/src/components/VideoCard.tsx b/frontend/src/components/VideoCard.tsx index 6c657c32..b24f3d89 100644 --- a/frontend/src/components/VideoCard.tsx +++ b/frontend/src/components/VideoCard.tsx @@ -18,6 +18,7 @@ import Avatar from "./ui/Avatar"; import AddToPlaylist from "./AddToPlaylist"; import DownloadButton from "./DownloadButton"; import VideoInfoCard from "./VideoInfoCard"; +import { navItemAttrs } from "../lib/listNav"; import clsx from "clsx"; import type { Video, VideoStatus } from "../lib/api"; import { FEED_VIEW_SPEC, type FeedView } from "../lib/feedView"; @@ -388,6 +389,7 @@ function VideoCard({ onToggleSave, onOpen, menuProps, + navIndex, }: { video: Video; view: FeedView; @@ -403,6 +405,8 @@ function VideoCard({ * blow past this component's memo for every card in the virtualized feed on every parent * render. The card calls it with the video it already has. */ menuProps?: (v: Video) => Record; + /** Its position in a keyboard-navigable list (`lib/listNav`) — omit outside one (Home's rails). */ + navIndex?: number; }) { const { t, i18n } = useTranslation(); const { openChannel } = useNavigationActions(); @@ -465,6 +469,9 @@ function VideoCard({ target="_blank" rel="noreferrer" onClick={(e) => openInApp(e, video, onOpen)} + // Enter on a keyboard-selected card clicks THIS link (lib/listNav), so it opens exactly like a + // click: the in-app player, resuming where the reader left off. + data-nav-open="" title={titleClamped ? (video.title ?? undefined) : undefined} className={clsx( "font-medium leading-snug hover:text-accent", @@ -534,6 +541,7 @@ function VideoCard({ data-testid="video-card" data-video-id={video.id} {...menuProps?.(video)} + {...(navIndex !== undefined && navItemAttrs(navIndex))} className={clsx(rowShell, "flex items-center gap-3 px-3 py-2", watched && "opacity-55")} > {/* Actions lead: this is where the eye already is (the title is the thing you read), so @@ -597,6 +605,7 @@ function VideoCard({ data-testid="video-card" data-video-id={video.id} {...menuProps?.(video)} + {...(navIndex !== undefined && navItemAttrs(navIndex))} className={clsx(rowShell, "flex gap-3 p-2", watched && "opacity-55")} > ( + keyboardNav + renderItem={(v, itemWidth, index) => ( opts.onOpenChange(false)); + // an Escape the user aimed at whatever is actually on screen. A hover surface is a PASSIVE layer: + // it does not own the keyboard, so the page's own keys keep working under a resting pointer. + useLayer(opts.open && !referenceHidden, () => opts.onOpenChange(false), { passive: isHover }); return { nodeId, diff --git a/frontend/src/components/ui/KeyboardHelp.dom.test.tsx b/frontend/src/components/ui/KeyboardHelp.dom.test.tsx index 7949ff89..707b76d9 100644 --- a/frontend/src/components/ui/KeyboardHelp.dom.test.tsx +++ b/frontend/src/components/ui/KeyboardHelp.dom.test.tsx @@ -14,6 +14,7 @@ import { useKeyScope, type KeyScope, } from "../../lib/keymap"; +import { pushLayer, removeLayer } from "../../lib/layerStack"; vi.mock("react-i18next", async (importOriginal) => { const actual = await importOriginal(); @@ -27,9 +28,11 @@ afterEach(() => { closeKeyboardHelp(); }); -/** App's job: it binds the `global` scope (h / Mod+K / Shift+W) once for the whole tree. */ +/** App's job: it binds the `global` scope (h / Mod+K / Shift+W) and the `page` scope (the `g` chords) + * once for the whole tree. Home is the one module this stand-in account reaches, so the page block + * is the single `G H` chord, sharing Shift+W's line. */ function Host({ children }: { children?: React.ReactNode }) { - useGlobalKeymap(true, () => {}); + useGlobalKeymap(true, () => {}, ["home"]); return ( <> {children} @@ -81,15 +84,16 @@ describe("KeyboardHelp", () => { ); pressH(); const rows = descriptions(); - // A plexPlayer row first, the app-wide rows last — the reading order the reader wants. + // A plexPlayer row first, the app-wide rows last — the reading order the reader wants. (Home's + // line comes from the page block, which sits between the two; Shift+W joins it there.) expect(rows[0]).toBe("keys.playPause"); - expect(rows.slice(-3)).toEqual(["keys.palette", "keys.help", "keys.home"]); + expect(rows.slice(-3)).toEqual(["keys.home", "keys.palette", "keys.help"]); }); it("lists only the app-wide keys when no surface is registered", () => { render(); pressH(); - expect(descriptions()).toEqual(["keys.palette", "keys.help", "keys.home"]); + expect(descriptions()).toEqual(["keys.home", "keys.palette", "keys.help"]); }); // The context menu is NOT app-wide: it has its own scope, registered by `useContextMenu`, so the @@ -120,6 +124,24 @@ describe("KeyboardHelp", () => { expect(descriptions()).not.toContain("keys.contextMenu"); }); + // Opened over a player, Shift+F10 has nothing to reach — focus is inside the player, not on a row. + it("drops the context-menu key from a sheet opened over a keyboard-owning layer", () => { + function WithMenu() { + useContextMenu(); + return null; + } + render( + + + , + ); + const id = pushLayer(); + pressH(); + removeLayer(id); + expect(descriptions()).not.toContain("keys.contextMenu"); + expect(descriptions()).toContain("keys.help"); + }); + // Two live context menus (the Playlists page mounts one, its rail another) must not double the row. it("lists the context-menu chords once however many surfaces register", () => { function WithMenu() { diff --git a/frontend/src/components/ui/KeyboardHelp.tsx b/frontend/src/components/ui/KeyboardHelp.tsx index 497ada8b..e6a8d7e0 100644 --- a/frontend/src/components/ui/KeyboardHelp.tsx +++ b/frontend/src/components/ui/KeyboardHelp.tsx @@ -5,9 +5,12 @@ import { closeKeyboardHelp, formatChord, helpLines, + isPageLevelScope, useActiveScopes, useHelpConditionValues, useKeyboardHelpOpen, + useKeyboardHelpOverLayer, + useNavigablePages, } from "../../lib/keymap"; // The app's one keyboard cheat sheet. Mounted once by App; the global `h` that opens it lives in @@ -29,11 +32,21 @@ export default function KeyboardHelp() { const open = useKeyboardHelpOpen(); const scopes = useActiveScopes(); const conditions = useHelpConditionValues(); + const overLayer = useKeyboardHelpOverLayer(); + const pages = useNavigablePages(); if (!open) return null; // A `when` row is listed only where it applies — "Previous / next episode" inside a movie would be // a promise the player does not keep. - const lines = helpLines(scopes).filter((l) => !l.when || !!conditions[l.when]); + // Opened over a player or a dialog, the page-level keys (`s`, `g …`, `j`/`k`) are quiet — listing + // them there would promise keys that do nothing until the overlay closes. And a `g` chord is listed + // only for a module this account can actually reach. + // The context-menu key goes too: every `useContextMenu` is a page list, and with an overlay up the + // focus Shift+F10 needs is inside the overlay (pre-existing since E10, found in S4 UAT). + const live = overLayer + ? scopes.filter((s) => !isPageLevelScope(s) && s !== "contextMenu") + : scopes; + const lines = helpLines(live, pages).filter((l) => !l.when || !!conditions[l.when]); return (
diff --git a/frontend/src/components/ui/Tooltip.layer.dom.test.tsx b/frontend/src/components/ui/Tooltip.layer.dom.test.tsx new file mode 100644 index 00000000..4b0c88c3 --- /dev/null +++ b/frontend/src/components/ui/Tooltip.layer.dom.test.tsx @@ -0,0 +1,66 @@ +// @vitest-environment jsdom +// +// E11 S4: a hover surface (tooltip, hover card) is a PASSIVE layer. It still sits on the layer stack — +// Escape closes it before the surface beneath — but it does not own the keyboard, so the page-level +// keys (`j`/`k`, `s`, the `g` chords) keep working while one happens to be open under a resting pointer. +// Without the flag, hovering any hinted icon silently disabled keyboard navigation. +// +// jsdom lays nothing out, and floating-ui's `hide` middleware then reports the reference as scrolled +// out of view — and an invisible surface is deliberately NOT a layer (AnchoredPopover). A test on that +// would pass without the flag, so the reference and the viewport get real boxes first, and the test +// proves the surface is a layer (Escape closes it) before trusting the "not blocking" half. +import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest"; +import { act, cleanup, fireEvent, render, screen, waitFor } from "@testing-library/react"; +import Tooltip from "./Tooltip"; +import { anyBlockingLayer } from "../../lib/layerStack"; + +const box = { x: 200, y: 200, top: 200, left: 200, width: 60, height: 24, right: 260, bottom: 224 }; + +beforeAll(() => { + class Stub { + observe() {} + unobserve() {} + disconnect() {} + takeRecords() { + return []; + } + } + globalThis.ResizeObserver ??= Stub as unknown as typeof ResizeObserver; + vi.spyOn(Element.prototype, "getBoundingClientRect").mockReturnValue({ + ...box, + toJSON: () => box, + } as DOMRect); + for (const [prop, value] of [ + ["clientWidth", 1280], + ["clientHeight", 800], + ] as const) { + Object.defineProperty(document.documentElement, prop, { configurable: true, value }); + } +}); +afterAll(() => vi.restoreAllMocks()); +afterEach(cleanup); + +describe("an open tooltip", () => { + it("is a layer Escape reaches, but does not block the page keys", async () => { + render( + + + , + ); + const trigger = screen.getByText("icon").parentElement as HTMLElement; + act(() => { + fireEvent.pointerEnter(trigger); + fireEvent.mouseEnter(trigger); + fireEvent.mouseMove(trigger); + }); + const surface = await screen.findByRole("tooltip"); + await waitFor(() => expect(surface.style.visibility).not.toBe("hidden")); + + expect(anyBlockingLayer()).toBe(false); + + act(() => { + fireEvent.keyDown(document, { key: "Escape" }); + }); + await waitFor(() => expect(screen.queryByRole("tooltip")).toBeNull()); + }); +}); diff --git a/frontend/src/components/ui/VirtualGrid.tsx b/frontend/src/components/ui/VirtualGrid.tsx index 24fe8d92..f496ff65 100644 --- a/frontend/src/components/ui/VirtualGrid.tsx +++ b/frontend/src/components/ui/VirtualGrid.tsx @@ -8,6 +8,7 @@ import { type ReactNode, } from "react"; import { useVirtualizer } from "@tanstack/react-virtual"; +import { useListNav } from "../../lib/listNav"; import { GRID_GAP, columnsFor, itemWidthFor } from "./gridColumns"; import { shouldPrefetchNext } from "./readAhead"; @@ -66,8 +67,13 @@ export interface VirtualGridProps { rowEst: number; /** Renders one item. `itemWidth` is the measured px width of one cell — for width-derived * layouts (e.g. a one-line row that drops columns as it narrows); ignore it otherwise, and to - * protect a memo'd item from re-rendering on every resize frame, pass a stable value down. */ - renderItem: (item: T, itemWidth: number) => ReactNode; + * protect a memo'd item from re-rendering on every resize frame, pass a stable value down. + * `index` is the item's position in `items` — what `navItemAttrs` wants on the item's root. */ + renderItem: (item: T, itemWidth: number, index: number) => ReactNode; + /** Take part in page keyboard navigation (`lib/listNav`): `j`/`k`/arrows/Enter over the items. + * The grid supplies the column count and scrolls an unmounted item's row in; the item must spread + * `navItemAttrs(index)` on its root and mark its "open" element `data-nav-open`. */ + keyboardNav?: boolean; /** Infinite scroll — omit all three for a finite client-side list. */ hasNextPage?: boolean; isFetchingNextPage?: boolean; @@ -85,6 +91,7 @@ export default function VirtualGrid({ hasNextPage = false, isFetchingNextPage = false, fetchNextPage, + keyboardNav = false, }: VirtualGridProps) { const listRef = useRef(null); const [scrollEl, setScrollEl] = useState(null); @@ -139,6 +146,15 @@ export default function VirtualGrid({ const virtualRows = virtualizer.getVirtualItems(); + // Only ~a screen of rows (plus overscan) is mounted, so a key that moves further — or past the + // overscan at the edge — has to scroll the row in before the item exists to be focused. + useListNav(listRef, { + enabled: keyboardNav, + count: items.length, + cols: colCount, + reveal: (index) => virtualizer.scrollToIndex(Math.floor(index / colCount), { align: "auto" }), + }); + // Infinite-scroll read-ahead: pull the next page once the last rendered row is within the // read-ahead window of the loaded end (see readAhead.ts — the window scales with the rendered row // count, clamped so a tall window doesn't prefetch unboundedly). A no-op for finite lists (no @@ -159,6 +175,7 @@ export default function VirtualGrid({ {virtualRows.map((vr) => { const row = rows[vr.index]; if (!row) return null; + const first = vr.index * colCount; return (
({ {/* A transparent Fragment carries React's key so the rendered item stays the DIRECT grid/column child — some cards rely on that (e.g. `h-full` stretch, actions pinned to the bottom edge), which a wrapper element would silently break. */} - {row.map((item) => ( - {renderItem(item, itemWidth)} + {row.map((item, i) => ( + {renderItem(item, itemWidth, first + i)} ))}
diff --git a/frontend/src/i18n/locales/en/keys.json b/frontend/src/i18n/locales/en/keys.json index 37f9076c..b7e3decd 100644 --- a/frontend/src/i18n/locales/en/keys.json +++ b/frontend/src/i18n/locales/en/keys.json @@ -21,5 +21,18 @@ "back": "Stop & back to feed", "browseBack": "Back one level", "closePlayer": "Close the player", - "palette": "Open the jump-to palette" + "palette": "Open the jump-to palette", + "goFeed": "Go to Feed", + "goChannels": "Go to Channels", + "goPlaylists": "Go to Playlists", + "goPlex": "Go to Plex", + "goNotifications": "Go to Notifications", + "goMessages": "Go to Messages", + "goDownloads": "Go to Downloads", + "goStats": "Go to Stats", + "goSettings": "Go to Settings", + "search": "Jump to the search box", + "listStep": "Next / previous item", + "gridMove": "Move around the list", + "open": "Open the selected item" } diff --git a/frontend/src/i18n/locales/hu/keys.json b/frontend/src/i18n/locales/hu/keys.json index b6e24c44..d50a001b 100644 --- a/frontend/src/i18n/locales/hu/keys.json +++ b/frontend/src/i18n/locales/hu/keys.json @@ -21,5 +21,18 @@ "back": "Leállítás és vissza a feedre", "browseBack": "Vissza egy szintet", "closePlayer": "Lejátszó bezárása", - "palette": "Ugrás-paletta megnyitása" + "palette": "Ugrás-paletta megnyitása", + "goFeed": "Ugrás: Hírfolyam", + "goChannels": "Ugrás: Csatornák", + "goPlaylists": "Ugrás: Lejátszási listák", + "goPlex": "Ugrás: Plex", + "goNotifications": "Ugrás: Értesítések", + "goMessages": "Ugrás: Üzenetek", + "goDownloads": "Ugrás: Letöltések", + "goStats": "Ugrás: Statisztika", + "goSettings": "Ugrás: Beállítások", + "search": "Ugrás a keresőbe", + "listStep": "Következő / előző elem", + "gridMove": "Mozgás a listában", + "open": "A kijelölt elem megnyitása" } diff --git a/frontend/src/index.css b/frontend/src/index.css index ba7ded13..d35aa695 100644 --- a/frontend/src/index.css +++ b/frontend/src/index.css @@ -84,7 +84,8 @@ [role="button"]:focus-visible, [role="tab"]:focus-visible, summary:focus-visible { - outline: 2px solid color-mix(in srgb, var(--accent) 60%, transparent); + /* Solid accent (E11 D2 = B): the 60% mix of R9 all but vanished on the dark glass. */ + outline: 2px solid var(--accent); outline-offset: 2px; } } @@ -435,10 +436,29 @@ body { .selectable:focus-visible { /* outline (not a box-shadow ring) so it never fights a glass surface's box-shadow and never gets clipped by an overflow-hidden ancestor; inset offset keeps it inside tightly-packed rows. */ - outline: 2px solid color-mix(in srgb, var(--accent) 60%, transparent); + outline: 2px solid var(--accent); outline-offset: -2px; } + /* E11 S4 · a list/grid item the keyboard landed on (`lib/listNav`): the SAME solid ring, around the + whole card rather than one control inside it (D2 = B). It is focus-visible only, so a mouse click + never paints it, and it is a ring rather than a fill, so it never reads like the hover tint. The + scroll margins are what `scrollIntoView({block:"nearest"})` honours: they keep a card stepped to + from sliding under the floating header (`--hdr-h`) or flush against the bottom edge. */ + [data-nav-index] { + scroll-margin-top: calc(var(--hdr-h, 0px) + 0.75rem); + scroll-margin-bottom: 0.75rem; + /* A transparent resting ring of the SAME geometry: the video card transitions `all`, so without + it the ring animated in from the initial outline (3px, currentColor) — a white flash on every + `j`, measured in Chrome. Now only the colour fades in. */ + outline: 2px solid transparent; + outline-offset: 2px; + } + [data-nav-index]:focus-visible { + outline: 2px solid var(--accent); + outline-offset: 2px; + } + /* ===== R10 S4 · Selection-paint recipes ===== The ~14 hand-rolled "this is the selected / current / active one" idioms all painted the SAME bg-accent fill via an inline ternary (`sel ? "bg-accent text-accent-fg border-accent" : "…"`), diff --git a/frontend/src/lib/globalKeymap.dom.test.tsx b/frontend/src/lib/globalKeymap.dom.test.tsx index 023f242c..e4ac224f 100644 --- a/frontend/src/lib/globalKeymap.dom.test.tsx +++ b/frontend/src/lib/globalKeymap.dom.test.tsx @@ -39,7 +39,7 @@ function Host({ onState: (s: string) => void; onHome?: () => void; }) { - useGlobalKeymap(enabled, onHome); + useGlobalKeymap(enabled, (page) => page === "home" && onHome(), ["home"]); const help = useKeyboardHelpOpen(); const palette = usePaletteOpen(); useEffect(() => { diff --git a/frontend/src/lib/keymap.sequence.test.ts b/frontend/src/lib/keymap.sequence.test.ts new file mode 100644 index 00000000..2e8d6c3b --- /dev/null +++ b/frontend/src/lib/keymap.sequence.test.ts @@ -0,0 +1,166 @@ +// @vitest-environment jsdom +// +// Two-step sequences (`g h`) and the page-level quiet rule, in their own file: the pending first step +// is module state, and a test that left one pending would turn the NEXT test's first key into its +// second step. Every test here starts a fake clock far past the previous test's, so nothing carries. +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { SHORTCUTS, dispatch, formatChord, helpLines, type ShortcutId } from "./keymap"; +import { pushLayer, removeLayer } from "./layerStack"; + +let clock = Date.UTC(2030, 0, 1); +beforeEach(() => { + vi.useFakeTimers({ toFake: ["Date"] }); + clock += 3_600_000; + vi.setSystemTime(clock); +}); +afterEach(() => vi.useRealTimers()); + +function press(init: KeyboardEventInit, on?: HTMLElement): KeyboardEvent { + const e = new KeyboardEvent("keydown", { bubbles: true, cancelable: true, ...init }); + (on ?? document.body).dispatchEvent(e); + return e; +} + +/** Every page row's handler, recording which one ran. */ +function pageHandlers(calls: string[]) { + return Object.fromEntries( + SHORTCUTS.filter((s) => s.scope === "page").map((s) => [s.id, () => void calls.push(s.id)]), + ) as { readonly [K in ShortcutId<"page">]: () => void }; +} +const listHandlers = (calls: string[]) => + ({ + next: () => calls.push("next"), + prev: () => calls.push("prev"), + left: () => {}, + right: () => {}, + up: () => {}, + down: () => {}, + open: () => {}, + }) as const; + +describe("two-step sequences", () => { + it("runs the row once both steps arrive, and not on the first alone", () => { + const calls: string[] = []; + expect(dispatch(press({ key: "g" }), "page", pageHandlers(calls))).toBe(true); + expect(calls).toEqual([]); + expect(dispatch(press({ key: "f" }), "page", pageHandlers(calls))).toBe(true); + expect(calls).toEqual(["goFeed"]); + }); + + // Every scope has its own window listener, so the `h` completing `g h` reaches the global one too. + it("keeps the completing key from every other scope, whichever listener runs first", () => { + const calls: string[] = []; + const global = { + help: () => calls.push("help"), + palette: () => {}, + home: () => {}, + }; + // another scope BEFORE the owner: the pending step belongs to `page`, so it stands aside + dispatch(press({ key: "g" }), "page", pageHandlers(calls)); + const h = press({ key: "h" }); + expect(dispatch(h, "global", global)).toBe(false); + expect(dispatch(h, "page", pageHandlers(calls))).toBe(true); + expect(calls).toEqual(["goHome"]); + + // another scope AFTER the owner: the owner stamped the event + dispatch(press({ key: "g" }), "page", pageHandlers(calls)); + const j = press({ key: "j" }); // breaks the sequence: no `g j` + expect(dispatch(j, "page", pageHandlers(calls))).toBe(false); + expect(dispatch(j, "list", listHandlers(calls))).toBe(false); + expect(calls).toEqual(["goHome"]); + }); + + it("gives up after the window, so a late key is an ordinary key again", () => { + const calls: string[] = []; + dispatch(press({ key: "g" }), "page", pageHandlers(calls)); + vi.setSystemTime(clock + 1_600); + expect(dispatch(press({ key: "j" }), "list", listHandlers(calls))).toBe(true); + expect(dispatch(press({ key: "f" }), "page", pageHandlers(calls))).toBe(false); + expect(calls).toEqual(["next"]); + }); + + it("is not broken by reaching for a modifier between the steps", () => { + const calls: string[] = []; + dispatch(press({ key: "g" }), "page", pageHandlers(calls)); + dispatch(press({ key: "Shift", shiftKey: true }), "page", pageHandlers(calls)); + dispatch(press({ key: "d" }), "page", pageHandlers(calls)); + expect(calls).toEqual(["goDownloads"]); + }); + + it("does not start or finish inside a text field", () => { + const calls: string[] = []; + const input = document.createElement("input"); + document.body.append(input); + expect(dispatch(press({ key: "g" }, input), "page", pageHandlers(calls))).toBe(false); + dispatch(press({ key: "g" }), "page", pageHandlers(calls)); + expect(dispatch(press({ key: "f" }, input), "page", pageHandlers(calls))).toBe(false); + expect(calls).toEqual([]); + input.remove(); + }); +}); + +describe("page-level scopes under a layer", () => { + it("are quiet while a keyboard-owning layer is open", () => { + const calls: string[] = []; + const id = pushLayer(); + expect(dispatch(press({ key: "j" }), "list", listHandlers(calls))).toBe(false); + expect(dispatch(press({ key: "s" }), "search", { focus: () => calls.push("s") })).toBe(false); + dispatch(press({ key: "g" }), "page", pageHandlers(calls)); + dispatch(press({ key: "f" }), "page", pageHandlers(calls)); + removeLayer(id); + expect(calls).toEqual([]); + }); + + // The layer's OWN keys must keep working — that is the player the page keys are standing aside for. + it("leave the layer's own scope alone", () => { + const calls: string[] = []; + const id = pushLayer(); + expect( + dispatch(press({ key: "k" }), "plexPlayer", { playPause: () => calls.push("pp") } as never), + ).toBe(true); + removeLayer(id); + expect(calls).toEqual(["pp"]); + }); + + it("keep working under a passive layer (an open tooltip)", () => { + const calls: string[] = []; + const id = pushLayer({ passive: true }); + expect(dispatch(press({ key: "j" }), "list", listHandlers(calls))).toBe(true); + removeLayer(id); + expect(calls).toEqual(["next"]); + }); +}); + +describe("the page-level rows", () => { + // Owner rule D1, enforced rather than commented: on the Hungarian layout the owner types on, a + // chord with a modifier (or `/`, `?`, which ARE Shift chords there) is not a quick key. + it("need no modifier and no Shift-only character", () => { + for (const s of SHORTCUTS) { + if (!["page", "search", "list"].includes(s.scope)) continue; + for (const chord of s.keys) { + for (const step of chord.split(" ")) { + expect(step.includes("+"), `${s.scope}:${s.id} ${chord}`).toBe(false); + expect(/^[a-z0-9,.-]$|^(Enter|Escape|Arrow(Up|Down|Left|Right))$/.test(step), chord).toBe( + true, + ); + } + } + } + }); + + it("render as the keys to press, step by step", () => { + expect(formatChord("g h")).toBe("G H"); + expect(formatChord("g ,")).toBe("G ,"); + }); + + // `g h` shares Home's line with Shift+W; leaving out an unreachable chord must not take that with it. + it("drop an unreachable module's chord but keep the line it shares", () => { + const lines = helpLines(["page", "global"], ["home", "feed"]); + expect(lines.find((l) => l.labelKey === "keys.home")?.chords).toEqual(["g h", "Shift+w"]); + expect(lines.some((l) => l.labelKey === "keys.goFeed")).toBe(true); + expect(lines.some((l) => l.labelKey === "keys.goPlex")).toBe(false); + expect( + helpLines(["page", "global"], []).find((l) => l.labelKey === "keys.home")?.chords, + ).toEqual(["Shift+w"]); + }); +}); diff --git a/frontend/src/lib/keymap.ts b/frontend/src/lib/keymap.ts index 83a236a2..68dedce6 100644 --- a/frontend/src/lib/keymap.ts +++ b/frontend/src/lib/keymap.ts @@ -1,5 +1,7 @@ -import { useEffect, useRef } from "react"; +import { useEffect, useRef, type RefObject } from "react"; +import { anyBlockingLayer } from "./layerStack"; import { createStore } from "./store"; +import type { Page } from "./urlState"; // ── The app's ONE keyboard map ──────────────────────────────────────────────────────────────── // Before E10 S3 six surfaces each owned a private `keydown` listener with its own key literals and @@ -18,8 +20,26 @@ import { createStore } from "./store"; // one place, and the sheet renders this table rather than a hand-copied array beside it. /** Which surface a shortcut belongs to. `global` is live app-wide; the rest only while that surface - * is mounted. The help sheet shows `global` plus the topmost registered scope. */ -export type KeyScope = "global" | "undo" | "contextMenu" | "ytPlayer" | "plexPlayer" | "plexBrowse"; + * is mounted. The help sheet shows `global` plus the registered scopes. + * + * `page`, `search` and `list` are the PAGE-LEVEL scopes (E11 S4): they act on the page underneath + * every overlay, so `dispatch` keeps them quiet while a layer that owns the keyboard is open. That is + * also what keeps them clear of the players' bare letters — the Plex player's `s` and `k` only ever + * run with the player's own layer up. */ +export type KeyScope = + | "global" + | "page" + | "search" + | "list" + | "undo" + | "contextMenu" + | "ytPlayer" + | "plexPlayer" + | "plexBrowse"; + +const PAGE_LEVEL: ReadonlySet = new Set(["page", "search", "list"]); +/** True for a scope that acts on the page beneath the overlays (see `KeyScope`). */ +export const isPageLevelScope = (scope: KeyScope): boolean => PAGE_LEVEL.has(scope); /** Extra render condition for the help sheet (the shortcut itself is always live). */ type ShortcutWhen = "episode"; @@ -42,11 +62,19 @@ export interface Shortcut { * and a player whose Escape died inside its own subtitle-offset input is a trap. Every other * shortcut stays out of the way of typing. */ readonly allowInInput?: boolean; + /** The module a `g` chord jumps to. The sheet lists the row only while that module is available + * to this account, and the handler refuses it otherwise — a gated module has no page to land on. */ + readonly page?: Page; } // Chord syntax: `Mod` = Ctrl on Windows/Linux, ⌘ on macOS. `Shift`/`Alt` are literal. The final // segment is a `KeyboardEvent.key` name, except `Space` (which also accepts `code`, because a -// focused button reports `key === " "` inconsistently across engines). +// focused button reports `key === " "` inconsistently across engines). A SPACE separates the two +// steps of a sequence: `g h` is `g`, then `h` within SEQUENCE_MS. +// +// Owner rule (E11 D1): no new binding may need a modifier on the Hungarian QWERTZ layout — so no `/` +// or `?` (both Shift chords there). Letters, digits, `-`, `,`, `.`, Esc/Enter/arrows only. `keyMatches` +// compares `e.key`, so a letter follows the printed legend (the HU Z/Y swap included). export const SHORTCUTS = [ // --- global --------------------------------------------------------------------------------- // `h`, not `?`: on a Hungarian layout `?` is a Shift chord, and the one help key people already @@ -60,6 +88,50 @@ export const SHORTCUTS = [ // sits directly above `s`, so a blind reach for the subtitle key would yank you out of the film // and onto the landing page. A chord makes it the deliberate "I'm stuck — answer the question". { id: "home", scope: "global", keys: ["Shift+w"], labelKey: "keys.home" }, + + // --- page-level: jump to a module (quiet under an overlay) ----------------------------------- + // English initials, because the Hungarian ones collide (Letöltések / Lejátszási listák). `g h` is + // Shift+W's modifier-free twin and shares its help line. Admin pages stay on the palette. + { id: "goHome", scope: "page", keys: ["g h"], labelKey: "keys.home", page: "home" }, + { id: "goFeed", scope: "page", keys: ["g f"], labelKey: "keys.goFeed", page: "feed" }, + { id: "goChannels", scope: "page", keys: ["g c"], labelKey: "keys.goChannels", page: "channels" }, + { + id: "goPlaylists", + scope: "page", + keys: ["g p"], + labelKey: "keys.goPlaylists", + page: "playlists", + }, + { id: "goPlex", scope: "page", keys: ["g x"], labelKey: "keys.goPlex", page: "plex" }, + { + id: "goNotifications", + scope: "page", + keys: ["g n"], + labelKey: "keys.goNotifications", + page: "notifications", + }, + { id: "goMessages", scope: "page", keys: ["g m"], labelKey: "keys.goMessages", page: "messages" }, + { + id: "goDownloads", + scope: "page", + keys: ["g d"], + labelKey: "keys.goDownloads", + page: "downloads", + }, + { id: "goStats", scope: "page", keys: ["g s"], labelKey: "keys.goStats", page: "stats" }, + { id: "goSettings", scope: "page", keys: ["g ,"], labelKey: "keys.goSettings", page: "settings" }, + + // --- the module's search box (wherever a SearchBar is mounted) ------------------------------- + { id: "focus", scope: "search", keys: ["s"], labelKey: "keys.search" }, + + // --- list / grid navigation (wherever a list registers with `useListNav`) -------------------- + { id: "next", scope: "list", keys: ["j"], labelKey: "keys.listStep" }, + { id: "prev", scope: "list", keys: ["k"], labelKey: "keys.listStep" }, + { id: "left", scope: "list", keys: ["ArrowLeft"], labelKey: "keys.gridMove" }, + { id: "right", scope: "list", keys: ["ArrowRight"], labelKey: "keys.gridMove" }, + { id: "up", scope: "list", keys: ["ArrowUp"], labelKey: "keys.gridMove" }, + { id: "down", scope: "list", keys: ["ArrowDown"], labelKey: "keys.gridMove" }, + { id: "open", scope: "list", keys: ["Enter"], labelKey: "keys.open" }, // --- the row context menu (wherever useContextMenu is wired) --------------------------------- // Its OWN scope, not `global`: only the surfaces that actually call `useContextMenu` register it, // so the sheet stops advertising "Open the context menu" on the feed, Plex, Downloads and Settings @@ -206,23 +278,87 @@ export function matchesShortcut( return !!row?.keys.some((k) => matches(e, k)); } +// ── Two-step sequences (`g h`) ───────────────────────────────────────────────────────────────── +// Each scope keeps its own window listener, so a sequence cannot live in one listener's closure: the +// `h` that completes `g h` also reaches the global listener (where `h` opens the sheet) and every +// other live scope. So the pending first step is module state, and it belongs to the scope that +// started it — every OTHER scope ignores the next key while it is pending, and the owner stamps the +// event once it has resolved it, so a scope whose listener runs after the owner's ignores it too. +// The stamp is the layerStack Escape trick: every listener in one dispatch gets the same object. +const SEQUENCE_MS = 1500; +let pending: { scope: KeyScope; key: string; at: number } | null = null; +const sequenceEvents = new WeakSet(); +// A modifier's own keydown (reaching for Shift) neither completes nor breaks a sequence. +const MODIFIER_KEYS = new Set(["Shift", "Control", "Alt", "AltGraph", "Meta", "CapsLock"]); + +function liveSequence(e: KeyboardEvent): typeof pending { + if (!pending) return null; + if (Date.now() - pending.at >= SEQUENCE_MS) { + pending = null; + return null; + } + return MODIFIER_KEYS.has(e.key) ? null : pending; +} + +const stepsOf = (chord: string): string[] => chord.split(" "); + /** Run `scope`'s map against an event. Returns true when a shortcut matched (the handler decides - * whether to `preventDefault`, because only it knows if it actually acted). `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). */ + * 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 + * 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( e: KeyboardEvent, scope: S, handlers: { readonly [K in ShortcutId]: (e: KeyboardEvent) => void }, ): boolean { + if (sequenceEvents.has(e)) return false; + const live = liveSequence(e); + if (live && live.scope !== scope) return false; + const quiet = isPageLevelScope(scope) && anyBlockingLayer(); const typing = isTypingTarget(e); + const run = (id: string) => (handlers as Record void>)[id]?.(e); + + if (live) { + // This key ends the sequence either way: it completes a row, or it breaks the attempt. + pending = null; + sequenceEvents.add(e); + if (quiet || typing) return false; + for (const s of ROWS) { + if (s.scope !== scope || s.handledLocally) continue; + const hit = s.keys.some((k) => { + const [first, second] = stepsOf(k); + return second !== undefined && first === live.key && matches(e, second); + }); + if (hit) { + run(s.id); + return true; + } + } + return false; + } + + if (quiet) return false; for (const s of ROWS) { if (s.scope !== scope || s.handledLocally) continue; if (typing && !s.allowInInput) continue; - if (!s.keys.some((k) => matches(e, k))) continue; - (handlers as Record void>)[s.id]?.(e); + if (!s.keys.some((k) => stepsOf(k).length === 1 && matches(e, k))) continue; + run(s.id); return true; } + if (typing) return false; + // Nothing matched outright — does this key open one of the scope's sequences? + for (const s of ROWS) { + if (s.scope !== scope || s.handledLocally) continue; + const first = s.keys.map(stepsOf).find((st) => st.length === 2 && matches(e, st[0]!))?.[0]; + if (first !== undefined) { + pending = { scope, key: first, at: Date.now() }; + sequenceEvents.add(e); + return true; + } + } return false; } @@ -261,18 +397,40 @@ export function useActiveScopes(): KeyScope[] { // rail each mount a `useContextMenu`), and `helpLines` appends a row's chords once per occurrence — // so without this the sheet rendered "⇧ F10 Menu ⇧ F10 Menu" under a duplicated React key // (E10 review round 2). Innermost registration wins the ordering. - return [...new Set([...stack].reverse())].concat("global"); + const live = [...new Set([...stack].reverse())]; + // The page-level scopes go in a FIXED order after the surfaces, not in registration order: App + // registers `page` in its own effect, which React runs after every child's — so by mount order + // the module jumps would head the sheet, above the list keys of the page you are looking at. + const surfaces = live.filter((s) => !isPageLevelScope(s)); + const pageLevel = (["list", "search", "page"] as const).filter((s) => live.includes(s)); + return [...surfaces, ...pageLevel, "global"]; } // ── The keyboard help sheet ─────────────────────────────────────────────────────────────────── // One sheet for the whole app, opened by the global `h` (or a surface's own keyboard button), so // there is exactly one answer to "what can I press here" instead of one per player. const helpOpen = createStore(false); +// Whether the sheet was opened OVER a layer that owns the keyboard (a player, a dialog). The page-level +// keys are quiet there, so the sheet must not list them — and it cannot ask the layer stack itself +// once it is showing, because its own Modal is a layer too. So the answer is taken at the moment of +// opening, before that Modal mounts. (The palette's "Help" row runs only after the palette has closed.) +const helpOverLayer = createStore(false); export const closeKeyboardHelp = (): void => helpOpen.set(false); -export const toggleKeyboardHelp = (): void => helpOpen.set((v) => !v); +export const toggleKeyboardHelp = (): void => { + const next = !helpOpen.get(); + if (next) helpOverLayer.set(anyBlockingLayer()); + helpOpen.set(next); +}; /** Subscribe to whether the sheet is showing (the Plex player suppresses wheel-volume under it). */ export const useKeyboardHelpOpen = (): boolean => helpOpen.use(); +/** Whether the sheet was opened over a keyboard-owning layer (see `helpOverLayer`). */ +export const useKeyboardHelpOverLayer = (): boolean => helpOverLayer.use(); + +// The modules this account can reach — published by `useGlobalKeymap`, read by the `g` chords and +// by the sheet (which drops the chord of a gated module rather than promise a page that isn't there). +const navigablePages = createStore([]); +export const useNavigablePages = (): readonly Page[] => navigablePages.use(); // ── The command palette ─────────────────────────────────────────────────────────────────────── const paletteOpen = createStore(false); @@ -280,21 +438,32 @@ const paletteOpen = createStore(false); export const closePalette = (): void => paletteOpen.set(false); export const usePaletteOpen = (): boolean => paletteOpen.use(); -/** Bind the `global` scope, once, for the whole app. It lives here rather than inside either - * overlay because both are opened by it: a listener that sat in one of them would have to route a - * key belonging to the other, and `dispatch`'s exhaustiveness would stop meaning anything. App - * calls this; the overlays are pure renderers of their store. +/** Bind the `global` and `page` scopes, once, for the whole app. It lives here rather than inside + * either overlay because both are opened by it: a listener that sat in one of them would have to + * route a key belonging to the other, and `dispatch`'s exhaustiveness would stop meaning anything. + * App calls this; the overlays are pure renderers of their store. + * + * `pages` is the set of modules this account can reach (a `g` chord to anything else is a no-op). * * `enabled` must track where those overlays (and the pages Shift+W jumps to) are actually MOUNTED. * Bound unconditionally it swallowed `h` and Mod+K on the loading/Welcome/setup screens — the * browser shortcut was preventDefault'ed, nothing appeared, and the flipped module-level store then * popped the overlay open by itself the moment the signed-in shell mounted (E10 review round 1). */ -export function useGlobalKeymap(enabled: boolean, onHome: () => void): void { - // The Home jump is the one global key whose action lives OUTSIDE this file — navigation is a +export function useGlobalKeymap( + enabled: boolean, + onNavigate: (page: Page) => void, + pages: readonly Page[], +): void { + // The page jumps are the global keys whose action lives OUTSIDE this file — navigation is a // provider and `lib/` may not import a component — so App hands it in. Kept in a ref so a fresh // closure each render doesn't rebind the window listener (and can't leave a stale `setPage`). - const homeRef = useRef(onHome); - homeRef.current = onHome; + const navRef = useRef(onNavigate); + navRef.current = onNavigate; + useKeyScope("page", enabled); + const pagesKey = pages.join(","); + useEffect(() => { + navigablePages.set(pagesKey ? (pagesKey.split(",") as Page[]) : []); + }, [pagesKey]); useEffect(() => { if (!enabled) { // Leaving a stale `true` behind would spring the overlay open on the next mount. @@ -302,6 +471,17 @@ export function useGlobalKeymap(enabled: boolean, onHome: () => void): void { paletteOpen.set(false); return; } + // Built from the rows, so a new `g` row needs no second edit here — the row's `page` IS the action. + const pageHandlers = Object.fromEntries( + ROWS.filter((s) => s.scope === "page").map((s) => [ + s.id, + (ev: KeyboardEvent) => { + if (!s.page || !navigablePages.get().includes(s.page)) return; + ev.preventDefault(); + navRef.current(s.page); + }, + ]), + ) as { readonly [K in ShortcutId<"page">]: (e: KeyboardEvent) => void }; const onKey = (e: KeyboardEvent) => { // Two of the three global keys TOGGLE an overlay, and each overlay mount/unmount costs a real // history entry (useBackToClose); the third (Shift+W) pushes a page entry of its own. Either @@ -309,10 +489,10 @@ export function useGlobalKeymap(enabled: boolean, onHome: () => void): void { // our own depth bookkeeping keeps counting them — a desynced stack whose next browser Back // walks out of the app. Holding a key is not a second press. if (e.repeat) return; - dispatch(e, "global", { + const handled = dispatch(e, "global", { help: (ev) => { ev.preventDefault(); - helpOpen.set((v) => !v); + toggleKeyboardHelp(); }, palette: (ev) => { // Mod+K is the browser's "focus the search bar" in a few builds; we want it. @@ -321,9 +501,10 @@ export function useGlobalKeymap(enabled: boolean, onHome: () => void): void { }, home: (ev) => { ev.preventDefault(); - homeRef.current(); + navRef.current("home"); }, }); + if (!handled) dispatch(e, "page", pageHandlers); }; window.addEventListener("keydown", onKey); return () => window.removeEventListener("keydown", onKey); @@ -344,6 +525,30 @@ export function useHelpConditions(conditions: HelpConditions): void { }, [episode]); } +// ── The module's search box ─────────────────────────────────────────────────────────────────── +/** Bind `s` to "put the caret in this search box" while the box is mounted. The one SearchBar in the + * header calls this; the text is selected so the next keystrokes replace the old query (an arrow key + * keeps it). `preventDefault` keeps the `s` itself out of the field it just focused. */ +export function useSearchFocusKey(input: RefObject): void { + useKeyScope("search"); + useEffect(() => { + const onKey = (e: KeyboardEvent) => { + if (e.repeat) return; + dispatch(e, "search", { + focus: (ev) => { + const el = input.current; + if (!el) return; + ev.preventDefault(); + el.focus(); + el.select(); + }, + }); + }; + window.addEventListener("keydown", onKey); + return () => window.removeEventListener("keydown", onKey); + }, [input]); +} + export const useHelpConditionValues = (): HelpConditions => helpConditions.use(); // ── Display ─────────────────────────────────────────────────────────────────────────────────── @@ -361,8 +566,13 @@ const GLYPHS: Record = { const isMac = (): boolean => typeof navigator !== "undefined" && /mac|iphone|ipad/i.test(navigator.userAgent); -/** A chord as the help sheet shows it: `Mod+Shift+z` → `Ctrl ⇧ Z` (`⌘ ⇧ Z` on a Mac). */ +/** A chord as the help sheet shows it: `Mod+Shift+z` → `Ctrl ⇧ Z` (`⌘ ⇧ Z` on a Mac), and a + * sequence step by step: `g h` → `G H`. */ export function formatChord(chord: string): string { + return stepsOf(chord).map(formatStep).join(" "); +} + +function formatStep(chord: string): string { return chord .split("+") .map((part, i, all) => { @@ -380,15 +590,19 @@ export interface HelpLine { when?: ShortcutWhen; } -/** The shortcuts of `scopes`, grouped into help lines by description. Scopes come out in the order +/** The shortcuts of `scopes`, grouped into help lines by description (with `pages`, the `g` chords + * are limited to those modules). Scopes come out in the order * ASKED FOR, rows within a scope in table order — so a surface passing its own scope first heads its * sheet with its own keys and leaves the app-wide ones underneath, where they read as context. */ -export function helpLines(scopes: readonly KeyScope[]): HelpLine[] { +export function helpLines(scopes: readonly KeyScope[], pages?: readonly Page[]): HelpLine[] { const out: HelpLine[] = []; const byLabel = new Map(); for (const scope of scopes) { for (const s of ROWS) { if (s.scope !== scope) continue; + // A `g` chord to a module this account cannot reach is left out ROW by row, not line by line: + // `g h` shares its line with Shift+W, which must stay. + if (s.page && pages && !pages.includes(s.page)) continue; const existing = byLabel.get(s.labelKey); if (existing) { existing.chords.push(...s.keys); diff --git a/frontend/src/lib/layerStack.ts b/frontend/src/lib/layerStack.ts index 2ac0057b..d56d8512 100644 --- a/frontend/src/lib/layerStack.ts +++ b/frontend/src/lib/layerStack.ts @@ -16,15 +16,28 @@ import { useEffect, useRef } from "react"; let stack: number[] = []; let nextId = 1; +// Layers that take Escape but do not own the keyboard: an open tooltip or hover card. They sit on the +// stack for Escape ordering like any other layer, but `anyBlockingLayer` looks past them — otherwise +// a tooltip that happens to be open under a resting mouse pointer would silence the page's `j`/`k`. +const passive = new Set(); -export function pushLayer(): number { +export function pushLayer(opts?: { passive?: boolean }): number { const id = nextId++; stack.push(id); + if (opts?.passive) passive.add(id); return id; } export function removeLayer(id: number): void { stack = stack.filter((x) => x !== id); + passive.delete(id); +} + +/** True while a layer that owns the keyboard is open — a modal, a player, a click-opened menu. The + * page-level keys (`s`, the `g` chords, list navigation) stay quiet then: they act on the page + * underneath, which the user cannot see or reach while such a layer is up. */ +export function anyBlockingLayer(): boolean { + return stack.some((id) => !passive.has(id)); } export function isTopmost(id: number): boolean { @@ -62,14 +75,17 @@ export function escapeHandled(e: KeyboardEvent): boolean { export function useLayer( active: boolean, onEscape?: () => void, + opts?: { passive?: boolean }, ): { isTopmost: () => boolean; anyAbove: () => boolean } { const idRef = useRef(null); const escRef = useRef(onEscape); escRef.current = onEscape; + const passiveRef = useRef(!!opts?.passive); + passiveRef.current = !!opts?.passive; useEffect(() => { if (!active) return; - const id = pushLayer(); + const id = pushLayer({ passive: passiveRef.current }); idRef.current = id; let onKey: ((e: KeyboardEvent) => void) | undefined; if (escRef.current) { diff --git a/frontend/src/lib/listNav.dom.test.tsx b/frontend/src/lib/listNav.dom.test.tsx new file mode 100644 index 00000000..3e24205b --- /dev/null +++ b/frontend/src/lib/listNav.dom.test.tsx @@ -0,0 +1,220 @@ +// @vitest-environment jsdom +// +// `lib/listNav` against real DOM: `j`/`k` from item to item and across lists in reading order, the +// arrows around a grid, Enter as a click on the item's own "open" element — and the places where the +// keys must stay out of the way (a field, an overlay, a control inside the card). +import { useRef } from "react"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { act, cleanup, render, screen, waitFor } from "@testing-library/react"; +import { navItemAttrs, useListNav } from "./listNav"; +import { pushLayer, removeLayer } from "./layerStack"; + +afterEach(cleanup); + +function List({ + id, + count, + cols = 1, + mounted, + reveal, + onOpen = () => {}, +}: { + id: string; + count: number; + cols?: number; + /** Mount only the first N items, the way a virtualized grid leaves the rest out of the DOM. */ + mounted?: number; + reveal?: (index: number) => void; + onOpen?: (id: string) => void; +}) { + const ref = useRef(null); + useListNav(ref, { enabled: true, count, cols, reveal }); + return ( +
+ {Array.from({ length: mounted ?? count }, (_, i) => ( + + ))} +
+ ); +} + +/** A keydown where a real one starts: on whatever holds focus, bubbling to the window listener. */ +function press(key: string): KeyboardEvent { + const e = new KeyboardEvent("keydown", { key, bubbles: true, cancelable: true }); + act(() => { + (document.activeElement ?? document.body).dispatchEvent(e); + }); + return e; +} + +const focused = () => (document.activeElement as HTMLElement | null)?.dataset.testid; + +describe("useListNav", () => { + it("lands the first j on an item, then steps one item at a time", () => { + render(); + press("j"); + expect(focused()).toBe("a0"); + press("j"); + expect(focused()).toBe("a1"); + press("k"); + expect(focused()).toBe("a0"); + }); + + // The Tab order is the reader's, not ours: only the item the keyboard is on is focusable, and the + // previous one gives its tabindex back. + it("roves a single tabindex=-1 and never adds a Tab stop", () => { + render(); + expect(screen.getByTestId("a0").hasAttribute("tabindex")).toBe(false); + press("j"); + expect(screen.getByTestId("a0").getAttribute("tabindex")).toBe("-1"); + press("j"); + expect(screen.getByTestId("a0").hasAttribute("tabindex")).toBe(false); + expect(screen.getByTestId("a1").getAttribute("tabindex")).toBe("-1"); + }); + + // The player (E11 S2) restores focus to whatever held it when it opened. If the item gave its + // tabindex back on blur, that restore would silently fail and focus would fall to . + it("keeps the item focusable after focus leaves it, so a dialog can restore onto it", () => { + render( + <> + + + , + ); + press("j"); + act(() => screen.getByTestId("outside").focus()); + act(() => screen.getByTestId("a0").focus()); + expect(focused()).toBe("a0"); + }); + + it("stops at both ends of the page instead of wrapping", () => { + render(); + press("j"); + press("k"); + expect(focused()).toBe("a0"); + press("j"); + press("j"); + expect(focused()).toBe("a1"); + }); + + // Owner decision D4: j/k walk every collection on the page in reading order. + it("continues into the next list, and back, in document order", () => { + render( + <> + + + , + ); + press("j"); + press("j"); + press("j"); + expect(focused()).toBe("b0"); + press("k"); + expect(focused()).toBe("a1"); + }); + + it("moves around a grid with the arrows, row by row", () => { + // 3 columns, 7 items: rows [0 1 2] [3 4 5] [6] + render(); + press("j"); + press("ArrowRight"); + expect(focused()).toBe("a1"); + press("ArrowDown"); + expect(focused()).toBe("a4"); + press("ArrowDown"); // the row below is short: land on its last item + expect(focused()).toBe("a6"); + press("ArrowDown"); // already on the last row + expect(focused()).toBe("a6"); + press("ArrowUp"); + expect(focused()).toBe("a3"); + press("ArrowLeft"); // left wraps to the end of the row above — reading order + expect(focused()).toBe("a2"); + }); + + it("works the arrows from a control inside the card too", () => { + render(); + act(() => screen.getByTestId("a0-action").focus()); + press("ArrowDown"); + expect(focused()).toBe("a2"); + }); + + // Outside a list an arrow scrolls the page; that native meaning must survive. + it("leaves the arrows alone when focus is not in a list", () => { + render(); + expect(press("ArrowDown").defaultPrevented).toBe(false); + expect(focused()).toBeUndefined(); + }); + + it("opens the item on Enter by clicking its own open element", () => { + const onOpen = vi.fn(); + render(); + press("j"); + press("j"); + expect(press("Enter").defaultPrevented).toBe(true); + expect(onOpen).toHaveBeenCalledWith("a1"); + }); + + it("leaves Enter to a control inside the card", () => { + const onOpen = vi.fn(); + render(); + act(() => screen.getByTestId("a0-action").focus()); + expect(press("Enter").defaultPrevented).toBe(false); + expect(onOpen).not.toHaveBeenCalled(); + }); + + it("stays out of a text field", () => { + render( + <> + + + , + ); + act(() => screen.getByTestId("field").focus()); + press("j"); + expect(focused()).toBe("field"); + }); + + it("is quiet under a layer that owns the keyboard, but not under a tooltip", () => { + render(); + const modal = pushLayer(); + press("j"); + expect(focused()).toBeUndefined(); + removeLayer(modal); + + const tooltip = pushLayer({ passive: true }); + press("j"); + expect(focused()).toBe("a0"); + removeLayer(tooltip); + }); + + // A virtualized grid only mounts a screen of rows: the next item may not exist yet. + it("asks the list to reveal an unmounted item, then focuses it once it renders", async () => { + const reveal = vi.fn(); + const { rerender } = render(); + press("j"); + press("j"); + press("j"); + expect(reveal).toHaveBeenCalledWith(2); + expect(focused()).toBe("a1"); + rerender(); + await waitFor(() => expect(focused()).toBe("a2")); + }); + + it("unbinds when the last list unmounts", () => { + const { unmount } = render(); + unmount(); + expect(press("j").defaultPrevented).toBe(false); + }); +}); diff --git a/frontend/src/lib/listNav.ts b/frontend/src/lib/listNav.ts new file mode 100644 index 00000000..d8be132a --- /dev/null +++ b/frontend/src/lib/listNav.ts @@ -0,0 +1,218 @@ +import { useEffect, useRef, type RefObject } from "react"; +import { dispatch, useKeyScope } from "./keymap"; + +// ── Keyboard navigation over lists and grids (E11 S4, UX-6) ────────────────────────────────────── +// Before this, a feed card cost ~10 Tab stops (thumb, play, info, title, channel, five actions), so +// the tenth card sat ~135 Tabs from the top of the page. `j`/`k` step from ITEM to item instead, the +// arrows move around a grid, and Enter opens — the same thing a click on the item does. +// +// How an item takes part, without this file knowing what an item is: +// - its root element carries `data-nav-index` (its index in the list — `navItemAttrs`), +// - the one element whose click means "open" carries `data-nav-open` (the title link on a video card), +// - the list's container registers with `useListNav`, saying how many items there are, how many +// columns a row has, and how to bring an unmounted item into the DOM (a virtualized grid). +// +// The Tab order is untouched: an item root is NOT a Tab stop. It gets `tabindex="-1"` at the moment +// the keyboard lands on it — one item at a time, the previous one gives it back — so Tab still walks +// the controls inside the cards, a mouse click on a card's blank area still focuses nothing, and the +// player's focus restore (E11 S2) can land back on the card it was opened from. +// +// Every page-level list shares ONE window listener, and the lists are walked in DOCUMENT order: `j` +// at the end of one list continues at the start of the next (owner decision D4, "reading order"). +// The arrows stay inside the list they are in. + +interface NavList { + readonly el: HTMLElement; + readonly count: () => number; + readonly cols: () => number; + readonly reveal: (index: number) => void; +} + +const lists = new Map(); +// The item we gave a tabindex to, so the next move can take it back. +let roved: HTMLElement | null = null; + +const INDEX = "data-nav-index"; +const LIST = "data-nav-list"; +// How long to wait for a virtualized item to mount after asking the list to reveal it. +const REVEAL_FRAMES = 10; + +/** Spread onto an item's ROOT element. */ +export const navItemAttrs = (index: number): { "data-nav-index": number } => ({ + [INDEX]: index, +}); + +const indexOf = (item: Element): number => Number(item.getAttribute(INDEX)); + +function itemAt(list: NavList, index: number): HTMLElement | null { + return list.el.querySelector(`[${INDEX}="${index}"]`); +} + +function ordered(): NavList[] { + return [...lists.values()] + .filter((l) => l.el.isConnected) + .sort((a, b) => + a.el.compareDocumentPosition(b.el) & Node.DOCUMENT_POSITION_FOLLOWING ? -1 : 1, + ); +} + +/** The item holding focus (on its root or on a control inside it), and the list it belongs to. */ +function current(): { list: NavList; index: number; item: HTMLElement } | null { + const item = (document.activeElement as HTMLElement | null)?.closest(`[${INDEX}]`); + const list = + item && lists.get(item.parentElement?.closest(`[${LIST}]`) as HTMLElement); + return item && list ? { list, index: indexOf(item), item } : null; +} + +function scrollParent(node: HTMLElement): HTMLElement | null { + for (let el = node.parentElement; el; el = el.parentElement) { + const oy = getComputedStyle(el).overflowY; + if (oy === "auto" || oy === "scroll") return el; + } + return null; +} + +/** Where `j` lands when nothing in a list has focus yet: the first item the reader can see, not the + * top of the list — after scrolling down a feed, the first press must not throw them back up. "Can + * see" = its bottom edge is below the top of the scroller's visible band, which starts under the + * floating header (the item's own `scroll-margin-top` is exactly that header clearance). */ +function firstVisible(): { list: NavList; index: number } | null { + let fallback: { list: NavList; index: number } | null = null; + for (const list of ordered()) { + const items = [...list.el.querySelectorAll(`[${INDEX}]`)] + .filter((el) => el.parentElement?.closest(`[${LIST}]`) === list.el) + .sort((a, b) => indexOf(a) - indexOf(b)); + if (!items.length) continue; + fallback ??= { list, index: indexOf(items[0]!) }; + const top = scrollParent(list.el)?.getBoundingClientRect().top ?? 0; + for (const el of items) { + const band = top + (parseFloat(getComputedStyle(el).scrollMarginTop) || 0); + if (el.getBoundingClientRect().bottom > band) return { list, index: indexOf(el) }; + } + } + return fallback; +} + +function focusItem(el: HTMLElement): void { + if (roved && roved !== el) roved.removeAttribute("tabindex"); + roved = null; + // An item that is focusable in its own right (a Plex card is a role=button) keeps its tabindex. + if (!el.hasAttribute("tabindex")) { + el.setAttribute("tabindex", "-1"); + roved = el; + } + el.focus({ preventScroll: true }); + // `nearest` + the item's scroll margins: a visible item does not move, an item under the floating + // header or past the bottom edge scrolls just far enough to clear it. + el.scrollIntoView?.({ block: "nearest" }); +} + +function focusIndex(list: NavList, index: number): void { + const el = itemAt(list, index); + if (el) return focusItem(el); + // Virtualized and not mounted: have the list scroll it in, then take it as soon as it renders. + list.reveal(index); + let frames = 0; + const wait = () => { + const got = itemAt(list, index); + if (got) focusItem(got); + else if (++frames < REVEAL_FRAMES) requestAnimationFrame(wait); + }; + requestAnimationFrame(wait); +} + +function step(delta: 1 | -1): void { + const cur = current(); + if (!cur) { + const start = firstVisible(); + if (start) focusIndex(start.list, start.index); + return; + } + const next = cur.index + delta; + if (next >= 0 && next < cur.list.count()) return focusIndex(cur.list, next); + // Past this list's end: on to the neighbouring list in reading order, skipping empty ones. At the + // first or last item of the page it stops — no wrap to the other end, no loading the next page. + const all = ordered(); + for (let at = all.indexOf(cur.list) + delta; at >= 0 && at < all.length; at += delta) { + const n = all[at]!.count(); + if (n > 0) return focusIndex(all[at]!, delta > 0 ? 0 : n - 1); + } +} + +/** Move inside the focused item's list. ←→ step one item (wrapping rows, the reading order); ↑↓ one + * row, landing on the last item when the row below is a short last row. Returns false when focus is + * not in a list, so the arrow keeps its native meaning (scrolling) there. */ +function move(dx: -1 | 0 | 1, dy: -1 | 0 | 1): boolean { + const cur = current(); + if (!cur) return false; + const count = cur.list.count(); + const cols = Math.max(1, cur.list.cols()); + const row = Math.floor(cur.index / cols); + let target = cur.index; + if (dx) target = cur.index + dx; + else if (dy > 0 && row < Math.floor((count - 1) / cols)) + target = Math.min(cur.index + cols, count - 1); + else if (dy < 0 && row > 0) target = cur.index - cols; + if (target !== cur.index && target >= 0 && target < count) focusIndex(cur.list, target); + return true; +} + +function onKey(e: KeyboardEvent): void { + const arrow = (dx: -1 | 0 | 1, dy: -1 | 0 | 1) => (ev: KeyboardEvent) => { + if (move(dx, dy)) ev.preventDefault(); + }; + dispatch(e, "list", { + next: (ev) => { + ev.preventDefault(); + step(1); + }, + prev: (ev) => { + ev.preventDefault(); + step(-1); + }, + left: arrow(-1, 0), + right: arrow(1, 0), + up: arrow(0, -1), + down: arrow(0, 1), + // Only on the item ROOT: Enter on a button inside the card must stay that button's Enter. + open: (ev) => { + const cur = current(); + if (!cur || document.activeElement !== cur.item) return; + const target = cur.item.querySelector("[data-nav-open]"); + if (!target) return; + ev.preventDefault(); + // A real click on the item's own "open" element, so Enter runs exactly the click path — not a + // second copy of what opening means that could drift from it. + target.click(); + }, + }); +} + +/** Register `ref` as a keyboard-navigable list while `enabled`. `count` is the number of items (the + * loaded ones, for an infinite list), `cols` the items per row (1 for a column), and `reveal` scrolls + * an item that is not mounted into view — omit it for a list that mounts everything. */ +export function useListNav( + ref: RefObject, + opts: { enabled: boolean; count: number; cols: number; reveal?: (index: number) => void }, +): void { + const optsRef = useRef(opts); + optsRef.current = opts; + useKeyScope("list", opts.enabled); + useEffect(() => { + const el = ref.current; + if (!opts.enabled || !el) return; + lists.set(el, { + el, + count: () => optsRef.current.count, + cols: () => optsRef.current.cols, + reveal: (i) => optsRef.current.reveal?.(i), + }); + el.setAttribute(LIST, ""); + if (lists.size === 1) window.addEventListener("keydown", onKey); + return () => { + lists.delete(el); + el.removeAttribute(LIST); + if (lists.size === 0) window.removeEventListener("keydown", onKey); + }; + }, [ref, opts.enabled]); +} diff --git a/frontend/src/lib/pageKeys.dom.test.tsx b/frontend/src/lib/pageKeys.dom.test.tsx new file mode 100644 index 00000000..526b6c9c --- /dev/null +++ b/frontend/src/lib/pageKeys.dom.test.tsx @@ -0,0 +1,94 @@ +// @vitest-environment jsdom +// +// The `page` scope as App binds it through `useGlobalKeymap`: the `g` chords jump to a module this +// account can reach, stay out of an overlay's way, and leave the global keys (Shift+W, h) intact. +// Its own file for the same reason as keymap.sequence.test.ts: a pending `g` is module state. +import { useEffect } from "react"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { act, cleanup, render } from "@testing-library/react"; +import { closeKeyboardHelp, useGlobalKeymap, useKeyboardHelpOpen } from "./keymap"; +import { pushLayer, removeLayer } from "./layerStack"; +import type { Page } from "./urlState"; + +let clock = Date.UTC(2031, 0, 1); +beforeEach(() => { + vi.useFakeTimers({ toFake: ["Date"] }); + clock += 3_600_000; + vi.setSystemTime(clock); +}); +afterEach(() => { + cleanup(); + closeKeyboardHelp(); + vi.useRealTimers(); +}); + +const ALL: Page[] = ["home", "feed", "channels", "playlists", "plex", "downloads", "settings"]; + +function Host({ + onNavigate, + pages = ALL, + onHelp = () => {}, +}: { + onNavigate: (p: Page) => void; + pages?: Page[]; + onHelp?: (open: boolean) => void; +}) { + useGlobalKeymap(true, onNavigate, pages); + const help = useKeyboardHelpOpen(); + useEffect(() => { + onHelp(help); + }, [help, onHelp]); + return null; +} + +const key = (init: KeyboardEventInit) => + act(() => { + window.dispatchEvent( + new KeyboardEvent("keydown", { bubbles: true, cancelable: true, ...init }), + ); + }); + +describe("the g chords", () => { + it("jump to the module named by the second key", () => { + const nav = vi.fn(); + render(); + key({ key: "f" }); + expect(nav).not.toHaveBeenCalled(); + key({ key: "g" }); + key({ key: "f" }); + key({ key: "g" }); + key({ key: "," }); + expect(nav.mock.calls).toEqual([["feed"], ["settings"]]); + }); + + // `h` alone opens the sheet; the `h` of `g h` must not ALSO do that. + it("take the h of g h away from the help sheet", () => { + const nav = vi.fn(); + let help = false; + render( (help = v)} />); + key({ key: "g" }); + key({ key: "h" }); + expect(nav).toHaveBeenCalledWith("home"); + expect(help).toBe(false); + }); + + it("refuse a module this account cannot reach", () => { + const nav = vi.fn(); + render(); + key({ key: "g" }); + key({ key: "x" }); + expect(nav).not.toHaveBeenCalled(); + }); + + it("stand aside for an overlay, while Shift+W still answers there", () => { + const nav = vi.fn(); + render(); + const id = pushLayer(); + key({ key: "g" }); + key({ key: "f" }); + expect(nav).not.toHaveBeenCalled(); + key({ key: "w", shiftKey: true }); + expect(nav).toHaveBeenCalledWith("home"); + removeLayer(id); + }); +}); From 75d89613d0c88162855620d2045fe1b7f83dda7d Mon Sep 17 00:00:00 2001 From: npeter83 Date: Sun, 13 Sep 2026 05:03:35 +0200 Subject: [PATCH 06/16] feat(palette): PageUp/PageDown move a page of results at a time MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The jump-to palette lists every module, channel, playlist, view and action — 461 rows on the owner's account, ~11 of them in view — and the arrows were the only way through it: forty presses to cross the channel group. - PageDown/PageUp step by the rows that fit in the list right now, minus one, so the row you paged from stays in sight at the new page's edge. Measured per press (the list is 50vh and group headings eat into it); a fixed 10 when there is no layout to measure. - Unlike the arrows they stop at the ends instead of wrapping: a page-sized jump that lands back at the top reads as having lost your place. - Home/End stay the text field's own. Measured in the browser: 0 → 10 → 20 rows, stops on row 461, PageUp from the end steps 9 (a group heading is in view), the selected row stays visible throughout. --- .../components/ui/CommandPalette.dom.test.tsx | 71 +++++++++++++++++++ frontend/src/components/ui/CommandPalette.tsx | 29 ++++++++ 2 files changed, 100 insertions(+) diff --git a/frontend/src/components/ui/CommandPalette.dom.test.tsx b/frontend/src/components/ui/CommandPalette.dom.test.tsx index 3a3abaca..f27e85aa 100644 --- a/frontend/src/components/ui/CommandPalette.dom.test.tsx +++ b/frontend/src/components/ui/CommandPalette.dom.test.tsx @@ -262,3 +262,74 @@ describe("CommandPalette · accessibility", () => { expect(status.textContent).toBe(""); }); }); + +// Owner request (E11): with 461 rows and ~11 in view, the arrows alone were a slog. PageUp/PageDown +// move by what fits in the list, and — unlike the arrows — stop at the ends instead of wrapping. +describe("CommandPalette · paging", () => { + const many = (): CommandGroup[] => [ + { + id: "rows", + label: "Rows", + items: Array.from({ length: 12 }, (_, i) => ({ + id: `r${i}`, + label: `Row ${i}`, + run: () => {}, + })), + }, + ]; + const openMany = () => render( {}} groups={many()} />); + const rect = (top: number, height: number) => + ({ + top, + bottom: top + height, + height, + left: 0, + right: 100, + width: 100, + x: 0, + y: top, + }) as DOMRect; + + it("pages by the rows in view minus one, and stops at both ends", () => { + // The list shows 100px and the rows are 30px tall: 3 fit, so a page is 2 rows. + const spy = vi.spyOn(Element.prototype, "getBoundingClientRect").mockImplementation(function ( + this: Element, + ) { + if (this.getAttribute("role") === "listbox") return rect(0, 100); + const all = [...document.querySelectorAll('[role="option"]')]; + return rect(all.indexOf(this) * 30, 30); + }); + // Restored in `finally`: a spy left behind by a failing assertion fails every later test too. + try { + openMany(); + press("PageDown"); + expect(selected()).toBe("r2"); + press("PageDown"); + expect(selected()).toBe("r4"); + for (let i = 0; i < 10; i++) press("PageDown"); + expect(selected()).toBe("r11"); // no wrap to the top + press("PageUp"); + expect(selected()).toBe("r9"); + for (let i = 0; i < 10; i++) press("PageUp"); + expect(selected()).toBe("r0"); + } finally { + spy.mockRestore(); + } + }); + + it("steps a fixed page when there is no layout to measure", () => { + openMany(); + press("PageDown"); + expect(selected()).toBe("r10"); + press("PageDown"); + expect(selected()).toBe("r11"); + }); + + it("leaves Home and End to the text field", () => { + openMany(); + press("PageDown"); + expect(fireEvent.keyDown(field(), { key: "Home" })).toBe(true); // not preventDefault-ed + expect(fireEvent.keyDown(field(), { key: "End" })).toBe(true); + expect(selected()).toBe("r10"); + }); +}); diff --git a/frontend/src/components/ui/CommandPalette.tsx b/frontend/src/components/ui/CommandPalette.tsx index 0102f038..1caea4db 100644 --- a/frontend/src/components/ui/CommandPalette.tsx +++ b/frontend/src/components/ui/CommandPalette.tsx @@ -71,6 +71,24 @@ function rankItem(item: CommandItem, groupLabel: string, words: string[]): Rank * every keystroke queues one announcement per letter. */ const ANNOUNCE_DELAY_MS = 400; +/** PageUp/PageDown step without layout to measure (jsdom, or a list not painted yet). */ +const PAGE_FALLBACK = 10; + +/** How far PageUp/PageDown move: the rows that fit in the scrolling list right now, minus one, so the + * row you paged from stays in sight as the new page's edge (a native listbox does the same). Measured + * rather than fixed: the list is 50vh tall and group headings eat into it. With 461 rows and ~11 in + * view, the arrows alone meant forty presses to cross the channel group (E11, owner request). */ +function pageStep(list: HTMLElement | null): number { + const box = list?.getBoundingClientRect(); + if (!list || !box || box.height === 0) return PAGE_FALLBACK; + let fits = 0; + for (const option of list.querySelectorAll('[role="option"]')) { + const r = option.getBoundingClientRect(); + if (r.top >= box.top - 0.5 && r.bottom <= box.bottom + 0.5) fits++; + } + return Math.max(1, fits - 1); +} + export default function CommandPalette({ open, onClose, @@ -91,6 +109,7 @@ export default function CommandPalette({ // Ref on the highlighted row rather than a selector lookup: item ids come from domain data // (playlist names, channel handles) and would need escaping to be safe in a query string. const activeRef = useRef(null); + const listRef = useRef(null); // A fresh open starts from a blank field: the palette is for "take me somewhere", and a leftover // query from last time hides everything you might want now. @@ -166,6 +185,15 @@ export default function CommandPalette({ const step = e.key === "ArrowDown" ? 1 : -1; const from = Math.max(activeIndex, 0); setPicked(flat[(from + step + flat.length) % flat.length]!.id); + } else if (e.key === "PageDown" || e.key === "PageUp") { + // A page at a time, and NO wrap: the arrows wrap because one step past the end is a small, + // obvious move, but a page-sized jump that lands back at the top reads as "it lost my place". + // Home/End stay the field's own (caret to the start/end of the query). + e.preventDefault(); + if (flat.length === 0) return; + const step = pageStep(listRef.current) * (e.key === "PageDown" ? 1 : -1); + const to = Math.min(flat.length - 1, Math.max(0, Math.max(activeIndex, 0) + step)); + setPicked(flat[to]!.id); } else if (e.key === "Enter") { const item = flat[activeIndex]; if (item) { @@ -208,6 +236,7 @@ export default function CommandPalette({ element; the "nothing matches" line sits OUTSIDE it, since a listbox may hold only options and groups. */}
Date: Sun, 13 Sep 2026 05:20:53 +0200 Subject: [PATCH 07/16] feat(ui): PageUp/PageDown page through every dropdown list, not just the palette The palette got PageUp/PageDown, and the same crawl stayed everywhere else a list opens under a control: the sidebar's channel filter, the Plex genre/studio/collection pickers, the filter and sort menus, submenus of every playlist you own. floating-ui's list navigation has only the arrows and Home/End. - `lib/listPaging`: the one definition of a page (the rows wholly in view minus one, a fixed 10 with no layout) and of where it lands (stops at both ends, never on a row it may not take, looking back toward the start rather than past it). The palette now uses it instead of its own copy, and `listNav` takes its scroll-parent helper from here. - `AnchoredPopover`: one keydown handler on the reference AND the surface, so a combobox (keys on its input, `aria-activedescendant`) and a roving menu (keys on the focused row) both page. It runs after floating-ui's own, which has by then switched to keyboard modality and scrolls the new row in. A disabled row is never the target; a submenu's key does not also page the menu around it; with no mounted rows (closed, a hover card, a dialog) the key keeps its native meaning. Measured in the browser: the channel filter shows 8 rows and pages by 7 with the active row in view after the scroll; a menu pages to its last row and back, the page behind it does not scroll; the palette still pages by 11 of its 12 visible rows. --- .../src/components/ui/ActionMenu.dom.test.tsx | 64 +++++++++++++++++++ .../src/components/ui/AnchoredPopover.tsx | 48 +++++++++++++- .../src/components/ui/Combobox.dom.test.tsx | 41 ++++++++++++ frontend/src/components/ui/CommandPalette.tsx | 36 ++++------- frontend/src/lib/listNav.ts | 9 +-- frontend/src/lib/listPaging.test.ts | 43 +++++++++++++ frontend/src/lib/listPaging.ts | 49 ++++++++++++++ 7 files changed, 255 insertions(+), 35 deletions(-) create mode 100644 frontend/src/lib/listPaging.test.ts create mode 100644 frontend/src/lib/listPaging.ts diff --git a/frontend/src/components/ui/ActionMenu.dom.test.tsx b/frontend/src/components/ui/ActionMenu.dom.test.tsx index 732f0454..b5b54ace 100644 --- a/frontend/src/components/ui/ActionMenu.dom.test.tsx +++ b/frontend/src/components/ui/ActionMenu.dom.test.tsx @@ -130,3 +130,67 @@ describe("ActionMenu", () => { expect(screen.getByTestId("custom-trigger")).toBeTruthy(); }); }); + +// PageUp/PageDown in a roving menu (E11, owner request) — the same page as the palette's and the +// combobox's (lib/listPaging); here the keyboard's row holds real DOM focus (the `tabindex=0` rover). +describe("ActionMenu · paging", () => { + it("moves the roving focus a page at a time, over disabled rows, stopping at the ends", () => { + renderMenu( + Array.from({ length: 25 }, (_, i) => ({ + id: `r${i}`, + label: `Row ${i}`, + onSelect: vi.fn(), + disabled: i === 20, + })), + ); + fireEvent.click(screen.getByRole("button", { name: "Catalog actions" })); + const menu = screen.getByRole("menu", { hidden: true }); + const rover = () => + screen + .getAllByRole("menuitem", { hidden: true }) + .find((el) => el.getAttribute("tabindex") === "0")?.textContent; + + expect(fireEvent.keyDown(menu, { key: "PageDown" })).toBe(false); + expect(rover()).toBe("Row 10"); + fireEvent.keyDown(menu, { key: "PageDown" }); + expect(rover()).toBe("Row 21"); // Row 20 is disabled + fireEvent.keyDown(menu, { key: "PageDown" }); + expect(rover()).toBe("Row 24"); + fireEvent.keyDown(menu, { key: "PageUp" }); + expect(rover()).toBe("Row 14"); + }); + + it("pages only the submenu the key was pressed in, not the menu around it", () => { + // A submenu's surface sits inside its parent's in the React tree, so the key bubbles through the + // parent's handler too; paging both would drag the focus back out to the parent's row. + renderMenu([ + { + id: "move", + label: "Move to", + items: Array.from({ length: 25 }, (_, i) => ({ + id: `s${i}`, + label: `Sub ${i}`, + onSelect: vi.fn(), + })), + }, + ...Array.from({ length: 12 }, (_, i) => ({ + id: `p${i}`, + label: `Top ${i}`, + onSelect: vi.fn(), + })), + ]); + fireEvent.click(screen.getByRole("button", { name: "Catalog actions" })); + fireEvent.click(screen.getByText("Move to")); + const rovers = () => + screen + .getAllByRole("menuitem", { hidden: true }) + .filter((el) => el.getAttribute("tabindex") === "0") + .map((el) => el.textContent ?? ""); + const outer = () => rovers().filter((label) => !label.startsWith("Sub")); + const before = outer(); + + fireEvent.keyDown(screen.getByText("Sub 0").closest("button")!, { key: "PageDown" }); + expect(rovers()).toContain("Sub 10"); + expect(outer()).toEqual(before); + }); +}); diff --git a/frontend/src/components/ui/AnchoredPopover.tsx b/frontend/src/components/ui/AnchoredPopover.tsx index 05323d9d..1b87842f 100644 --- a/frontend/src/components/ui/AnchoredPopover.tsx +++ b/frontend/src/components/ui/AnchoredPopover.tsx @@ -3,6 +3,7 @@ import { useContext, useEffect, useId, + useMemo, useRef, useState, type ButtonHTMLAttributes, @@ -37,6 +38,7 @@ import { } from "@floating-ui/react"; import { useLayer } from "../../lib/layerStack"; import { useFullscreenPortalHost } from "../../lib/fullscreen"; +import { pageStep, pageTarget, scrollParent } from "../../lib/listPaging"; // R8 popover platform. ONE anchored-popover primitive over @floating-ui/react, replacing the ~9 // hand-rolled positioning engines (measured flip / width clamp / rAF scroll-follow / scroller-relative @@ -52,7 +54,8 @@ import { useFullscreenPortalHost } from "../../lib/fullscreen"; // // S2 (this layer) adds the keyboard/ARIA model the roles promise, mirroring `ViewSwitcher` (the house's // textbook menu) but via Floating UI's idioms rather than a second hand-rolled roving engine: -// - useListNavigation → roving focus over the registered items (↑/↓/Home/End, loop), menu/listbox only +// - useListNavigation → roving focus over the registered items (↑/↓/Home/End, loop), menu/listbox only; +// PageUp/PageDown (a page, no loop) are ours, for every list surface incl. the combobox // - useTypeahead → type-to-focus a matching item by its label // - FloatingFocusManager → focus moves into the surface on open and RETURNS to the trigger on close // (a body-portaled popover sits at the end of the document; without this, Tab can't reach it and @@ -272,7 +275,48 @@ export function useAnchoredPopover(opts: AnchoredPopoverOptions) { }, }); - const interactions = useInteractions([click, hover, focus, dismiss, role, listNav, typeahead]); + // PageUp/PageDown: a page of rows at a time (lib/listPaging, shared with the palette). floating-ui's + // list navigation knows only the arrows and Home/End, and the arrows alone crawl through the sidebar's + // channel filter or a Plex studio list. On BOTH elements: a combobox's keys arrive on its input, a + // menu's on the focused row inside the surface. Last in the list, so floating-ui's own keydown has + // already switched to keyboard modality — which is what makes it scroll the new row into view. A + // nested menu's surface bubbles through its parent's in the React tree; the inner one handles the + // key first and marks it, so the outer one stands down. + const onPageKey = (e: React.KeyboardEvent) => { + if ((e.key !== "PageDown" && e.key !== "PageUp") || e.defaultPrevented) return; + const rows = elementsRef.current; + const live = rows.filter((el): el is HTMLElement => el != null); + // No mounted rows — a closed surface, a hover card, a dialog — and the key keeps its own meaning. + if (!live.length) return; + e.preventDefault(); + const step = pageStep(scrollParent(live[0]!), live); + const to = pageTarget( + activeIndex ?? 0, + e.key === "PageDown" ? step : -step, + rows.length, + (i) => !!rows[i] && !rows[i]!.hasAttribute("disabled"), + ); + if (to === null) return; + setActiveIndex(to); + setHoverIndex(null); + }; + const pageKeyRef = useRef(onPageKey); + pageKeyRef.current = onPageKey; + const paging = useMemo(() => { + const onKeyDown = (e: React.KeyboardEvent) => pageKeyRef.current(e); + return { reference: { onKeyDown }, floating: { onKeyDown } }; + }, []); + + const interactions = useInteractions([ + click, + hover, + focus, + dismiss, + role, + listNav, + typeahead, + paging, + ]); // `hide()` flips this true once the trigger is scrolled out of its clipping scroller; the surface is // then painted invisible (PopoverSurface) but stays open so it can reappear on scroll-back. diff --git a/frontend/src/components/ui/Combobox.dom.test.tsx b/frontend/src/components/ui/Combobox.dom.test.tsx index 19401cc5..30dab7a6 100644 --- a/frontend/src/components/ui/Combobox.dom.test.tsx +++ b/frontend/src/components/ui/Combobox.dom.test.tsx @@ -247,3 +247,44 @@ describe("Combobox", () => { expect(screen.getByRole("combobox").getAttribute("aria-expanded")).toBe("false"); // closed }); }); + +// PageUp/PageDown (E11, owner request): the sidebar's channel filter and the Plex pickers run to +// hundreds of rows, and floating-ui's list navigation has only the arrows. jsdom has no layout, so a +// page is the fixed fallback of 10 rows; the measured page is pinned in lib/listPaging. +describe("Combobox · paging", () => { + const rows = (disabled: number[] = []) => + Array.from({ length: 25 }, (_, i) => ({ value: `Row ${i}`, disabled: disabled.includes(i) })); + const activeValue = (input: HTMLElement) => + document.getElementById(input.getAttribute("aria-activedescendant") ?? "")?.textContent; + + it("moves the active option a page at a time, stopping at both ends", () => { + wrap(); + const input = screen.getByRole("combobox"); + input.focus(); + fireEvent.focus(input); + expect(fireEvent.keyDown(input, { key: "PageDown" })).toBe(false); // the page does not scroll + expect(activeValue(input)).toBe("Row 10"); + expect(document.activeElement).toBe(input); + fireEvent.keyDown(input, { key: "PageDown" }); + fireEvent.keyDown(input, { key: "PageDown" }); + expect(activeValue(input)).toBe("Row 24"); // no wrap to the top + fireEvent.keyDown(input, { key: "PageUp" }); + expect(activeValue(input)).toBe("Row 14"); + }); + + it("never lands on a disabled option, and Enter commits where it landed", () => { + const onChange = vi.fn(); + wrap(); + const input = screen.getByRole("combobox"); + input.focus(); + fireEvent.focus(input); + fireEvent.keyDown(input, { key: "PageDown" }); + fireEvent.keyDown(input, { key: "Enter" }); + expect(onChange).toHaveBeenCalledWith(["Row 11"]); + }); + + it("leaves the key alone while the list is closed", () => { + wrap(); + expect(fireEvent.keyDown(screen.getByRole("combobox"), { key: "PageDown" })).toBe(true); + }); +}); diff --git a/frontend/src/components/ui/CommandPalette.tsx b/frontend/src/components/ui/CommandPalette.tsx index 1caea4db..d21fddbc 100644 --- a/frontend/src/components/ui/CommandPalette.tsx +++ b/frontend/src/components/ui/CommandPalette.tsx @@ -3,6 +3,7 @@ import { useTranslation } from "react-i18next"; import { Search } from "lucide-react"; import Modal from "./Modal"; import { closeTopOverlayThenRun } from "../../lib/history"; +import { pageStep, pageTarget } from "../../lib/listPaging"; // The app's jump-to bar: one text field over every place you can go and every app-wide thing you can // do, opened with Ctrl/Cmd+K. It is the "one-layer operation" half of E10 — reaching a channel three @@ -71,24 +72,6 @@ function rankItem(item: CommandItem, groupLabel: string, words: string[]): Rank * every keystroke queues one announcement per letter. */ const ANNOUNCE_DELAY_MS = 400; -/** PageUp/PageDown step without layout to measure (jsdom, or a list not painted yet). */ -const PAGE_FALLBACK = 10; - -/** How far PageUp/PageDown move: the rows that fit in the scrolling list right now, minus one, so the - * row you paged from stays in sight as the new page's edge (a native listbox does the same). Measured - * rather than fixed: the list is 50vh tall and group headings eat into it. With 461 rows and ~11 in - * view, the arrows alone meant forty presses to cross the channel group (E11, owner request). */ -function pageStep(list: HTMLElement | null): number { - const box = list?.getBoundingClientRect(); - if (!list || !box || box.height === 0) return PAGE_FALLBACK; - let fits = 0; - for (const option of list.querySelectorAll('[role="option"]')) { - const r = option.getBoundingClientRect(); - if (r.top >= box.top - 0.5 && r.bottom <= box.bottom + 0.5) fits++; - } - return Math.max(1, fits - 1); -} - export default function CommandPalette({ open, onClose, @@ -186,14 +169,17 @@ export default function CommandPalette({ const from = Math.max(activeIndex, 0); setPicked(flat[(from + step + flat.length) % flat.length]!.id); } else if (e.key === "PageDown" || e.key === "PageUp") { - // A page at a time, and NO wrap: the arrows wrap because one step past the end is a small, - // obvious move, but a page-sized jump that lands back at the top reads as "it lost my place". - // Home/End stay the field's own (caret to the start/end of the query). + // A page at a time, stopping at the ends (lib/listPaging). The list is 50vh and group headings + // eat into it, so the page is measured. Home/End stay the field's own (caret to the query's ends). e.preventDefault(); - if (flat.length === 0) return; - const step = pageStep(listRef.current) * (e.key === "PageDown" ? 1 : -1); - const to = Math.min(flat.length - 1, Math.max(0, Math.max(activeIndex, 0) + step)); - setPicked(flat[to]!.id); + const list = listRef.current; + const step = pageStep(list, list?.querySelectorAll('[role="option"]') ?? []); + const to = pageTarget( + Math.max(activeIndex, 0), + e.key === "PageDown" ? step : -step, + flat.length, + ); + if (to !== null) setPicked(flat[to]!.id); } else if (e.key === "Enter") { const item = flat[activeIndex]; if (item) { diff --git a/frontend/src/lib/listNav.ts b/frontend/src/lib/listNav.ts index d8be132a..369ef486 100644 --- a/frontend/src/lib/listNav.ts +++ b/frontend/src/lib/listNav.ts @@ -1,5 +1,6 @@ import { useEffect, useRef, type RefObject } from "react"; import { dispatch, useKeyScope } from "./keymap"; +import { scrollParent } from "./listPaging"; // ── Keyboard navigation over lists and grids (E11 S4, UX-6) ────────────────────────────────────── // Before this, a feed card cost ~10 Tab stops (thumb, play, info, title, channel, five actions), so @@ -64,14 +65,6 @@ function current(): { list: NavList; index: number; item: HTMLElement } | null { return item && list ? { list, index: indexOf(item), item } : null; } -function scrollParent(node: HTMLElement): HTMLElement | null { - for (let el = node.parentElement; el; el = el.parentElement) { - const oy = getComputedStyle(el).overflowY; - if (oy === "auto" || oy === "scroll") return el; - } - return null; -} - /** Where `j` lands when nothing in a list has focus yet: the first item the reader can see, not the * top of the list — after scrolling down a feed, the first press must not throw them back up. "Can * see" = its bottom edge is below the top of the scroller's visible band, which starts under the diff --git a/frontend/src/lib/listPaging.test.ts b/frontend/src/lib/listPaging.test.ts new file mode 100644 index 00000000..f8900af5 --- /dev/null +++ b/frontend/src/lib/listPaging.test.ts @@ -0,0 +1,43 @@ +// One definition of a PageUp/PageDown page for the palette and every popover list (E11, owner +// request). The DOM-bound halves are covered where they are wired (CommandPalette, Combobox, +// ActionMenu); this pins the arithmetic both lean on. +import { describe, expect, it } from "vitest"; +import { PAGE_FALLBACK, pageStep, pageTarget } from "./listPaging"; + +const box = (top: number, height: number) => + ({ getBoundingClientRect: () => ({ top, bottom: top + height, height }) }) as unknown as Element; + +describe("pageStep", () => { + it("is the rows wholly in view minus one", () => { + // A 100px list with 30px rows: 0–30, 30–60, 60–90 fit; 90–120 is cut off. + const rows = [0, 30, 60, 90, 120].map((top) => box(top, 30)); + expect(pageStep(box(0, 100), rows)).toBe(2); + }); + + it("never stands still, and falls back without layout", () => { + expect(pageStep(box(0, 20), [box(0, 30)])).toBe(1); + expect(pageStep(box(0, 0), [box(0, 30)])).toBe(PAGE_FALLBACK); + expect(pageStep(null, [])).toBe(PAGE_FALLBACK); + }); +}); + +describe("pageTarget", () => { + it("moves a page and stops at both ends instead of wrapping", () => { + expect(pageTarget(0, 10, 25)).toBe(10); + expect(pageTarget(20, 10, 25)).toBe(24); + expect(pageTarget(24, 10, 25)).toBe(24); + expect(pageTarget(3, -10, 25)).toBe(0); + expect(pageTarget(0, 10, 0)).toBeNull(); + }); + + it("passes over a row it may not land on, then looks back — never past the start", () => { + const off = new Set([10, 11]); + const usable = (i: number) => !off.has(i); + expect(pageTarget(0, 10, 25, usable)).toBe(12); // on past the landing row first + off.add(24); + off.add(23); + expect(pageTarget(15, 10, 25, usable)).toBe(22); // at the end: back toward the start + expect(pageTarget(22, 10, 25, usable)).toBeNull(); // nothing between the start and the end + expect(pageTarget(15, -10, 25, (i) => i > 12)).toBe(13); + }); +}); diff --git a/frontend/src/lib/listPaging.ts b/frontend/src/lib/listPaging.ts new file mode 100644 index 00000000..78f9c033 --- /dev/null +++ b/frontend/src/lib/listPaging.ts @@ -0,0 +1,49 @@ +// ── PageUp/PageDown over a list the keyboard otherwise walks row by row (E11, owner request) ───── +// The jump-to palette (461 rows, ~11 in view) and every popover list — the sidebar's channel filter +// over hundreds of channels, the Plex pickers, the filter and sort menus — had only the arrows. One +// definition of "a page" here, so the palette and the dropdowns cannot drift apart. + +/** A page when there is no layout to measure (jsdom, or a list not painted yet). */ +export const PAGE_FALLBACK = 10; + +/** The nearest ancestor that scrolls vertically. */ +export function scrollParent(node: Element): HTMLElement | null { + for (let el = node.parentElement; el; el = el.parentElement) { + const oy = getComputedStyle(el).overflowY; + if (oy === "auto" || oy === "scroll") return el; + } + return null; +} + +/** How far one PageUp/PageDown moves: the rows that fit wholly inside `scroller` right now, minus one, + * so the row you paged from stays in sight at the new page's edge (a native listbox does the same). + * Measured rather than fixed: a list's height follows the viewport, and group headings eat into it. */ +export function pageStep(scroller: Element | null, rows: Iterable): number { + const box = scroller?.getBoundingClientRect(); + if (!box || box.height === 0) return PAGE_FALLBACK; + let fits = 0; + for (const row of rows) { + const r = row.getBoundingClientRect(); + if (r.top >= box.top - 0.5 && r.bottom <= box.bottom + 0.5) fits++; + } + return Math.max(1, fits - 1); +} + +/** Where a page move of `delta` from row `from` lands among `count` rows. It stops at both ends — no + * wrap, unlike the arrows: one step past the end is a small, obvious move, but a page-sized jump that + * lands back at the top reads as having lost your place. A row `usable` refuses (a disabled menu row) + * is never the target: the search goes on past the landing row first, then back toward the start, + * never beyond it. `null` = nowhere to go. */ +export function pageTarget( + from: number, + delta: number, + count: number, + usable: (index: number) => boolean = () => true, +): number | null { + if (count <= 0) return null; + const to = Math.min(count - 1, Math.max(0, from + delta)); + const dir = delta < 0 ? -1 : 1; + for (let i = to; i >= 0 && i < count; i += dir) if (usable(i)) return i; + for (let i = to - dir; (i - from) * dir > 0; i -= dir) if (usable(i)) return i; + return null; +} From 6d09b94bbb3ff1cd52822a298af5a0c475471331 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Sun, 13 Sep 2026 05:38:36 +0200 Subject: [PATCH 08/16] feat(keys): j/k and the arrows walk Home's rails and every Plex grid and rail MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second slice of the E11 keyboard layer (S4b). S4a gave the feed, the library and the channel page `j`/`k`, the arrows and Enter; Home's shelves and the whole Plex module were still Tab-stop country — a Plex poster is its own Tab stop, and a rail's cards sat behind every control of the cards before. - `lib/listNav`: a list that mounts every item no longer declares a count or columns — both are measured off the DOM when a key is pressed, so an auto-fill grid's columns follow its width and a rail is one row. ↑ on a list's top row and ↓ on its bottom row go on to the neighbouring list, onto the item horizontally nearest the one left (owner decision D4: ←→ within a rail, ↑↓ to the rail above or below). The first `j` skips cards a rail has scrolled out to the left. - `Carousel keyboardNav` registers the scroller; the scroller gets a 4px inset (against a -4px margin, layout unchanged) so the focus ring is not clipped, and `[data-nav-index]` a 2.5rem inline scroll margin so a card stepped into view clears the rail's edge fade and paging arrow. - `ui/NavGrid`: the plain, fully mounted grid that registers itself. - `navIndex` on PlexPosterCard / SeasonCard / EpisodeCard (the card root is the item, the poster the open element — the ring goes round poster and title, D2), PosterTile and Home's ShelfTile (the button is the item); cast, related and collection-strip buttons carry the attrs directly. - Wired: Home's comfort and resume rails; Plex Browse titles + episodes, Discover rails + genres, collection view, playlists/collections browse, person known-for + filmography, show seasons, season episodes, cast/related strips, info-page cast + collection strips. - `listNav.wiring.contract.test`: every `Carousel` says `keyboardNav`, every card says `navIndex`, and a card with `navIndex` sits inside a NavGrid / Carousel / VirtualGrid — both halves are opt-in, and either one missing leaves a list the keyboard silently skips. --- CLAUDE.md | 2 +- frontend/src/components/Home.dom.test.tsx | 44 ++++++ frontend/src/components/Home.tsx | 11 +- frontend/src/components/PlexBrowse.tsx | 15 +- .../src/components/PlexCollectionView.tsx | 8 +- .../src/components/PlexDiscover.dom.test.tsx | 39 ++++- frontend/src/components/PlexDiscover.tsx | 19 ++- frontend/src/components/PlexInfo.tsx | 13 +- frontend/src/components/PosterTile.tsx | 4 + .../listNav.wiring.contract.test.ts | 138 ++++++++++++++++++ frontend/src/components/plexBrowseLists.tsx | 15 +- .../src/components/plexCards.dom.test.tsx | 49 ++++++- frontend/src/components/plexCards.tsx | 24 ++- frontend/src/components/plexViews.tsx | 31 ++-- .../src/components/ui/Carousel.dom.test.tsx | 38 ++++- frontend/src/components/ui/Carousel.tsx | 24 ++- frontend/src/components/ui/NavGrid.tsx | 23 +++ frontend/src/components/ui/README.md | 2 +- frontend/src/index.css | 2 + frontend/src/lib/listNav.dom.test.tsx | 96 +++++++++++- frontend/src/lib/listNav.ts | 83 +++++++++-- 21 files changed, 615 insertions(+), 65 deletions(-) create mode 100644 frontend/src/components/listNav.wiring.contract.test.ts create mode 100644 frontend/src/components/ui/NavGrid.tsx diff --git a/CLAUDE.md b/CLAUDE.md index b93f9bfc..42c11893 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -90,7 +90,7 @@ CQRS, a repository/DI framework, a router library, a generic CRUD-router factory | FE per-account state | `usePref` | raw `readAccount`/`writeAccount` or bespoke hooks | | FE action button | `ui/Button` — pick a `SIZES` rung | a hand-rolled ` + ))} +
+ ); + } + render(); + press("j"); + expect(focused()).toBe("t0"); + expect(screen.getByTestId("t0").hasAttribute("tabindex")).toBe(false); + press("j"); + expect(focused()).toBe("t1"); + expect(screen.getByTestId("t1").hasAttribute("tabindex")).toBe(false); + }); + + // Landing again on the item that already holds the roved tabindex (focus went to in between) + // forgot it, so it kept tabindex="-1" for good — and a click on its blank area focused the card. + it("gives the tabindex back after landing twice on the same item", () => { + render(); + press("j"); + expect(focused()).toBe("a0"); + act(() => (document.activeElement as HTMLElement).blur()); + press("j"); + expect(focused()).toBe("a0"); + press("j"); + expect(focused()).toBe("a1"); + expect(screen.getByTestId("a0").hasAttribute("tabindex")).toBe(false); + }); + + // Hiding the focused feed card unmounts it and drops focus to ; the next `j` started over at + // the first visible card instead of going on from the hidden card's slot. + it("goes on from the slot of an item that was removed under the focus", () => { + function Removable() { + const [ids, setIds] = useState([0, 1, 2, 3]); + const ref = useRef(null); + useListNav(ref, { enabled: true }); + return ( +
+ {ids.map((id, i) => ( +
+ +
+ ))} +
+ ); + } + withLayout( + (_, n) => ({ left: 0, top: n * 100 }), + () => { + render(); + press("j"); + press("j"); + press("j"); + expect(focused()).toBe("r2"); + act(() => screen.getByTestId("r2-hide").focus()); + fireEvent.click(screen.getByTestId("r2-hide")); + expect(document.activeElement).toBe(document.body); + press("j"); + expect(focused()).toBe("r3"); + }, + ); + }); +}); diff --git a/frontend/src/lib/listNav.ts b/frontend/src/lib/listNav.ts index 1e1a1bdd..fa3cf1ec 100644 --- a/frontend/src/lib/listNav.ts +++ b/frontend/src/lib/listNav.ts @@ -120,13 +120,44 @@ function firstVisible(): { list: NavList; index: number } | null { return fallback; } +/** The item focus was last inside, so a `j` after that item was REMOVED (a feed card hidden, a + * download deleted — focus falls to ) goes on from its slot instead of starting over. */ +let last: { list: NavList; index: number; el: HTMLElement } | null = null; + +function onFocusIn(e: FocusEvent): void { + const item = (e.target as HTMLElement | null)?.closest?.(`[${INDEX}]`); + const list = + item && lists.get(item.parentElement?.closest(`[${LIST}]`) as HTMLElement); + if (item && list) last = { list, index: indexOf(item), el: item }; +} + +/** Where `j` (`k`: the item before) lands after the last-focused item left the DOM: the item now in + * its slot, if that slot is on screen. An item that only scrolled out of a virtualized list has left + * the DOM too, but its slot is off screen then, so the reader's viewport (`firstVisible`) wins. */ +function slotOfRemoved(delta: 1 | -1): { list: NavList; index: number } | null { + if (!last || last.el.isConnected || lists.get(last.list.el) !== last.list) return null; + const n = last.list.count(); + const index = Math.min(Math.max(0, delta > 0 ? last.index : last.index - 1), n - 1); + const el = index >= 0 ? itemAt(last.list, index) : null; + if (!el) return null; + const box = scrollParent(last.list.el)?.getBoundingClientRect(); + const r = el.getBoundingClientRect(); + const band = (box?.top ?? 0) + (parseFloat(getComputedStyle(el).scrollMarginTop) || 0); + return r.bottom > band && r.top < (box?.bottom ?? window.innerHeight) + ? { list: last.list, index } + : null; +} + function focusItem(el: HTMLElement, reveal = true): void { - if (roved && roved !== el) roved.removeAttribute("tabindex"); - roved = null; - // An item that is focusable in its own right (a Plex card is a role=button) keeps its tabindex. - if (!el.hasAttribute("tabindex")) { - el.setAttribute("tabindex", "-1"); - roved = el; + if (roved !== el) { + roved?.removeAttribute("tabindex"); + roved = null; + // An item that is a Tab stop in its own right (a native + + ))} + + ); + } + + // `s`, a query, Escape: the feed stays mounted with new results and the old card is gone. The slot + // kept for a REMOVED card sent the next `j` to the third new result instead of the first. + it("starts from the first visible item after focus went on to a field and the items changed", () => { + withLayout( + (_, n) => ({ left: 0, top: n * 100 }), + () => { + const page = (ids: number[]) => ( + <> + + + + ); + const { rerender } = render(page([0, 1, 2, 3])); + press("j"); + press("j"); + press("j"); + expect(focused()).toBe("r2"); + act(() => screen.getByTestId("field").focus()); + rerender(page([4, 5, 6, 7])); + act(() => screen.getByTestId("field").blur()); + press("j"); + expect(focused()).toBe("r4"); + }, + ); + }); + + // A confirm (or the item's own menu) takes the focus before the delete it confirms: back from it, the + // next `j` still goes on from the deleted row's slot. + it("keeps the slot when the removal was confirmed in a dialog", () => { + withLayout( + (_, n) => ({ left: 0, top: n * 100 }), + () => { + const page = (ids: number[]) => ( + <> +
+ +
+ + + ); + const { rerender } = render(page([0, 1, 2, 3])); + press("j"); + press("j"); + press("j"); + act(() => screen.getByTestId("ok").focus()); + rerender(page([0, 1, 3])); + act(() => screen.getByTestId("ok").blur()); + press("j"); + expect(focused()).toBe("r3"); + }, + ); + }); + + // A card root is a plain
: focused by `j` it had no role and no name, so a screen reader read + // an unlabelled run of text. + it("names a focused card after what it opens while it holds the focus, then gives the name back", () => { + render(); + press("j"); + const a0 = screen.getByTestId("a0"); + expect(a0.getAttribute("role")).toBe("group"); + expect(a0.getAttribute("aria-label")).toBe("title"); + press("j"); + expect(a0.hasAttribute("role")).toBe(false); + expect(a0.hasAttribute("aria-label")).toBe(false); + expect(screen.getByTestId("a1").getAttribute("aria-label")).toBe("title"); + }); +}); diff --git a/frontend/src/lib/listNav.scope.dom.test.tsx b/frontend/src/lib/listNav.scope.dom.test.tsx new file mode 100644 index 00000000..49af4087 --- /dev/null +++ b/frontend/src/lib/listNav.scope.dom.test.tsx @@ -0,0 +1,54 @@ +// @vitest-environment jsdom +// +// The `list` key scope — what puts j/k, the arrows and Enter on the cheat sheet — is live only while a +// registered list has an item to land on. An empty download queue, a list still loading or one that +// failed mounts its container all the same, and the sheet listed the keys over nothing (deep-review +// E11 round 2). Its own file: listNav keeps module state. +import { useRef } from "react"; +import { afterEach, describe, expect, it } from "vitest"; +import { cleanup, render, screen, waitFor } from "@testing-library/react"; +import { navItemAttrs, useListNav } from "./listNav"; +import { useActiveScopes } from "./keymap"; + +afterEach(cleanup); + +function Scopes() { + return {useActiveScopes().join(" ")}; +} + +function Queue({ count }: { count: number }) { + const ref = useRef(null); + useListNav(ref, { enabled: true }); + return ( +
+ {count === 0 ? ( +

Nothing queued

+ ) : ( + Array.from({ length: count }, (_, i) => ( +
+ item +
+ )) + )} +
+ ); +} + +const page = (count: number) => ( + <> + + + +); +const scopes = () => screen.getByTestId("scopes").textContent!.split(" "); + +describe("useListNav · the list key scope", () => { + it("is not live over a list with nothing in it, and is while it has items", async () => { + const { rerender } = render(page(0)); + expect(scopes()).not.toContain("list"); + rerender(page(2)); + await waitFor(() => expect(scopes()).toContain("list")); + rerender(page(0)); + await waitFor(() => expect(scopes()).not.toContain("list")); + }); +}); diff --git a/frontend/src/lib/listNav.ts b/frontend/src/lib/listNav.ts index fa3cf1ec..dda79144 100644 --- a/frontend/src/lib/listNav.ts +++ b/frontend/src/lib/listNav.ts @@ -1,4 +1,4 @@ -import { useEffect, useRef, type RefObject } from "react"; +import { useEffect, useRef, useState, type RefObject } from "react"; import { dispatch, useKeyScope } from "./keymap"; import { scrollParent } from "./listPaging"; @@ -35,8 +35,9 @@ interface NavList { } const lists = new Map(); -// The item we gave a tabindex to, so the next move can take it back. +// The item we gave a tabindex to, so the next move can take it back — and whether we named it too. let roved: HTMLElement | null = null; +let named = false; const INDEX = "data-nav-index"; const LIST = "data-nav-list"; @@ -125,10 +126,17 @@ function firstVisible(): { list: NavList; index: number } | null { let last: { list: NavList; index: number; el: HTMLElement } | null = null; function onFocusIn(e: FocusEvent): void { - const item = (e.target as HTMLElement | null)?.closest?.(`[${INDEX}]`); + const target = e.target as HTMLElement | null; + const item = target?.closest?.(`[${INDEX}]`); const list = item && lists.get(item.parentElement?.closest(`[${LIST}]`) as HTMLElement); if (item && list) last = { list, index: indexOf(item), el: item }; + // Focus went on to page content that is not an item — the search box, a filter chip: the reader + // left the list, and whatever it shows next (new results) is not "the slot they were in" (deep-review + // E11 round 2). A layer opened FROM the item — its menu, a confirm — does not count: focus comes + // back from it, and what it did (a hidden card, a deleted row) is the removal the slot is kept for. + // Removing the focused node itself fires no focusin. + else if (!target?.closest?.('[data-floating-ui-portal], [role="dialog"]')) last = null; } /** Where `j` (`k`: the item before) lands after the last-focused item left the DOM: the item now in @@ -150,13 +158,31 @@ function slotOfRemoved(delta: 1 | -1): { list: NavList; index: number } | null { function focusItem(el: HTMLElement, reveal = true): void { if (roved !== el) { + if (named) roved?.removeAttribute("role"); + if (named) roved?.removeAttribute("aria-label"); roved?.removeAttribute("tabindex"); roved = null; + named = false; // An item that is a Tab stop in its own right (a native +
+ + + ); + const { rerender } = render(page([0, 1, 2, 3])); + press("j"); + press("j"); + press("j"); + rerender(page([0, 1, 3])); + act(() => screen.getByTestId("undo").focus()); + rerender(page([0, 1, 2, 3])); + act(() => screen.getByTestId("undo").blur()); + press("j"); + expect(focused()).toBe("r2"); + }, + ); + }); }); diff --git a/frontend/src/lib/listNav.ts b/frontend/src/lib/listNav.ts index dda79144..330fe48e 100644 --- a/frontend/src/lib/listNav.ts +++ b/frontend/src/lib/listNav.ts @@ -44,9 +44,16 @@ const LIST = "data-nav-list"; // How long to wait for a virtualized item to mount after asking the list to reveal it. const REVEAL_FRAMES = 10; -/** Spread onto an item's ROOT element. */ -export const navItemAttrs = (index: number): { "data-nav-index": number } => ({ +/** Spread onto an item's ROOT element. `name` is what a screen reader calls the item while the keyboard + * is on it (its title) — pass it where the element Enter opens wraps the whole card (a Plex poster: + * badges, year, rating and summary run together), shows a caption that is not the item's name, or + * holds no text at all (deep-review E11 round 3). */ +export const navItemAttrs = ( + index: number, + name?: string | null, +): { "data-nav-index": number; "data-nav-name"?: string } => ({ [INDEX]: index, + ...(name ? { "data-nav-name": name } : null), }); const indexOf = (item: Element): number => Number(item.getAttribute(INDEX)); @@ -133,10 +140,11 @@ function onFocusIn(e: FocusEvent): void { if (item && list) last = { list, index: indexOf(item), el: item }; // Focus went on to page content that is not an item — the search box, a filter chip: the reader // left the list, and whatever it shows next (new results) is not "the slot they were in" (deep-review - // E11 round 2). A layer opened FROM the item — its menu, a confirm — does not count: focus comes - // back from it, and what it did (a hidden card, a deleted row) is the removal the slot is kept for. - // Removing the focused node itself fires no focusin. - else if (!target?.closest?.('[data-floating-ui-portal], [role="dialog"]')) last = null; + // E11 round 2). A layer opened FROM the item — its menu, a confirm, the Undo toast its removal put + // up — does not count: focus comes back from it, and what it did (a hidden card, a deleted row, the + // card put back) is what the slot is kept for. Removing the focused node itself fires no focusin. + else if (!target?.closest?.('[data-floating-ui-portal], [role="dialog"], [data-toast-region]')) + last = null; } /** Where `j` (`k`: the item before) lands after the last-focused item left the DOM: the item now in @@ -171,8 +179,15 @@ function focusItem(el: HTMLElement, reveal = true): void { // A card root is a plain
: focused, it has no role and no name, and a screen reader read it // as an unlabelled run of text. While it holds the focus it is a group named after what Enter // opens (deep-review E11 round 2). A row () keeps its own role. + // The item's own name when it gives one (`navItemAttrs`), else the open element's label or text: + // a title link's text is the title (deep-review E11 round 3). const open = el.querySelector("[data-nav-open]"); - const name = (open?.getAttribute("aria-label") || open?.textContent || "").trim(); + const name = ( + el.getAttribute("data-nav-name") || + open?.getAttribute("aria-label") || + open?.textContent || + "" + ).trim(); if ( el.tagName === "DIV" && name && From 4c7ab12a3ebaf6550c8015618654fb77a6280093 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Sun, 13 Sep 2026 21:02:22 +0200 Subject: [PATCH 16/16] =?UTF-8?q?release:=200.85.0=20=E2=80=94=20E11=20Pow?= =?UTF-8?q?er=20Navigation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Back, dialogs and menus keep your place, and the keyboard walks every list: five sprints plus three epic-end review rounds (19 → 14, 18 → 12, 14 → 14). Minor: j/k list navigation, s and g-chords, and Back-to-item are features. --- CHANGELOG.md | 25 +++++++++++++++++++++++++ VERSION | 2 +- frontend/src/lib/releaseNotes.ts | 24 ++++++++++++++++++++++++ 3 files changed, 50 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5a533e01..93de23a2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,31 @@ edit the release notes and re-run the generator. Upgrading, pinning a version and rolling back: [docs/self-hosting.md](docs/self-hosting.md#upgrading). +## 0.85.0 — 2026-09-13 + +Siftlode can now be driven from the keyboard, and Back, dialogs and menus stop losing your place. j/k and the arrow keys walk every list and grid, Enter opens what you are on, and Back puts you on the item you opened a view from instead of at the top of the page. + +### Features + +- j/k and the arrow keys walk the feed, the library, a channel page, the Home rails, every Plex grid and rail, the Channels, Playlists and Downloads tables and the conversation list; Enter opens the item you are on. After you hide or delete an item, the next j goes on from where it was. +- Back returns you to the item a view was opened from — the Plex title you opened from a Home rail, the channel you opened from its row — with the keyboard still on it. +- Shortcuts to get around: s jumps to the search field, g-chords jump to a module, and the cheat sheet (h) names every key in the language you are reading. PageUp/PageDown move a page at a time in the command palette and in every dropdown list. +- The command palette ranks what you typed first: “theme” now puts the Theme rows above a channel whose handle happens to contain it. +- A card reached with Tab or j shows one ring around the whole card, and a screen reader calls it by its title. + +### Fixes + +- Back and overlays keep their books straight: Back after navigating over About also closes About, a late clean-up no longer undoes the navigation you just made, and Shift+W on Home closes the player. +- Dialogs and menus hand the keyboard back where it came from. Escape from a card’s context menu returns to the card, not its thumbnail link; closing the player under another dialog no longer leaves the page unscrollable; a button that is busy with a request keeps the focus instead of dropping it to the page. +- A write that fails says so, with the reason — several saves, share links and admin actions used to fail silently or in English under the Hungarian interface. +- A split video export that fails part-way (the job limit) tells you how many clips were added and leaves only the rest selected, so trying again does not queue the first ones twice. +- “Feed” is called Feed everywhere, in both languages. + +### Internal + +- Five sprints: history integrity, one dialog/focus contract, palette ranking and accessibility, the keyboard layer, and a Hungarian/English keyboard-only pass. Focus and event-order questions were measured in a real browser before each fix, not argued. +- Three epic-end review rounds — 19 → 14, 18 → 12 and a targeted 14 → 14 — each regression test mutation-checked. The last round's findings all sat inside the previous round's own fixes, which is where the loop was closed. + ## 0.84.0 — 2026-09-12 A bug-fix release, from a list you wrote while using Siftlode. Two of these were destroying your own data: a YouTube video rewound itself when you reopened it, and clearing a resume point on a half-watched Plex item told Plex nothing. Subtitles now appear in seconds instead of minutes, the Plex player's bitrate is a real measurement rather than a guess, and a channel you turn full history on for fills in front of you instead of looking frozen. diff --git a/VERSION b/VERSION index 09c49413..137c1281 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.84.0 +0.85.0 diff --git a/frontend/src/lib/releaseNotes.ts b/frontend/src/lib/releaseNotes.ts index cd3303e9..65288f7f 100644 --- a/frontend/src/lib/releaseNotes.ts +++ b/frontend/src/lib/releaseNotes.ts @@ -14,6 +14,30 @@ export interface ReleaseEntry { } export const RELEASE_NOTES: ReleaseEntry[] = [ + { + version: "0.85.0", + date: "2026-09-13", + summary: + "Siftlode can now be driven from the keyboard, and Back, dialogs and menus stop losing your place. j/k and the arrow keys walk every list and grid, Enter opens what you are on, and Back puts you on the item you opened a view from instead of at the top of the page.", + features: [ + "j/k and the arrow keys walk the feed, the library, a channel page, the Home rails, every Plex grid and rail, the Channels, Playlists and Downloads tables and the conversation list; Enter opens the item you are on. After you hide or delete an item, the next j goes on from where it was.", + "Back returns you to the item a view was opened from \u2014 the Plex title you opened from a Home rail, the channel you opened from its row \u2014 with the keyboard still on it.", + "Shortcuts to get around: s jumps to the search field, g-chords jump to a module, and the cheat sheet (h) names every key in the language you are reading. PageUp/PageDown move a page at a time in the command palette and in every dropdown list.", + "The command palette ranks what you typed first: \u201ctheme\u201d now puts the Theme rows above a channel whose handle happens to contain it.", + "A card reached with Tab or j shows one ring around the whole card, and a screen reader calls it by its title.", + ], + fixes: [ + "Back and overlays keep their books straight: Back after navigating over About also closes About, a late clean-up no longer undoes the navigation you just made, and Shift+W on Home closes the player.", + "Dialogs and menus hand the keyboard back where it came from. Escape from a card\u2019s context menu returns to the card, not its thumbnail link; closing the player under another dialog no longer leaves the page unscrollable; a button that is busy with a request keeps the focus instead of dropping it to the page.", + "A write that fails says so, with the reason \u2014 several saves, share links and admin actions used to fail silently or in English under the Hungarian interface.", + "A split video export that fails part-way (the job limit) tells you how many clips were added and leaves only the rest selected, so trying again does not queue the first ones twice.", + "\u201cFeed\u201d is called Feed everywhere, in both languages.", + ], + chores: [ + "Five sprints: history integrity, one dialog/focus contract, palette ranking and accessibility, the keyboard layer, and a Hungarian/English keyboard-only pass. Focus and event-order questions were measured in a real browser before each fix, not argued.", + "Three epic-end review rounds \u2014 19 \u2192 14, 18 \u2192 12 and a targeted 14 \u2192 14 \u2014 each regression test mutation-checked. The last round's findings all sat inside the previous round's own fixes, which is where the loop was closed.", + ], + }, { version: "0.84.0", date: "2026-09-12",