From fa8bdf83a887933af40b42f16b0c37d8a6f67def Mon Sep 17 00:00:00 2001 From: npeter83 Date: Thu, 20 Aug 2026 05:12:33 +0200 Subject: [PATCH 1/8] test(e4): Modal focus-trap coverage (FE-9 deferred gap) Backfills the E4 deep-review's deferred Modal focus-management test. jsdom returns null for every offsetParent (no layout), so focusablesIn() would see zero controls; the test stubs HTMLElement.prototype.offsetParent to report "visible" for connected nodes (what a real browser reports for the shown dialog), and a second describe documents the raw-jsdom empty-guard path. Covers: initial focus on open, Tab/Shift+Tab edge-wrap, pull-back on escaped focus, the active===panel leak guard, no-trap in the interior, Escape closes the topmost only, and restore-to-trigger on unmount. useBackToClose is mocked (its history stack is covered by history.dom.test.ts). 10 tests; full FE gate green (tsc/eslint/prettier/knip, 257 vitest). --- frontend/src/components/Modal.dom.test.tsx | 191 +++++++++++++++++++++ 1 file changed, 191 insertions(+) create mode 100644 frontend/src/components/Modal.dom.test.tsx diff --git a/frontend/src/components/Modal.dom.test.tsx b/frontend/src/components/Modal.dom.test.tsx new file mode 100644 index 00000000..d622314c --- /dev/null +++ b/frontend/src/components/Modal.dom.test.tsx @@ -0,0 +1,191 @@ +// @vitest-environment jsdom +// +// Modal: focus management (FE-9). This was DEFERRED from the E4 deep-review because it's +// jsdom-untestable as written — `focusablesIn` filters on `el.offsetParent !== null`, and jsdom +// returns null for EVERY element (it never lays out), so the trap would see zero focusables and do +// nothing. We stub `HTMLElement.prototype.offsetParent` to report "visible" (non-null) for any +// connected node, which is exactly what a real browser reports for the shown dialog controls; the +// separate no-stub describe below documents the raw-jsdom reality (empty → the guard no-ops). +// +// The overlay/history machinery (useBackToClose) is exercised in history.dom.test.ts, so it's mocked +// out here to isolate the focus-trap behaviour from its module-global history side-effects. +import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; +import { render, screen, act, cleanup } from "@testing-library/react"; + +vi.mock("react-i18next", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, useTranslation: () => ({ t: (key: string) => key }) }; +}); +// Isolate the focus trap from the Back-to-close history stack (tested in history.dom.test.ts). +vi.mock("../lib/history", () => ({ useBackToClose: () => {} })); + +import Modal from "./Modal"; + +// Fire a keydown on window (where Modal installs its handler), inside act. +const press = (key: string, shiftKey = false) => + act(() => { + window.dispatchEvent( + new KeyboardEvent("keydown", { key, shiftKey, bubbles: true, cancelable: true }), + ); + }); + +const panelEl = () => document.querySelector('[tabindex="-1"]')!; +const closeBtn = () => screen.getByRole("button", { name: "common.close" }); + +// A modal with three extra focusable controls; DOM order inside the panel is [close, A, input, B]. +function Body() { + return ( + <> + + + + + ); +} + +describe("Modal focus trap (offsetParent stubbed 'visible')", () => { + let origDesc: PropertyDescriptor | undefined; + beforeEach(() => { + origDesc = Object.getOwnPropertyDescriptor(HTMLElement.prototype, "offsetParent"); + Object.defineProperty(HTMLElement.prototype, "offsetParent", { + configurable: true, + get(this: HTMLElement) { + return this.isConnected ? document.body : null; + }, + }); + }); + afterEach(() => { + cleanup(); + if (origDesc) Object.defineProperty(HTMLElement.prototype, "offsetParent", origDesc); + else delete (HTMLElement.prototype as unknown as Record).offsetParent; + }); + + it("moves initial focus to the first control on open", () => { + render( + {}}> + + , + ); + expect(document.activeElement).toBe(closeBtn()); + }); + + it("Tab from the last control wraps to the first", () => { + render( + {}}> + + , + ); + screen.getByText("beta").focus(); // last + press("Tab"); + expect(document.activeElement).toBe(closeBtn()); + }); + + it("Shift+Tab from the first control wraps to the last", () => { + render( + {}}> + + , + ); + closeBtn().focus(); // first + press("Tab", true); + expect(document.activeElement).toBe(screen.getByText("beta")); + }); + + it("Tab pulls focus back inside when it has escaped to the page behind", () => { + const outside = document.createElement("button"); + document.body.appendChild(outside); + render( + {}}> + + , + ); + outside.focus(); // focus escaped the dialog + expect(document.activeElement).toBe(outside); + press("Tab"); + expect(document.activeElement).toBe(closeBtn()); + document.body.removeChild(outside); + }); + + it("Shift+Tab from the panel container itself goes to the last control (no leak to the background)", () => { + render( + {}}> + + , + ); + panelEl().focus(); // active === panel (tabIndex=-1, reached by clicking dead space) + press("Tab", true); + expect(document.activeElement).toBe(screen.getByText("beta")); + }); + + it("does not trap a Tab in the middle — only the edges wrap", () => { + render( + {}}> + + , + ); + const mid = screen.getByLabelText("field"); + mid.focus(); + press("Tab"); // interior: native Tab handles it, the trap must not force-move + expect(document.activeElement).toBe(mid); + }); + + it("Escape calls onClose", () => { + const onClose = vi.fn(); + render( + + + , + ); + press("Escape"); + expect(onClose).toHaveBeenCalledTimes(1); + }); + + it("Escape closes only the topmost modal", () => { + const outerClose = vi.fn(); + const innerClose = vi.fn(); + render( + + + , + ); + render( + + + , + ); + press("Escape"); + expect(innerClose).toHaveBeenCalledTimes(1); + expect(outerClose).not.toHaveBeenCalled(); + }); + + it("restores focus to the trigger on unmount", () => { + const trigger = document.createElement("button"); + document.body.appendChild(trigger); + trigger.focus(); // captured as restoreTo during Modal's first render + const { unmount } = render( + {}}> + + , + ); + expect(document.activeElement).not.toBe(trigger); // focus moved into the dialog + unmount(); + expect(document.activeElement).toBe(trigger); + document.body.removeChild(trigger); + }); +}); + +describe("Modal focus trap (raw jsdom — every offsetParent null)", () => { + afterEach(cleanup); + + it("with no visible focusables: initial focus falls to the panel and Tab is a safe no-op", () => { + render( + {}}> + + , + ); + // focusablesIn() returns [] in raw jsdom, so the initializer focuses the panel itself. + expect(document.activeElement).toBe(panelEl()); + expect(() => press("Tab")).not.toThrow(); + expect(document.activeElement).toBe(panelEl()); + }); +}); From e8a1c252a9ae1bf0689c40a5277b15533e004a30 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Thu, 20 Aug 2026 05:14:12 +0200 Subject: [PATCH 2/8] test(e4): useHistorySubview seed + popstate URL-fallback (S2 fix) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Backfills the E4 deep-review's deferred coverage for the subview history primitive. The keeper case is the S2 bookmark fix: a popstate that lands on a URL-only entry (no history.state._sub) must restore the sub-view from the ?v param via the SAME fallback the mount seed uses, not collapse to root. Covers: seed from URL vs state._sub vs root, the no-SubviewUrl state-only path, open() push+mirror + encode→null key-clear, and popstate restore from state / from URL (the fix) / collapse-to-root. 9 tests, gate green. --- frontend/src/lib/history.subview.dom.test.ts | 104 +++++++++++++++++++ 1 file changed, 104 insertions(+) create mode 100644 frontend/src/lib/history.subview.dom.test.ts diff --git a/frontend/src/lib/history.subview.dom.test.ts b/frontend/src/lib/history.subview.dom.test.ts new file mode 100644 index 00000000..75603f4f --- /dev/null +++ b/frontend/src/lib/history.subview.dom.test.ts @@ -0,0 +1,104 @@ +// @vitest-environment jsdom +// +// useHistorySubview: a module's active sub-view rides in history.state._sub, with optional address-bar +// mirroring so a deep sub-view (Plex show/season/info/person/playlist) is bookmarkable. This backfills +// the E4 deep-review's deferred coverage for the S2 fix — the seed AND the popstate handler must share +// the SAME URL fallback, so a popstate that lands on a bookmark entry (URL-only, no _sub — e.g. after +// an overlay's programmatic history.back()) restores the sub-view from the URL instead of collapsing +// it to root. The overlay/back-ladder half of history.ts is covered in history.dom.test.ts. +import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; +import { renderHook, act } from "@testing-library/react"; +import { useHistorySubview, type SubviewUrl } from "./history"; + +// A minimal string-view mirror: "root" is the un-mirrored root (encode → null), anything else mirrors +// verbatim to ?v=. decode rejects the empty string (→ null, i.e. not a valid sub-view). +const url: SubviewUrl = { + key: "v", + encode: (v) => (v === "root" ? null : v), + decode: (raw) => raw || null, +}; + +const setUrl = (search: string, state: unknown = null) => + window.history.replaceState(state, "", search || "/"); +const popTo = (search: string, state: unknown = null) => + act(() => { + window.history.replaceState(state, "", search || "/"); + window.dispatchEvent(new PopStateEvent("popstate", { state })); + }); + +beforeEach(() => setUrl("/")); +afterEach(() => vi.clearAllMocks()); + +describe("useHistorySubview seed", () => { + it("decodes the ? URL param when the entry has no _sub (fresh bookmark)", () => { + setUrl("?v=a"); + const { result } = renderHook(() => useHistorySubview("root", url)); + expect(result.current.view).toBe("a"); + }); + + it("prefers history.state._sub over the URL param (in-app entry)", () => { + setUrl("?v=a", { _sub: "b" }); + const { result } = renderHook(() => useHistorySubview("root", url)); + expect(result.current.view).toBe("b"); + }); + + it("falls back to root when neither state nor a decodable URL param is present", () => { + setUrl("/"); + const { result } = renderHook(() => useHistorySubview("root", url)); + expect(result.current.view).toBe("root"); + }); + + it("ignores the URL entirely when no SubviewUrl is supplied (state-only mirror)", () => { + setUrl("?v=a", { _sub: "fromState" }); + const { result } = renderHook(() => useHistorySubview("root")); + expect(result.current.view).toBe("fromState"); + // A URL param with no url config must NOT leak in. + setUrl("?v=a", null); + const { result: r2 } = renderHook(() => useHistorySubview("root")); + expect(r2.current.view).toBe("root"); + }); +}); + +describe("useHistorySubview open", () => { + it("pushes state._sub and mirrors the URL via encode", () => { + const { result } = renderHook(() => useHistorySubview("root", url)); + act(() => result.current.open("a")); + expect(result.current.view).toBe("a"); + expect(window.history.state?._sub).toBe("a"); + expect(window.location.search).toBe("?v=a"); + }); + + it("clears the mirrored key when encode returns null (root/ephemeral view)", () => { + setUrl("?v=a"); + const { result } = renderHook(() => useHistorySubview("root", url)); + act(() => result.current.open("root")); + expect(result.current.view).toBe("root"); + expect(window.history.state?._sub).toBe("root"); + expect(window.location.search).toBe(""); // ?v dropped + }); +}); + +describe("useHistorySubview popstate", () => { + it("restores the sub-view from state._sub on Back/Forward", () => { + const { result } = renderHook(() => useHistorySubview("root", url)); + popTo("?v=a", { _sub: "a" }); + expect(result.current.view).toBe("a"); + }); + + it("restores from the ? URL when the landed entry has no _sub — the S2 bookmark fix", () => { + // Start in-app at root, then a popstate lands on a URL-only bookmark entry (no _sub). Reading + // history.state alone would wrongly collapse to root; the shared URL fallback must decode ?v=b. + const { result } = renderHook(() => useHistorySubview("root", url)); + expect(result.current.view).toBe("root"); + popTo("?v=b", null); + expect(result.current.view).toBe("b"); + }); + + it("collapses to root when a popstate lands on an entry with neither _sub nor a decodable URL", () => { + setUrl("?v=a", { _sub: "a" }); + const { result } = renderHook(() => useHistorySubview("root", url)); + expect(result.current.view).toBe("a"); + popTo("/", null); + expect(result.current.view).toBe("root"); + }); +}); From 0401dae689be981a2832870fd55219e01cecaca6 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Thu, 20 Aug 2026 05:16:06 +0200 Subject: [PATCH 3/8] test(e4): extract PLEX_SUB_URL for round-trip coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move `type Sub` + `PLEX_SUB_URL` out of the 538-line PlexBrowse into a small plexSubUrl.ts so the encode/decode round-trip and its guards can be unit-tested without dragging the component (and its lazy hls.js player) into the test graph. Pure code motion — PlexBrowse imports the two symbols; the `SubviewUrl` import moves with them. Tests lock: encode of each bookmarkable kind, grid/player → null, encode→decode identity, a person name containing ':' round-tripping, and the decode guards (positive-integer playlist id; season needs both ids; non-empty show/info/person arg; unknown kind/garbage → null). 12 tests; full FE gate green (tsc/eslint/prettier/knip). --- frontend/src/components/PlexBrowse.tsx | 60 +------------------ frontend/src/components/plexSubUrl.test.ts | 69 ++++++++++++++++++++++ frontend/src/components/plexSubUrl.ts | 61 +++++++++++++++++++ 3 files changed, 132 insertions(+), 58 deletions(-) create mode 100644 frontend/src/components/plexSubUrl.test.ts create mode 100644 frontend/src/components/plexSubUrl.ts diff --git a/frontend/src/components/PlexBrowse.tsx b/frontend/src/components/PlexBrowse.tsx index 9fe25528..545d4770 100644 --- a/frontend/src/components/PlexBrowse.tsx +++ b/frontend/src/components/PlexBrowse.tsx @@ -21,7 +21,8 @@ import { } from "../lib/api"; import { plexLang } from "../i18n"; import { useDebounced } from "../lib/useDebounced"; -import { useHistorySubview, type SubviewUrl } from "../lib/history"; +import { useHistorySubview } from "../lib/history"; +import { PLEX_SUB_URL, type Sub } from "./plexSubUrl"; import { toggleWatchState } from "../lib/plexWatch"; import SortControl from "./SortControl"; import { PageToolbar } from "./PageShell"; @@ -36,63 +37,6 @@ import { PlexInfoView, PlexPersonView, PlexSeasonView, PlexShowView } from "./pl const PlexPlayer = lazy(() => import("./PlexPlayer")); const PlexPlaylistView = lazy(() => import("./PlexPlaylistView")); -type Sub = - | { kind: "grid" } - | { kind: "show"; id: string } - | { kind: "season"; showId: string; seasonId: string } - | { kind: "player"; id: string; queue?: string[] } - | { kind: "info"; id: string } - | { kind: "person"; name: string } - | { kind: "playlist"; id: number }; - -// Mirror the Plex sub-view to ?v=: so a show / season / movie-info / person / playlist page -// is bookmarkable. Grid (root) and the ephemeral player are not mirrored (encode → null). The person -// name is the whole tail after the first ":", so a name containing ":" round-trips. -const PLEX_SUB_URL: SubviewUrl = { - key: "v", - encode: (v) => { - switch (v.kind) { - case "show": - return `show:${v.id}`; - case "season": - return `season:${v.showId}:${v.seasonId}`; - case "info": - return `info:${v.id}`; - case "person": - return `person:${v.name}`; - case "playlist": - return `playlist:${v.id}`; - default: - return null; // grid + the ephemeral player are not URL-mirrored - } - }, - decode: (raw) => { - const i = raw.indexOf(":"); - const kind = i === -1 ? raw : raw.slice(0, i); - const arg = i === -1 ? "" : raw.slice(i + 1); - switch (kind) { - case "show": - return arg ? { kind: "show", id: arg } : null; - case "info": - return arg ? { kind: "info", id: arg } : null; - case "person": - return arg ? { kind: "person", name: arg } : null; - case "playlist": { - // Positive-integer id only — a fractional/zero/negative ?v=playlist: would open a - // sub-view for a non-existent id (matches the ?pl guard in PlaylistsProvider.readSelectedId). - const id = arg ? Number(arg) : NaN; - return Number.isInteger(id) && id > 0 ? { kind: "playlist", id } : null; - } - case "season": { - const [showId, seasonId] = arg.split(":"); - return showId && seasonId ? { kind: "season", showId, seasonId } : null; - } - default: - return null; - } - }, -}; - // The Plex module's content area: browse/search the mirrored library as poster cards, drill from a // show into seasons/episodes. Library scope / watch-state / sort live in the left PlexSidebar; the // search term comes from the shared top Header search box (q). A show drill-down rides history diff --git a/frontend/src/components/plexSubUrl.test.ts b/frontend/src/components/plexSubUrl.test.ts new file mode 100644 index 00000000..5a153802 --- /dev/null +++ b/frontend/src/components/plexSubUrl.test.ts @@ -0,0 +1,69 @@ +// PLEX_SUB_URL encode/decode round-trip + guards. Extracted from PlexBrowse for this test (E4 +// deep-review deferred gap). Pure — no DOM, no history. +import { describe, it, expect } from "vitest"; +import { PLEX_SUB_URL as U, type Sub } from "./plexSubUrl"; + +describe("PLEX_SUB_URL encode", () => { + it("mirrors each bookmarkable sub-view to ?v=:", () => { + expect(U.encode({ kind: "show", id: "s1" })).toBe("show:s1"); + expect(U.encode({ kind: "season", showId: "s1", seasonId: "se2" })).toBe("season:s1:se2"); + expect(U.encode({ kind: "info", id: "m9" })).toBe("info:m9"); + expect(U.encode({ kind: "person", name: "Jane Doe" })).toBe("person:Jane Doe"); + expect(U.encode({ kind: "playlist", id: 12 })).toBe("playlist:12"); + }); + + it("does NOT mirror the root grid or the ephemeral player (encode → null)", () => { + expect(U.encode({ kind: "grid" })).toBeNull(); + expect(U.encode({ kind: "player", id: "m9" })).toBeNull(); + }); +}); + +describe("PLEX_SUB_URL round-trip", () => { + const cases: Sub[] = [ + { kind: "show", id: "s1" }, + { kind: "season", showId: "s1", seasonId: "se2" }, + { kind: "info", id: "m9" }, + { kind: "person", name: "Jane Doe" }, + { kind: "playlist", id: 12 }, + ]; + it.each(cases)("encode→decode is identity for %o", (view) => { + const raw = U.encode(view); + expect(raw).not.toBeNull(); + expect(U.decode(raw as string)).toEqual(view); + }); + + it("a person name containing ':' round-trips (tail after the first colon)", () => { + const view: Sub = { kind: "person", name: "Dr: Strange: Origins" }; + expect(U.decode(U.encode(view) as string)).toEqual(view); + }); +}); + +describe("PLEX_SUB_URL decode guards", () => { + it("rejects a playlist id that is not a positive integer", () => { + expect(U.decode("playlist:")).toBeNull(); // Number("")===0, not >0 + expect(U.decode("playlist:0")).toBeNull(); + expect(U.decode("playlist:-1")).toBeNull(); + expect(U.decode("playlist:1.5")).toBeNull(); + expect(U.decode("playlist:abc")).toBeNull(); + expect(U.decode("playlist:12")).toEqual({ kind: "playlist", id: 12 }); + }); + + it("requires both ids for a season", () => { + expect(U.decode("season:s1")).toBeNull(); + expect(U.decode("season:")).toBeNull(); + expect(U.decode("season:s1:se2")).toEqual({ kind: "season", showId: "s1", seasonId: "se2" }); + }); + + it("requires a non-empty arg for show / info / person", () => { + expect(U.decode("show:")).toBeNull(); + expect(U.decode("info:")).toBeNull(); + expect(U.decode("person:")).toBeNull(); + }); + + it("rejects an unknown kind or garbage", () => { + expect(U.decode("grid")).toBeNull(); + expect(U.decode("player:m9")).toBeNull(); // player is not URL-restorable + expect(U.decode("bogus")).toBeNull(); + expect(U.decode("")).toBeNull(); + }); +}); diff --git a/frontend/src/components/plexSubUrl.ts b/frontend/src/components/plexSubUrl.ts new file mode 100644 index 00000000..d3fc2816 --- /dev/null +++ b/frontend/src/components/plexSubUrl.ts @@ -0,0 +1,61 @@ +import type { SubviewUrl } from "../lib/history"; + +// The Plex module's sub-view union and its address-bar mirror, extracted from PlexBrowse so the +// encode/decode round-trip (and its guards) can be unit-tested without pulling the whole component +// (and its lazy hls.js player) into the test graph. +export type Sub = + | { kind: "grid" } + | { kind: "show"; id: string } + | { kind: "season"; showId: string; seasonId: string } + | { kind: "player"; id: string; queue?: string[] } + | { kind: "info"; id: string } + | { kind: "person"; name: string } + | { kind: "playlist"; id: number }; + +// Mirror the Plex sub-view to ?v=: so a show / season / movie-info / person / playlist page +// is bookmarkable. Grid (root) and the ephemeral player are not mirrored (encode → null). The person +// name is the whole tail after the first ":", so a name containing ":" round-trips. +export const PLEX_SUB_URL: SubviewUrl = { + key: "v", + encode: (v) => { + switch (v.kind) { + case "show": + return `show:${v.id}`; + case "season": + return `season:${v.showId}:${v.seasonId}`; + case "info": + return `info:${v.id}`; + case "person": + return `person:${v.name}`; + case "playlist": + return `playlist:${v.id}`; + default: + return null; // grid + the ephemeral player are not URL-mirrored + } + }, + decode: (raw) => { + const i = raw.indexOf(":"); + const kind = i === -1 ? raw : raw.slice(0, i); + const arg = i === -1 ? "" : raw.slice(i + 1); + switch (kind) { + case "show": + return arg ? { kind: "show", id: arg } : null; + case "info": + return arg ? { kind: "info", id: arg } : null; + case "person": + return arg ? { kind: "person", name: arg } : null; + case "playlist": { + // Positive-integer id only — a fractional/zero/negative ?v=playlist: would open a + // sub-view for a non-existent id (matches the ?pl guard in PlaylistsProvider.readSelectedId). + const id = arg ? Number(arg) : NaN; + return Number.isInteger(id) && id > 0 ? { kind: "playlist", id } : null; + } + case "season": { + const [showId, seasonId] = arg.split(":"); + return showId && seasonId ? { kind: "season", showId, seasonId } : null; + } + default: + return null; + } + }, +}; From 062015ba27d3a72c4047898815dffdb3617c64ac Mon Sep 17 00:00:00 2001 From: npeter83 Date: Thu, 20 Aug 2026 05:21:13 +0200 Subject: [PATCH 4/8] test(e4): loadInitialPage precedence + channelViewFrom (S1 fix) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Split the two URL-derived nav seeds out of NavigationProvider into navigationInit.ts so they're unit-testable and the provider stays react-refresh-clean (component-only exports). channelViewFrom also dedups the identical _chan/?chan resolve the channelView seed and the onPop handler each hand-rolled — locking the S1 fix (a popstate on a URL-only bookmark entry must restore the channel from ?chan, not close it). Pure code motion. Tests: the full loadInitialPage precedence matrix (?page > ?v > ?pl > filters/?chan > stored landing > feed, with the beats-a-lower-param cases) and channelViewFrom (_chan+name, ?chan fallback, state-over-url, closed → null). 15 tests; full FE gate green (tsc/eslint/prettier/knip). --- .../src/components/NavigationProvider.tsx | 48 +++------- .../src/components/navigation.dom.test.ts | 95 +++++++++++++++++++ frontend/src/components/navigationInit.ts | 39 ++++++++ 3 files changed, 145 insertions(+), 37 deletions(-) create mode 100644 frontend/src/components/navigation.dom.test.ts create mode 100644 frontend/src/components/navigationInit.ts diff --git a/frontend/src/components/NavigationProvider.tsx b/frontend/src/components/NavigationProvider.tsx index 336984a5..869d293d 100644 --- a/frontend/src/components/NavigationProvider.tsx +++ b/frontend/src/components/NavigationProvider.tsx @@ -9,11 +9,12 @@ import { type ReactNode, } from "react"; import { useTranslation } from "react-i18next"; -import { hasFilterParams, isPage, readPage, urlWithParams, type Page } from "../lib/urlState"; +import { urlWithParams, type Page } from "../lib/urlState"; import { pageTitleKey } from "../lib/pageMeta"; -import { getAccountRaw, LS, setAccountRaw } from "../lib/storage"; +import { LS, setAccountRaw } from "../lib/storage"; import { useAccountRehydrate } from "../lib/useAccountRehydrate"; import { setNavigator } from "../lib/nav"; +import { channelViewFrom, loadInitialPage, type ChannelView } from "./navigationInit"; // Owns the app's navigation + browser-history state, lifted out of the App god component so pages // consume it via a hook instead of prop-drilling `page`/`setPage`/`onOpenChannel` through the tree. @@ -28,11 +29,6 @@ import { setNavigator } from "../lib/nav"; const PAGE_KEY = LS.page; -export interface ChannelView { - id: string; - name?: string; -} - export interface NavigationState { page: Page; /** The open channel page (null = none). Overlays `page`; rides in history.state._chan. */ @@ -49,19 +45,6 @@ export interface NavigationActions { exitYtSearch: () => void; } -function loadInitialPage(): Page { - const params = new URLSearchParams(window.location.search); - if (params.has("page")) return readPage(); - // A view-specific param implies its page, so a bookmarked deep link lands on the right module even - // without an explicit ?page. Feed filters (or a channel overlay) imply the feed. - if (params.has("v")) return "plex"; - if (params.has("pl")) return "playlists"; - if (hasFilterParams(params) || params.has("chan")) return "feed"; - // A bare URL → the account's stored landing page. - const stored = getAccountRaw(PAGE_KEY); - return isPage(stored) ? stored : "feed"; -} - const StateContext = createContext({ page: "feed", channelView: null, @@ -97,14 +80,9 @@ export function NavigationProvider({ children }: { children: ReactNode }) { // detail loads) AND is mirrored to the address bar as ?chan=, so Back closes it, a reload restores // it, and a bookmarked ?chan link opens it (unlike ytSearch, which is dropped). Reload of an in-app // entry reads history.state._chan; a fresh bookmark has only the ?chan param. - const [channelView, setChannelView] = useState(() => { - const st = window.history.state; - const id = - (st?._chan as string | undefined) ?? - new URLSearchParams(window.location.search).get("chan") ?? - undefined; - return id ? { id, name: (st?._chanName as string) ?? undefined } : null; - }); + const [channelView, setChannelView] = useState(() => + channelViewFrom(window.history.state, window.location.search), + ); // Refs so the STABLE action callbacks can read the latest state without depending on it — a dep // would recreate them on every navigation and re-render every actions consumer (e.g. VideoCard). @@ -244,15 +222,11 @@ export function NavigationProvider({ children }: { children: ReactNode }) { // The YouTube-search sub-view and the channel page ride in history.state, so Back/Forward // restores or clears them to match the entry we landed on. setYtSearch((e.state?._yt as string) ?? null); - // Fall back to the ?chan URL param (the browser restored the landed entry's URL) when the entry - // has no _chan — a fresh ?chan bookmark's entry is URL-only, and reading e.state alone would - // wrongly close the channel when a popstate lands here (e.g. after an overlay's programmatic - // history.back()). Mirrors the channelView initializer's URL fallback. - const chan = - (e.state?._chan as string | undefined) ?? - new URLSearchParams(window.location.search).get("chan") ?? - undefined; - setChannelView(chan ? { id: chan, name: (e.state?._chanName as string) ?? undefined } : null); + // The channel page rides in history.state._chan, with a ?chan URL fallback for a fresh bookmark + // entry (URL-only, no _chan) — reading e.state alone would wrongly close the channel when a + // popstate lands here (e.g. after an overlay's programmatic history.back()). channelViewFrom + // shares this resolution with the seed initializer above. + setChannelView(channelViewFrom(e.state, window.location.search)); } window.addEventListener("popstate", onPop); return () => window.removeEventListener("popstate", onPop); diff --git a/frontend/src/components/navigation.dom.test.ts b/frontend/src/components/navigation.dom.test.ts new file mode 100644 index 00000000..9a0bc15e --- /dev/null +++ b/frontend/src/components/navigation.dom.test.ts @@ -0,0 +1,95 @@ +// @vitest-environment jsdom +// +// NavigationProvider's URL-derived seeds, backfilling the E4 deep-review's deferred coverage: +// • loadInitialPage — the bookmark deep-link precedence (?page > ?v > ?pl > filters/?chan > the +// account's stored landing page > feed). +// • channelViewFrom — the shared _chan/?chan resolver behind BOTH the channelView seed and the +// onPop handler (the S1 fix): a popstate landing on a URL-only bookmark entry must restore the +// channel from ?chan, not close it. +// Only getAccountRaw is stubbed (the account's stored landing page); everything else is real. +import { describe, it, expect, beforeEach, vi } from "vitest"; + +const { stored } = vi.hoisted(() => ({ stored: {} as Record })); +vi.mock("../lib/storage", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, getAccountRaw: (k: string) => stored[k] ?? null }; +}); + +import { loadInitialPage, channelViewFrom } from "./navigationInit"; +import { LS } from "../lib/storage"; + +const setUrl = (search: string) => window.history.replaceState(null, "", search || "/"); + +beforeEach(() => { + setUrl("/"); + for (const k of Object.keys(stored)) delete stored[k]; +}); + +describe("loadInitialPage precedence", () => { + it("?page wins and is validated", () => { + setUrl("?page=stats"); + expect(loadInitialPage()).toBe("stats"); + }); + it("an invalid ?page falls back to feed", () => { + setUrl("?page=bogus"); + expect(loadInitialPage()).toBe("feed"); + }); + it("?v implies the plex module", () => { + setUrl("?v=show:1"); + expect(loadInitialPage()).toBe("plex"); + }); + it("?pl implies playlists", () => { + setUrl("?pl=3"); + expect(loadInitialPage()).toBe("playlists"); + }); + it("a filter param implies the feed", () => { + setUrl("?tags=1,2"); + expect(loadInitialPage()).toBe("feed"); + }); + it("?chan (the channel overlay) implies the feed", () => { + setUrl("?chan=UC1"); + expect(loadInitialPage()).toBe("feed"); + }); + it("?page beats a lower-precedence param", () => { + setUrl("?page=stats&v=show:1"); + expect(loadInitialPage()).toBe("stats"); + }); + it("?v beats ?pl and filters", () => { + setUrl("?v=show:1&pl=3&tags=1"); + expect(loadInitialPage()).toBe("plex"); + }); + it("a bare URL uses the account's stored landing page", () => { + stored[LS.page] = "downloads"; + expect(loadInitialPage()).toBe("downloads"); + }); + it("a bare URL with no / invalid stored page defaults to feed", () => { + expect(loadInitialPage()).toBe("feed"); + stored[LS.page] = "nope"; + expect(loadInitialPage()).toBe("feed"); + }); +}); + +describe("channelViewFrom", () => { + it("reads _chan (+ _chanName) from an in-app entry", () => { + expect(channelViewFrom({ _chan: "UC1", _chanName: "Chan" }, "")).toEqual({ + id: "UC1", + name: "Chan", + }); + }); + it("_chan without a name yields an undefined name", () => { + expect(channelViewFrom({ _chan: "UC1" }, "")).toEqual({ id: "UC1", name: undefined }); + }); + it("falls back to the ?chan URL param when the entry has no _chan (fresh bookmark) — the S1 fix", () => { + expect(channelViewFrom(null, "?chan=UC9")).toEqual({ id: "UC9", name: undefined }); + }); + it("prefers _chan over the URL param", () => { + expect(channelViewFrom({ _chan: "UCstate" }, "?chan=UCurl")).toEqual({ + id: "UCstate", + name: undefined, + }); + }); + it("returns null when neither is present (channel closed)", () => { + expect(channelViewFrom({}, "")).toBeNull(); + expect(channelViewFrom(null, "/")).toBeNull(); + }); +}); diff --git a/frontend/src/components/navigationInit.ts b/frontend/src/components/navigationInit.ts new file mode 100644 index 00000000..a57b0c03 --- /dev/null +++ b/frontend/src/components/navigationInit.ts @@ -0,0 +1,39 @@ +// URL-derived navigation seeds, split out of NavigationProvider so they stay unit-testable AND the +// provider file keeps react-refresh happy (component-only exports). `loadInitialPage` picks the landing +// module from a bookmark's params; `channelViewFrom` is the shared _chan/?chan resolver behind BOTH the +// channelView seed and the onPop handler. +import { hasFilterParams, isPage, readPage, type Page } from "../lib/urlState"; +import { getAccountRaw, LS } from "../lib/storage"; + +const PAGE_KEY = LS.page; + +export interface ChannelView { + id: string; + name?: string; +} + +export function loadInitialPage(): Page { + const params = new URLSearchParams(window.location.search); + if (params.has("page")) return readPage(); + // A view-specific param implies its page, so a bookmarked deep link lands on the right module even + // without an explicit ?page. Feed filters (or a channel overlay) imply the feed. + if (params.has("v")) return "plex"; + if (params.has("pl")) return "playlists"; + if (hasFilterParams(params) || params.has("chan")) return "feed"; + // A bare URL → the account's stored landing page. + const stored = getAccountRaw(PAGE_KEY); + return isPage(stored) ? stored : "feed"; +} + +/** Resolve the open channel page from a landed history entry: the in-app `_chan` id (with its optional + * `_chanName`), else the ?chan URL param — a fresh ?chan bookmark's entry is URL-only. The seed AND + * the popstate handler share this ONE resolver so a popstate that lands on a bookmark entry (no + * `_chan`) restores the channel from the URL instead of wrongly closing it. */ +export function channelViewFrom( + state: { _chan?: unknown; _chanName?: unknown } | null | undefined, + search: string, +): ChannelView | null { + const id = + (state?._chan as string | undefined) ?? new URLSearchParams(search).get("chan") ?? undefined; + return id ? { id, name: (state?._chanName as string) ?? undefined } : null; +} From d2f2085f1f9edc6fcfb9621901cd63ac86d579f6 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Thu, 20 Aug 2026 05:24:11 +0200 Subject: [PATCH 5/8] test(e4): readSelectedId ?pl guard, useUrlMirror dep-keying, usePref flush MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The remaining E4 deep-review deferred gaps, in one sprint: - readSelectedId split into playlistsInit.ts (react-refresh-clean, testable) — ?pl accepted only as a positive integer; empty/0/negative/fractional/non-numeric fall back to the stored selection; null when neither is present. - useUrlMirror: mounts write the param, a same-serialized rerender does NOT re-fire (stable dep), a null value clears the key, other params preserved. - usePref: a pending debounced write is FLUSHED on unmount (not dropped); an unmount with nothing pending stays silent. 20 new test cases; full FE gate green (tsc/eslint/prettier/knip, 309 vitest). --- frontend/src/components/PlaylistsProvider.tsx | 14 +---- frontend/src/components/playlistsInit.test.ts | 54 +++++++++++++++++++ frontend/src/components/playlistsInit.ts | 14 +++++ frontend/src/lib/usePref.dom.test.ts | 23 ++++++++ frontend/src/lib/useUrlMirror.dom.test.ts | 51 ++++++++++++++++++ 5 files changed, 144 insertions(+), 12 deletions(-) create mode 100644 frontend/src/components/playlistsInit.test.ts create mode 100644 frontend/src/components/playlistsInit.ts create mode 100644 frontend/src/lib/useUrlMirror.dom.test.ts diff --git a/frontend/src/components/PlaylistsProvider.tsx b/frontend/src/components/PlaylistsProvider.tsx index e837da9c..4ddf94fe 100644 --- a/frontend/src/components/PlaylistsProvider.tsx +++ b/frontend/src/components/PlaylistsProvider.tsx @@ -1,8 +1,9 @@ import { createContext, useCallback, useContext, useMemo, useState, type ReactNode } from "react"; -import { getAccountRaw, LS, setAccountRaw } from "../lib/storage"; +import { LS, setAccountRaw } from "../lib/storage"; import { useAccountRehydrate } from "../lib/useAccountRehydrate"; import { useUrlMirror } from "../lib/useUrlMirror"; import { useNavigation } from "./NavigationProvider"; +import { readSelectedId } from "./playlistsInit"; // Owns the Playlists module's shared local state — the rail's name filter and the selected playlist // — lifted out of the App god component so the header search box, the PlaylistsRail, and the @@ -38,17 +39,6 @@ export function usePlaylistsActions(): PlaylistsActions { return useContext(ActionsContext); } -function readSelectedId(): number | null { - // A ?pl bookmark wins over the stored selection so a pasted link opens that playlist. Accept only a - // positive integer id — a bare "?pl=" gives Number("")===0 (finite!) and "?pl=1.5" a fractional id, - // both of which would select a non-existent playlist. - const fromUrl = new URLSearchParams(window.location.search).get("pl"); - const n = fromUrl ? Number(fromUrl) : NaN; - if (Number.isInteger(n) && n > 0) return n; - const s = getAccountRaw(LS.playlist); - return s ? Number(s) : null; -} - export function PlaylistsProvider({ children }: { children: ReactNode }) { const [search, setSearchState] = useState(""); const [selectedId, setSelectedIdState] = useState(readSelectedId); diff --git a/frontend/src/components/playlistsInit.test.ts b/frontend/src/components/playlistsInit.test.ts new file mode 100644 index 00000000..bdaf7aaa --- /dev/null +++ b/frontend/src/components/playlistsInit.test.ts @@ -0,0 +1,54 @@ +// @vitest-environment jsdom +// +// readSelectedId: the Playlists selected-id seed. Backfills the E4 deep-review's deferred ?pl guard +// coverage — a ?pl bookmark wins over the stored selection but only when it's a positive integer; +// anything else (empty / zero / negative / fractional / non-numeric) falls back to the stored id. +import { describe, it, expect, beforeEach, vi } from "vitest"; + +const { stored } = vi.hoisted(() => ({ stored: { v: null as string | null } })); +vi.mock("../lib/storage", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, getAccountRaw: () => stored.v }; +}); + +import { readSelectedId } from "./playlistsInit"; + +const setUrl = (search: string) => window.history.replaceState(null, "", search || "/"); + +beforeEach(() => { + setUrl("/"); + stored.v = null; +}); + +describe("readSelectedId ?pl guard", () => { + it("accepts a positive-integer ?pl", () => { + setUrl("?pl=5"); + expect(readSelectedId()).toBe(5); + }); + + it("a ?pl bookmark wins over the stored selection", () => { + setUrl("?pl=5"); + stored.v = "7"; + expect(readSelectedId()).toBe(5); + }); + + it.each(["", "0", "-2", "1.5", "abc"])("rejects ?pl=%s and falls back to stored", (raw) => { + setUrl(`?pl=${raw}`); + stored.v = "7"; + expect(readSelectedId()).toBe(7); + }); + + it("rejects a bad ?pl and returns null when there is no stored selection", () => { + setUrl("?pl=0"); + expect(readSelectedId()).toBeNull(); + }); + + it("uses the stored selection when there is no ?pl", () => { + stored.v = "9"; + expect(readSelectedId()).toBe(9); + }); + + it("returns null when neither ?pl nor a stored selection is present", () => { + expect(readSelectedId()).toBeNull(); + }); +}); diff --git a/frontend/src/components/playlistsInit.ts b/frontend/src/components/playlistsInit.ts new file mode 100644 index 00000000..bc3c256b --- /dev/null +++ b/frontend/src/components/playlistsInit.ts @@ -0,0 +1,14 @@ +import { getAccountRaw, LS } from "../lib/storage"; + +// The Playlists module's selected-id seed, split out of PlaylistsProvider so its ?pl guard is +// unit-testable and the provider stays react-refresh-clean (component-only exports). +export function readSelectedId(): number | null { + // A ?pl bookmark wins over the stored selection so a pasted link opens that playlist. Accept only a + // positive integer id — a bare "?pl=" gives Number("")===0 (finite!) and "?pl=1.5" a fractional id, + // both of which would select a non-existent playlist. + const fromUrl = new URLSearchParams(window.location.search).get("pl"); + const n = fromUrl ? Number(fromUrl) : NaN; + if (Number.isInteger(n) && n > 0) return n; + const s = getAccountRaw(LS.playlist); + return s ? Number(s) : null; +} diff --git a/frontend/src/lib/usePref.dom.test.ts b/frontend/src/lib/usePref.dom.test.ts index e20831da..a9fb61aa 100644 --- a/frontend/src/lib/usePref.dom.test.ts +++ b/frontend/src/lib/usePref.dom.test.ts @@ -155,6 +155,29 @@ describe("usePref", () => { expect(savePrefs).toHaveBeenCalledTimes(1); expect(savePrefs).toHaveBeenCalledWith({ vol: 30 }); }); + + it("flushes a pending debounced write on unmount instead of dropping it", () => { + const qc = makeClient(); + seedMe(qc, {}); + const { result, unmount } = renderHook(() => usePref("vol", 0, { debounceMs: 300 }), { + wrapper: wrapperFor(qc), + }); + act(() => result.current[1](42)); + expect(savePrefs).not.toHaveBeenCalled(); // still inside the debounce window + unmount(); // cleanup must flush the pending write, not lose it + expect(savePrefs).toHaveBeenCalledTimes(1); + expect(savePrefs).toHaveBeenCalledWith({ vol: 42 }); + }); + + it("unmount with no pending write does not call savePrefs", () => { + const qc = makeClient(); + seedMe(qc, {}); + const { unmount } = renderHook(() => usePref("vol", 0, { debounceMs: 300 }), { + wrapper: wrapperFor(qc), + }); + unmount(); + expect(savePrefs).not.toHaveBeenCalled(); + }); }); }); diff --git a/frontend/src/lib/useUrlMirror.dom.test.ts b/frontend/src/lib/useUrlMirror.dom.test.ts new file mode 100644 index 00000000..d1d85f55 --- /dev/null +++ b/frontend/src/lib/useUrlMirror.dom.test.ts @@ -0,0 +1,51 @@ +// @vitest-environment jsdom +// +// useUrlMirror: mirror a set of nav params onto the CURRENT history entry (replaceState) whenever they +// change. Backfills the E4 deep-review's deferred coverage — the effect keys on the SERIALIZED updates +// so an inline object literal (new identity every render) only fires when a param actually changes; a +// null value clears a key; and every other param on the URL is preserved. +import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; +import { renderHook } from "@testing-library/react"; +import { useUrlMirror } from "./useUrlMirror"; + +const setUrl = (search: string) => window.history.replaceState(null, "", search || "/"); +let spy: ReturnType; + +beforeEach(() => { + setUrl("/"); + spy = vi.spyOn(window.history, "replaceState"); +}); +afterEach(() => spy.mockRestore()); + +describe("useUrlMirror", () => { + it("writes the param onto the current entry on mount", () => { + renderHook(() => useUrlMirror({ pl: "5" })); + expect(window.location.search).toBe("?pl=5"); + expect(spy).toHaveBeenCalledTimes(1); + }); + + it("does not re-fire when the serialized updates are unchanged (stable dep)", () => { + const { rerender } = renderHook((p: Record) => useUrlMirror(p), { + initialProps: { pl: "5" } as Record, + }); + expect(spy).toHaveBeenCalledTimes(1); + rerender({ pl: "5" }); // new object identity, same serialized value + expect(spy).toHaveBeenCalledTimes(1); + }); + + it("clears a key when its value turns null", () => { + const { rerender } = renderHook((p: Record) => useUrlMirror(p), { + initialProps: { pl: "5" } as Record, + }); + expect(window.location.search).toBe("?pl=5"); + rerender({ pl: null }); + expect(window.location.search).toBe(""); + expect(spy).toHaveBeenCalledTimes(2); + }); + + it("preserves other params already on the URL", () => { + setUrl("?page=playlists"); + renderHook(() => useUrlMirror({ pl: "5" })); + expect(window.location.search).toBe("?page=playlists&pl=5"); + }); +}); From 0b7180ce8f406db2b949f7bae9acc858173fd76a Mon Sep 17 00:00:00 2001 From: npeter83 Date: Thu, 20 Aug 2026 05:25:48 +0200 Subject: [PATCH 6/8] a11y(e4): aria-label on the icon-only Plex watched-toggle buttons MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The hover watched-toggle on Plex movie/episode cards is icon-only (WatchedIcon, no text), so it carried a `title` but no accessible name for screen readers — and the empty-Circle unwatched glyph could read like a radio. Mirror the title into `aria-label` (the Modal close-button idiom) so the control announces its action. The PlexInfo watched buttons already have visible text labels, so they need nothing. FE gate green (tsc/eslint/prettier/knip, 309 vitest). --- frontend/src/components/plexCards.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/frontend/src/components/plexCards.tsx b/frontend/src/components/plexCards.tsx index e15a5e3a..ffc58495 100644 --- a/frontend/src/components/plexCards.tsx +++ b/frontend/src/components/plexCards.tsx @@ -100,6 +100,7 @@ export function PlexPosterCard({ onToggleWatched(card); }} title={card.status === "watched" ? t("plex.markUnwatched") : t("plex.markWatched")} + aria-label={card.status === "watched" ? t("plex.markUnwatched") : t("plex.markWatched")} className="absolute top-1.5 right-1.5 p-1 rounded-full bg-black/60 reveal hover:bg-black/80 transition" > @@ -438,6 +439,7 @@ export function EpisodeCard({ onToggleWatched(); }} title={ep.status === "watched" ? t("plex.markUnwatched") : t("plex.markWatched")} + aria-label={ep.status === "watched" ? t("plex.markUnwatched") : t("plex.markWatched")} className="absolute top-1.5 right-1.5 p-1 rounded-full bg-black/60 reveal hover:bg-black/80 transition" > From 482dd2800650a4a4e86b5398cef2ce14bc42fd8d Mon Sep 17 00:00:00 2001 From: npeter83 Date: Thu, 20 Aug 2026 05:40:59 +0200 Subject: [PATCH 7/8] fix(e4): address code-review findings (a11y sweep + dedup guards) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolves the 5 /code-review high findings on this branch: - a11y sweep: extend aria-label to the remaining icon-only Plex card buttons the first pass missed — PlexPosterCard Info, SeasonCard watched-toggle + add-season (EpisodeCard already had them). "Fix a principle everywhere, in one sweep." - new shared lib/parseId.ts `parsePositiveIntId`, used by BOTH readSelectedId (now guarding the stored ?pl fallback the same as the URL — was unguarded) and plexSubUrl's ?v=playlist decode; removes the hand-duplicated guard. - PAGE_KEY exported from navigationInit and imported by NavigationProvider (was defined in both). - hoist a `watchedLabel` const per card so title + aria-label can't drift. +8 tests (parseId unit + guarded-stored ?pl); full FE gate green (tsc/eslint/ prettier/knip, 317 vitest). --- .../src/components/NavigationProvider.tsx | 6 ++--- frontend/src/components/navigationInit.ts | 2 +- frontend/src/components/playlistsInit.test.ts | 8 +++++++ frontend/src/components/playlistsInit.ts | 16 ++++++------- frontend/src/components/plexCards.tsx | 20 ++++++++++------ frontend/src/components/plexSubUrl.ts | 7 +++--- frontend/src/lib/parseId.test.ts | 23 +++++++++++++++++++ frontend/src/lib/parseId.ts | 9 ++++++++ 8 files changed, 68 insertions(+), 23 deletions(-) create mode 100644 frontend/src/lib/parseId.test.ts create mode 100644 frontend/src/lib/parseId.ts diff --git a/frontend/src/components/NavigationProvider.tsx b/frontend/src/components/NavigationProvider.tsx index 869d293d..802b8a1d 100644 --- a/frontend/src/components/NavigationProvider.tsx +++ b/frontend/src/components/NavigationProvider.tsx @@ -11,10 +11,10 @@ import { import { useTranslation } from "react-i18next"; import { urlWithParams, type Page } from "../lib/urlState"; import { pageTitleKey } from "../lib/pageMeta"; -import { LS, setAccountRaw } from "../lib/storage"; +import { setAccountRaw } from "../lib/storage"; import { useAccountRehydrate } from "../lib/useAccountRehydrate"; import { setNavigator } from "../lib/nav"; -import { channelViewFrom, loadInitialPage, type ChannelView } from "./navigationInit"; +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 // consume it via a hook instead of prop-drilling `page`/`setPage`/`onOpenChannel` through the tree. @@ -27,8 +27,6 @@ import { channelViewFrom, loadInitialPage, type ChannelView } from "./navigation // re-render on a page/channel switch — preserving the memo that keeps feed appends from re-rendering // every existing card. -const PAGE_KEY = LS.page; - export interface NavigationState { page: Page; /** The open channel page (null = none). Overlays `page`; rides in history.state._chan. */ diff --git a/frontend/src/components/navigationInit.ts b/frontend/src/components/navigationInit.ts index a57b0c03..da5640bb 100644 --- a/frontend/src/components/navigationInit.ts +++ b/frontend/src/components/navigationInit.ts @@ -5,7 +5,7 @@ import { hasFilterParams, isPage, readPage, type Page } from "../lib/urlState"; import { getAccountRaw, LS } from "../lib/storage"; -const PAGE_KEY = LS.page; +export const PAGE_KEY = LS.page; export interface ChannelView { id: string; diff --git a/frontend/src/components/playlistsInit.test.ts b/frontend/src/components/playlistsInit.test.ts index bdaf7aaa..5e67c970 100644 --- a/frontend/src/components/playlistsInit.test.ts +++ b/frontend/src/components/playlistsInit.test.ts @@ -48,6 +48,14 @@ describe("readSelectedId ?pl guard", () => { expect(readSelectedId()).toBe(9); }); + it.each(["", "0", "-2", "1.5", "abc"])( + "rejects a non-positive-integer STORED value %s (guarded like the URL)", + (raw) => { + stored.v = raw; + expect(readSelectedId()).toBeNull(); + }, + ); + it("returns null when neither ?pl nor a stored selection is present", () => { expect(readSelectedId()).toBeNull(); }); diff --git a/frontend/src/components/playlistsInit.ts b/frontend/src/components/playlistsInit.ts index bc3c256b..9ff88506 100644 --- a/frontend/src/components/playlistsInit.ts +++ b/frontend/src/components/playlistsInit.ts @@ -1,14 +1,14 @@ import { getAccountRaw, LS } from "../lib/storage"; +import { parsePositiveIntId } from "../lib/parseId"; // The Playlists module's selected-id seed, split out of PlaylistsProvider so its ?pl guard is // unit-testable and the provider stays react-refresh-clean (component-only exports). export function readSelectedId(): number | null { - // A ?pl bookmark wins over the stored selection so a pasted link opens that playlist. Accept only a - // positive integer id — a bare "?pl=" gives Number("")===0 (finite!) and "?pl=1.5" a fractional id, - // both of which would select a non-existent playlist. - const fromUrl = new URLSearchParams(window.location.search).get("pl"); - const n = fromUrl ? Number(fromUrl) : NaN; - if (Number.isInteger(n) && n > 0) return n; - const s = getAccountRaw(LS.playlist); - return s ? Number(s) : null; + // A ?pl bookmark wins over the stored selection so a pasted link opens that playlist. BOTH must be a + // positive-integer id (a bare "?pl=" gives Number("")===0, "?pl=1.5" a fractional id) — either would + // select a non-existent playlist, so the stored fallback is guarded exactly like the URL. + return ( + parsePositiveIntId(new URLSearchParams(window.location.search).get("pl")) ?? + parsePositiveIntId(getAccountRaw(LS.playlist)) + ); } diff --git a/frontend/src/components/plexCards.tsx b/frontend/src/components/plexCards.tsx index ffc58495..18595777 100644 --- a/frontend/src/components/plexCards.tsx +++ b/frontend/src/components/plexCards.tsx @@ -50,6 +50,7 @@ export function PlexPosterCard({ const { t } = useTranslation(); const inProgress = (card.position_seconds ?? 0) > 0 && card.status !== "watched"; const isPlayable = card.type !== "show"; // movies/episodes play; shows open the drill-down + const watchedLabel = card.status === "watched" ? t("plex.markUnwatched") : t("plex.markWatched"); const pct = inProgress && card.duration_seconds ? Math.min(100, Math.round(((card.position_seconds ?? 0) / card.duration_seconds) * 100)) @@ -99,8 +100,8 @@ export function PlexPosterCard({ e.stopPropagation(); onToggleWatched(card); }} - title={card.status === "watched" ? t("plex.markUnwatched") : t("plex.markWatched")} - aria-label={card.status === "watched" ? t("plex.markUnwatched") : t("plex.markWatched")} + title={watchedLabel} + aria-label={watchedLabel} className="absolute top-1.5 right-1.5 p-1 rounded-full bg-black/60 reveal hover:bg-black/80 transition" > @@ -114,6 +115,7 @@ export function PlexPosterCard({ onInfo(); }} title={t("plex.info.title")} + aria-label={t("plex.info.title")} className="absolute bottom-1.5 left-1.5 p-1 rounded-full bg-black/60 reveal hover:bg-black/80 transition" > @@ -192,6 +194,9 @@ export function SeasonCard({ }) { const { t } = useTranslation(); const watched = se.status === "watched"; + const watchedLabel = watched + ? t("plex.series.markSeasonUnwatched") + : t("plex.series.markSeasonWatched"); return (
@@ -241,6 +245,7 @@ export function SeasonCard({ onAdd(); }} title={t("plex.playlist.addSeason")} + aria-label={t("plex.playlist.addSeason")} className="absolute bottom-1.5 right-1.5 p-1 rounded-full bg-black/60 reveal hover:bg-black/80 transition" > @@ -404,6 +409,7 @@ export function EpisodeCard({ }) { const { t } = useTranslation(); const inProgress = (ep.position_seconds ?? 0) > 0 && ep.status !== "watched"; + const watchedLabel = ep.status === "watched" ? t("plex.markUnwatched") : t("plex.markWatched"); const pct = inProgress && ep.duration_seconds ? Math.min(100, Math.round(((ep.position_seconds ?? 0) / ep.duration_seconds) * 100)) @@ -438,8 +444,8 @@ export function EpisodeCard({ e.stopPropagation(); onToggleWatched(); }} - title={ep.status === "watched" ? t("plex.markUnwatched") : t("plex.markWatched")} - aria-label={ep.status === "watched" ? t("plex.markUnwatched") : t("plex.markWatched")} + title={watchedLabel} + aria-label={watchedLabel} className="absolute top-1.5 right-1.5 p-1 rounded-full bg-black/60 reveal hover:bg-black/80 transition" > diff --git a/frontend/src/components/plexSubUrl.ts b/frontend/src/components/plexSubUrl.ts index d3fc2816..a63e633a 100644 --- a/frontend/src/components/plexSubUrl.ts +++ b/frontend/src/components/plexSubUrl.ts @@ -1,4 +1,5 @@ import type { SubviewUrl } from "../lib/history"; +import { parsePositiveIntId } from "../lib/parseId"; // The Plex module's sub-view union and its address-bar mirror, extracted from PlexBrowse so the // encode/decode round-trip (and its guards) can be unit-tested without pulling the whole component @@ -46,9 +47,9 @@ export const PLEX_SUB_URL: SubviewUrl = { return arg ? { kind: "person", name: arg } : null; case "playlist": { // Positive-integer id only — a fractional/zero/negative ?v=playlist: would open a - // sub-view for a non-existent id (matches the ?pl guard in PlaylistsProvider.readSelectedId). - const id = arg ? Number(arg) : NaN; - return Number.isInteger(id) && id > 0 ? { kind: "playlist", id } : null; + // sub-view for a non-existent id (shares parsePositiveIntId with readSelectedId's ?pl guard). + const id = parsePositiveIntId(arg); + return id != null ? { kind: "playlist", id } : null; } case "season": { const [showId, seasonId] = arg.split(":"); diff --git a/frontend/src/lib/parseId.test.ts b/frontend/src/lib/parseId.test.ts new file mode 100644 index 00000000..fa2d98f1 --- /dev/null +++ b/frontend/src/lib/parseId.test.ts @@ -0,0 +1,23 @@ +import { describe, it, expect } from "vitest"; +import { parsePositiveIntId } from "./parseId"; + +describe("parsePositiveIntId", () => { + it("accepts a positive integer", () => { + expect(parsePositiveIntId("1")).toBe(1); + expect(parsePositiveIntId("42")).toBe(42); + }); + + it("rejects empty / nullish", () => { + expect(parsePositiveIntId("")).toBeNull(); + expect(parsePositiveIntId(null)).toBeNull(); + expect(parsePositiveIntId(undefined)).toBeNull(); + }); + + it("rejects zero, negative, fractional, and non-numeric", () => { + expect(parsePositiveIntId("0")).toBeNull(); + expect(parsePositiveIntId("-1")).toBeNull(); + expect(parsePositiveIntId("1.5")).toBeNull(); + expect(parsePositiveIntId("abc")).toBeNull(); + expect(parsePositiveIntId(" ")).toBeNull(); // Number(" ")===0 + }); +}); diff --git a/frontend/src/lib/parseId.ts b/frontend/src/lib/parseId.ts new file mode 100644 index 00000000..dd46fdb2 --- /dev/null +++ b/frontend/src/lib/parseId.ts @@ -0,0 +1,9 @@ +/** Parse a raw value to a positive-integer id, or null if it isn't one. A bare "" gives + * Number("")===0 (finite but not positive), "1.5" is fractional, "-1"/"abc" invalid — all rejected. + * The single guard behind the ?pl / ?v=playlist id params AND the stored playlist selection, so a bad + * value (a malformed link, a legacy/corrupted storage entry) never selects a non-existent playlist. */ +export function parsePositiveIntId(raw: string | null | undefined): number | null { + if (raw == null || raw === "") return null; + const n = Number(raw); + return Number.isInteger(n) && n > 0 ? n : null; +} From 99d63d272e35954df04e16f1c5909189f6b69b99 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Thu, 20 Aug 2026 05:52:36 +0200 Subject: [PATCH 8/8] test(e4): lock accessible names on the Plex card buttons Closes the re-review's test-coverage finding: a plexCards.dom.test asserting the icon-only card controls (watched-toggle, Info, add-season/episode) each expose their aria-label as an accessible name via getByRole({name}). A future edit that drops a label now fails the gate instead of silently reopening the unnamed-button gap the a11y sweep closed. 4 tests; full FE gate green (tsc/eslint/prettier/knip, 321 vitest). --- .../src/components/plexCards.dom.test.tsx | 82 +++++++++++++++++++ 1 file changed, 82 insertions(+) create mode 100644 frontend/src/components/plexCards.dom.test.tsx diff --git a/frontend/src/components/plexCards.dom.test.tsx b/frontend/src/components/plexCards.dom.test.tsx new file mode 100644 index 00000000..20261d36 --- /dev/null +++ b/frontend/src/components/plexCards.dom.test.tsx @@ -0,0 +1,82 @@ +// @vitest-environment jsdom +// +// Plex card a11y: locks the accessible names on the icon-only card controls (the E4 a11y sweep). These +// buttons are icon-only (WatchedIcon / Info / ListPlus, no text), so their name comes solely from the +// aria-label — a dropped label silently reintroduces an unnamed button for screen-reader users. i18n is +// key-passthrough, so the asserted names are the i18n keys. +import { describe, it, expect, afterEach, vi } from "vitest"; +import { render, screen, cleanup } from "@testing-library/react"; + +vi.mock("react-i18next", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, useTranslation: () => ({ t: (key: string) => key }) }; +}); + +import { PlexPosterCard, SeasonCard, EpisodeCard } from "./plexCards"; +import type { PlexCard, PlexSeasonDetail } from "../lib/api"; + +const movie = (status?: string): PlexCard => ({ + id: "1", + type: "movie", + title: "M", + thumb: "", + status, +}); +const episode = (status?: string): PlexCard => ({ + id: "e1", + type: "episode", + title: "E1", + thumb: "", + status, +}); +const season = (status = "new"): PlexSeasonDetail => ({ + id: "s1", + season_number: 1, + title: "S1", + thumb: "", + episode_count: 3, + status, + episodes: [], +}); + +afterEach(cleanup); + +describe("PlexPosterCard a11y", () => { + it("names the watched-toggle (mark-watched when unwatched) and the info button", () => { + render( + , + ); + expect(screen.getByRole("button", { name: "plex.markWatched" })).toBeTruthy(); + expect(screen.getByRole("button", { name: "plex.info.title" })).toBeTruthy(); + }); + + it("names the watched-toggle as mark-unwatched when the movie is watched", () => { + render( + , + ); + expect(screen.getByRole("button", { name: "plex.markUnwatched" })).toBeTruthy(); + }); +}); + +describe("SeasonCard a11y", () => { + it("names the watched-toggle and the add-season button", () => { + render(); + expect(screen.getByRole("button", { name: "plex.series.markSeasonWatched" })).toBeTruthy(); + expect(screen.getByRole("button", { name: "plex.playlist.addSeason" })).toBeTruthy(); + }); +}); + +describe("EpisodeCard a11y", () => { + it("names the watched-toggle and the add-episode button", () => { + render( + , + ); + expect(screen.getByRole("button", { name: "plex.markWatched" })).toBeTruthy(); + expect(screen.getByRole("button", { name: "plex.playlist.addEpisode" })).toBeTruthy(); + }); +});