From 046ff735cfe07f41f270b81f428bffc45b707c7e Mon Sep 17 00:00:00 2001 From: npeter83 Date: Sat, 19 Sep 2026 18:14:18 +0200 Subject: [PATCH] test: close the last coverage debt the roadmap still listed - `overflow-clip` gets a safelist anchor in index.css (its only occurrence is a `.ts` constant), and the chip-lane contract keeps it there. - PlexPlayer's Backspace ownership rule is pinned on all three branches: nothing above, a foreign layer above (it defers), and its own panel open under a layer (it still closes the panel). - Removing a playlist item bumps the list's `updated_at`, like the three other item-only writes. - Seen in a real browser on localdev: an exhausted comfort-shelf tile (an empty pinned playlist) is focusable, aria-disabled, and its Shift+F10 menu offers Play off and Remove from shelf. --- backend/tests/test_playlist_move.py | 7 ++ .../PlexPlayer.backspace.dom.test.tsx | 116 ++++++++++++++++++ frontend/src/index.css | 18 ++- frontend/src/lib/chipLane.contract.test.ts | 8 ++ ....test.ts => controlRules.contract.test.ts} | 0 5 files changed, 144 insertions(+), 5 deletions(-) create mode 100644 frontend/src/components/PlexPlayer.backspace.dom.test.tsx rename frontend/src/lib/{nativeSelect.contract.test.ts => controlRules.contract.test.ts} (100%) diff --git a/backend/tests/test_playlist_move.py b/backend/tests/test_playlist_move.py index 8bf1de21..3f9a5fc2 100644 --- a/backend/tests/test_playlist_move.py +++ b/backend/tests/test_playlist_move.py @@ -235,6 +235,13 @@ def test_item_edits_bump_updated_at(db, make_user, as_user): assert db.get(Playlist, src.id).updated_at > old assert db.get(Playlist, dst.id).updated_at > old + # …and removing one, the fourth item-only write (it had no test: E10 review coverage debt). + src.updated_at = old + db.commit() + assert client.delete(f"/api/playlists/{src.id}/items/v1").status_code == 200 + db.expire_all() + assert db.get(Playlist, src.id).updated_at > old + def test_the_list_endpoint_publishes_updated_at(db, make_user, as_user): user = make_user() diff --git a/frontend/src/components/PlexPlayer.backspace.dom.test.tsx b/frontend/src/components/PlexPlayer.backspace.dom.test.tsx new file mode 100644 index 00000000..8983176e --- /dev/null +++ b/frontend/src/components/PlexPlayer.backspace.dom.test.tsx @@ -0,0 +1,116 @@ +// @vitest-environment jsdom +// +// Who owns ⌫ while the Plex player is up. Three rounds got it wrong in turn: deferring to nothing +// (the cheat sheet took ⌫ and the player stopped underneath it), deferring to any layer above (the +// player's OWN menus push layers, so ⌫ stopped closing them), then naming just the sheet and the +// palette (the app-wide ErrorDialog is neither, so ⌫ tore the player down under it). The rule that +// holds: the player's own panels answer first; anything else layered above it takes the key. It had +// no test (E10 review coverage debt). +import { describe, it, expect, vi, beforeAll, afterEach } from "vitest"; +import { render, cleanup, waitFor, act } 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" } }), +})); + +import PlexPlayer from "./PlexPlayer"; +import { api, type PlexItemDetail } from "../lib/api"; +import { pushLayer, removeLayer } from "../lib/layerStack"; + +beforeAll(() => { + class Stub { + observe() {} + unobserve() {} + disconnect() {} + takeRecords() { + return []; + } + } + globalThis.ResizeObserver ??= Stub as unknown as typeof ResizeObserver; + globalThis.IntersectionObserver ??= Stub as unknown as typeof IntersectionObserver; + for (const [name, value] of Object.entries({ + play: () => Promise.resolve(), + pause: () => {}, + load: () => {}, + })) + Object.defineProperty(HTMLMediaElement.prototype, name, { configurable: true, value }); + Object.defineProperty(HTMLMediaElement.prototype, "seekable", { + configurable: true, + get: () => ({ length: 0, end: () => 0 }), + }); + Object.defineProperty(HTMLMediaElement.prototype, "textTracks", { + configurable: true, + get: () => Object.assign([], { addEventListener() {}, removeEventListener() {} }), + }); +}); +afterEach(() => { + cleanup(); + vi.restoreAllMocks(); +}); + +const detail = { + id: "1", + title: "Title 1", + type: "movie", + duration_seconds: 3000, + status: "new", + position_seconds: 0, + audio_streams: [], + subtitle_streams: [], + markers: [], +} as unknown as PlexItemDetail; + +async function mount() { + vi.spyOn(api, "plexItem").mockResolvedValue(detail); + vi.spyOn(api, "plexSession").mockResolvedValue({ + mode: "direct", + url: "/media/1", + start_s: 0, + } as Awaited>); + vi.spyOn(api, "plexProgress").mockResolvedValue(undefined as never); + const onClose = vi.fn(); + render( + + + , + ); + await waitFor(() => + expect(document.querySelector("video")?.getAttribute("src")).toBe("/media/1"), + ); + return onClose; +} + +const press = (key: string) => + act(() => { + window.dispatchEvent(new KeyboardEvent("keydown", { key, bubbles: true, cancelable: true })); + }); + +describe("PlexPlayer · who owns Backspace", () => { + it("goes back when nothing is layered over the player", async () => { + const onClose = await mount(); + press("Backspace"); + expect(onClose).toHaveBeenCalledTimes(1); + }); + + it("leaves the key to a layer above it that is not its own — an error dialog, say", async () => { + const onClose = await mount(); + const above = pushLayer(); + press("Backspace"); + expect(onClose).not.toHaveBeenCalled(); + removeLayer(above); + }); + + it("still closes its OWN panel first, even with a layer above it", async () => { + const onClose = await mount(); + press("d"); // the stats panel: one of the player's own + const above = pushLayer(); + press("Backspace"); // closes the panel, does not leave the player + removeLayer(above); + press("Backspace"); // nothing left of its own: now it goes back + expect(onClose).toHaveBeenCalledTimes(1); + }); +}); diff --git a/frontend/src/index.css b/frontend/src/index.css index 631d6a15..fa6267a1 100644 --- a/frontend/src/index.css +++ b/frontend/src/index.css @@ -6,6 +6,12 @@ consumer today). Keeps the semantic token set complete + future-proof. */ @source inline("text-danger-fg text-warn-fg text-success-fg text-info-fg"); +/* `overflow-clip` lives in exactly one place, a `.ts` constant (lib/useChipOverflowFit.ts). If source + detection ever stopped covering that file the utility would silently leave the bundle and the + 1024px sideways-scroll bug on the chip lanes would come back with every lane green — so it is + anchored here too. */ +@source inline("overflow-clip"); + /* `inline` is load-bearing: the color vars (--bg/--accent/… below) are redefined at runtime per html[data-scheme]/[data-theme], so utilities must inline the value and resolve it at the USE site, not reference a once-computed --color-* token. This also @@ -571,9 +577,9 @@ body { background-color: var(--warn); color: var(--warn-fg); } - .btn-primary:hover:not(:disabled), - .btn-danger:hover:not(:disabled), - .btn-warn:hover:not(:disabled) { + .btn-primary:hover:not(:disabled):not([aria-disabled="true"]), + .btn-danger:hover:not(:disabled):not([aria-disabled="true"]), + .btn-warn:hover:not(:disabled):not([aria-disabled="true"]) { opacity: 0.9; } @@ -615,8 +621,10 @@ body { } /* `:not(:disabled)` for the same reason ui/Button prefixes every variant hover with `enabled:` — a disabled control must not advertise a click that does nothing. The Tailwind half was gated in E10 - review round 1 and this half was missed, so a disabled glass button still lifted (round 2). */ -.glass-hover:not(:disabled):hover { + review round 1 and this half was missed, so a disabled glass button still lifted (round 2). + `:not([aria-disabled="true"])` too: a FocusSafeButton that holds the focus is disabled by + aria-disabled, not the native attribute (debt burn 2026-09), and lifted again until this. */ +.glass-hover:not(:disabled):not([aria-disabled="true"]):hover { border-color: color-mix(in srgb, var(--accent) 55%, transparent); box-shadow: inset 0 1px 0 color-mix(in srgb, #fff 12%, transparent), diff --git a/frontend/src/lib/chipLane.contract.test.ts b/frontend/src/lib/chipLane.contract.test.ts index 544f7e8d..15bda4c1 100644 --- a/frontend/src/lib/chipLane.contract.test.ts +++ b/frontend/src/lib/chipLane.contract.test.ts @@ -89,4 +89,12 @@ describe("chip lane containment contract", () => { // padding this canary, so it stayed satisfied with one real measurer fewer (round 2). expect(sources().filter(([, src]) => src.includes(MIRROR)).length).toBeGreaterThanOrEqual(3); }); + + // The clip is a Tailwind utility whose only occurrence is `chipLaneClass`, a `.ts` constant: if + // source detection ever stopped covering that file, the utility would leave the bundle and the + // leak above would return with every lane green. index.css anchors it; this keeps the anchor. + it("keeps the clip in the bundle whatever source detection sees", () => { + const css = readFileSync(new URL("../index.css", import.meta.url), "utf-8"); + expect(css).toMatch(/@source inline\("[^"]*\boverflow-clip\b[^"]*"\)/); + }); }); diff --git a/frontend/src/lib/nativeSelect.contract.test.ts b/frontend/src/lib/controlRules.contract.test.ts similarity index 100% rename from frontend/src/lib/nativeSelect.contract.test.ts rename to frontend/src/lib/controlRules.contract.test.ts