From 0521149cf17fdfaaa197ce6056da9a12591c0f3b Mon Sep 17 00:00:00 2001 From: npeter83 Date: Fri, 25 Sep 2026 02:25:54 +0200 Subject: [PATCH] =?UTF-8?q?fix(design):=20claim=20audit=20part=201=20?= =?UTF-8?q?=E2=80=94=20a=20closing=20key=20no=20longer=20re-presses=20its?= =?UTF-8?q?=20trigger,=20the=20Switch=20box=20and=20name,=20named=20z-leve?= =?UTF-8?q?ls=20(E28)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - In-place editors (Playlists heading + rail rename, Plex collection + playlist rename, PriorityStepper, Scheduler interval): a real Enter closed the editor, returned the focus to the trigger, and the same keystroke's activation pressed that trigger again - re-opening the editor, or in the rail selecting the renamed playlist. useReturnFocus.arm(e) now cancels the key's default (type-enforced); the rail and Scheduler editors do the same. The earlier tests dispatched a synthetic keydown, which has no default. - Plex playlist rename: an unchanged or empty title closes with no write, as the collection rename does. - Switch: a 36x24 box around the track (the 24 floor), 44 on a coarse pointer; `label` is required, and the Plex two-way-sync switch had no name. - z-index: numeric z-10/z-20 and a literal 40 moved onto the named layer scale; a lint rule + contract. - The /watch file card's Download is the filled primary: the page's one action. - Stale comments: the 1024x768 floor, the retired glass tuner. --- frontend/eslint.config.js | 7 +++ frontend/src/components/Playlists.tsx | 4 +- .../PlaylistsRail.rename.dom.test.tsx | 5 +- frontend/src/components/PlaylistsRail.tsx | 4 ++ .../src/components/PlexCollectionView.tsx | 4 +- .../PlexPlaylistView.header.dom.test.tsx | 20 +++++++- frontend/src/components/PlexPlaylistView.tsx | 11 +++-- frontend/src/components/PlexSearchBox.tsx | 2 +- .../src/components/Scheduler.dom.test.tsx | 10 ++++ frontend/src/components/Scheduler.tsx | 12 ++++- frontend/src/components/SettingsPanel.tsx | 7 ++- frontend/src/components/VideoCard.tsx | 2 +- .../src/components/WatchPage.dom.test.tsx | 2 + frontend/src/components/WatchPage.tsx | 10 +++- frontend/src/components/plexCards.tsx | 8 ++-- frontend/src/components/ui/Carousel.tsx | 2 +- .../src/components/ui/PriorityStepper.tsx | 4 +- frontend/src/components/ui/form.dom.test.tsx | 29 ++++++++++++ frontend/src/components/ui/form.tsx | 22 +++++---- frontend/src/components/ui/gridColumns.ts | 2 +- .../components/ui/useReturnFocus.dom.test.tsx | 46 +++++++++++++++++++ frontend/src/components/ui/useReturnFocus.ts | 10 +++- frontend/src/index.css | 9 ++-- frontend/src/lib/feedView.ts | 2 +- frontend/src/lib/zLayer.contract.test.ts | 34 ++++++++++++++ 25 files changed, 227 insertions(+), 41 deletions(-) create mode 100644 frontend/src/components/ui/form.dom.test.tsx create mode 100644 frontend/src/components/ui/useReturnFocus.dom.test.tsx create mode 100644 frontend/src/lib/zLayer.contract.test.ts diff --git a/frontend/eslint.config.js b/frontend/eslint.config.js index d8296925..eb2946ed 100644 --- a/frontend/eslint.config.js +++ b/frontend/eslint.config.js @@ -318,6 +318,11 @@ const BOX_PX = "/(^|[\\s:])(min-|max-)?(w|h|size|left|right|top|bottom|inset|gap|p[xytrbl]?|m[xytrbl]?)-\\[[\\d.]+px\\]/"; const BOX_PX_MESSAGE = "px-frozen box size doesn't scale with --font-scale: use the spacing scale (w-37.5, max-w-60), not w-[Npx]."; +// A stacking order outside the layer scale (DESIGN.md Tokens: `base < menu < chrome < … < tooltip` in +// index.css). A numeric `z-10` equals a level today but doesn't follow it when the scale moves; name it. +const Z_RAW = "/(^|[\\s:])-?z-([1-9]\\d*|\\[[^\\]]+\\])(?![\\w-])/"; +const Z_RAW_MESSAGE = + "z-index off the layer scale: use a named level (z-base, z-menu, z-chrome, z-panel, z-rail, z-overlay, z-popover, z-tooltip), not z-N or z-[…]."; export default tseslint.config( { ignores: ["dist", "node_modules", "playwright-report", "test-results", "e2e/.auth"] }, @@ -431,6 +436,8 @@ export default tseslint.config( // 150px at the default size) — 32 of these were swept in the 2026-09 debt burn. { selector: `Literal[value=${BOX_PX}]`, message: BOX_PX_MESSAGE }, { selector: `TemplateElement[value.raw=${BOX_PX}]`, message: BOX_PX_MESSAGE }, + { selector: `Literal[value=${Z_RAW}]`, message: Z_RAW_MESSAGE }, + { selector: `TemplateElement[value.raw=${Z_RAW}]`, message: Z_RAW_MESSAGE }, ], }, }, diff --git a/frontend/src/components/Playlists.tsx b/frontend/src/components/Playlists.tsx index 60d4313c..14d919e1 100644 --- a/frontend/src/components/Playlists.tsx +++ b/frontend/src/components/Playlists.tsx @@ -882,11 +882,11 @@ export default function Playlists() { onBlur={saveRename} onKeyDown={(e) => { if (e.key === "Enter") { - renameFocus.arm(); + renameFocus.arm(e); saveRename(); } if (e.key === "Escape") { - renameFocus.arm(); + renameFocus.arm(e); setRenaming(false); } }} diff --git a/frontend/src/components/PlaylistsRail.rename.dom.test.tsx b/frontend/src/components/PlaylistsRail.rename.dom.test.tsx index 0d1bea41..bed68b29 100644 --- a/frontend/src/components/PlaylistsRail.rename.dom.test.tsx +++ b/frontend/src/components/PlaylistsRail.rename.dom.test.tsx @@ -103,7 +103,8 @@ describe("PlaylistsRail · rename from the keyboard", () => { it("Escape cancels and puts the focus back on the row", async () => { const field = await openRename(); fireEvent.change(field, { target: { value: "alpha2" } }); - fireEvent.keyDown(field, { key: "Escape" }); + // Cancelled: the closing key must not go on to press the row the focus lands on (E28 audit). + expect(fireEvent.keyDown(field, { key: "Escape" })).toBe(false); await vi.waitFor(() => expect(document.activeElement).toBe(rowButton())); expect(rename).not.toHaveBeenCalled(); }); @@ -111,7 +112,7 @@ describe("PlaylistsRail · rename from the keyboard", () => { it("Enter commits and puts the focus back on the row", async () => { const field = await openRename(); fireEvent.change(field, { target: { value: "beta" } }); - fireEvent.keyDown(field, { key: "Enter" }); + expect(fireEvent.keyDown(field, { key: "Enter" })).toBe(false); expect(rename).toHaveBeenCalledWith(1, "beta"); await vi.waitFor(() => expect(document.activeElement).toBe(rowButton())); }); diff --git a/frontend/src/components/PlaylistsRail.tsx b/frontend/src/components/PlaylistsRail.tsx index 137a5d68..ed9616e7 100644 --- a/frontend/src/components/PlaylistsRail.tsx +++ b/frontend/src/components/PlaylistsRail.tsx @@ -332,12 +332,16 @@ export default function PlaylistsRail({ onChange={(e) => setRenameValue(e.target.value)} onBlur={() => void commitRename(pl)} onKeyDown={(e) => { + // preventDefault: the focus lands on the row before this keystroke is done, + // and a real Enter's own activation would then press it (see useReturnFocus). if (e.key === "Enter") { + e.preventDefault(); refocusRowRef.current = pl.id; void commitRename(pl); } // Unmounting the input fires no blur in React, so Escape really does cancel. if (e.key === "Escape") { + e.preventDefault(); refocusRowRef.current = pl.id; setRenamingId(null); } diff --git a/frontend/src/components/PlexCollectionView.tsx b/frontend/src/components/PlexCollectionView.tsx index a4a289bd..db764cd2 100644 --- a/frontend/src/components/PlexCollectionView.tsx +++ b/frontend/src/components/PlexCollectionView.tsx @@ -282,11 +282,11 @@ export default function PlexCollectionView({ onBlur={doRename} onKeyDown={(e) => { if (e.key === "Enter") { - renameFocus.arm(); + renameFocus.arm(e); doRename(); } if (e.key === "Escape") { - renameFocus.arm(); + renameFocus.arm(e); setRenaming(false); } }} diff --git a/frontend/src/components/PlexPlaylistView.header.dom.test.tsx b/frontend/src/components/PlexPlaylistView.header.dom.test.tsx index 6bceb0b8..405720d8 100644 --- a/frontend/src/components/PlexPlaylistView.header.dom.test.tsx +++ b/frontend/src/components/PlexPlaylistView.header.dom.test.tsx @@ -5,7 +5,7 @@ // (E9 review). The runtime sums the WHOLE list, an unknown duration counts as nothing, and a list // nothing sums for says no runtime at all. import { describe, it, expect, vi, beforeAll, afterEach } from "vitest"; -import { render, cleanup, screen } from "@testing-library/react"; +import { render, cleanup, screen, fireEvent } from "@testing-library/react"; import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; vi.mock("react-i18next", async (importOriginal) => ({ @@ -71,3 +71,21 @@ describe("PlexPlaylistView · the header's meta line", () => { expect(await meta([movie("1")])).toBe("plex.browse.items"); }); }); + +// E28 audit: Enter on an unchanged (or emptied) title wrote the same name back to Plex, and an empty +// one armed the focus hand-back while leaving the field open. Now both close with no write, as the +// collection rename does. +describe("PlexPlaylistView · rename", () => { + it("closes without a write when the title is unchanged or empty", async () => { + const write = vi.spyOn(api, "plexRenamePlaylist").mockResolvedValue({} as never); + await meta([movie("1", 5400)]); + for (const value of [null, " "]) { + fireEvent.click(screen.getByRole("button", { name: "plex.playlist.rename" })); + const field = screen.getByRole("textbox", { name: "plex.playlist.rename" }); + if (value !== null) fireEvent.change(field, { target: { value } }); + fireEvent.keyDown(field, { key: "Enter" }); + await screen.findByRole("heading", { name: "Films" }); + } + expect(write).not.toHaveBeenCalled(); + }); +}); diff --git a/frontend/src/components/PlexPlaylistView.tsx b/frontend/src/components/PlexPlaylistView.tsx index 383da8f8..fcbb8557 100644 --- a/frontend/src/components/PlexPlaylistView.tsx +++ b/frontend/src/components/PlexPlaylistView.tsx @@ -401,7 +401,12 @@ export default function PlexPlaylistView({ async function rename() { const title = name.trim(); - if (!title) return; + // Nothing to write (as the collection rename): close, so an armed focus hand-back fires now and + // not on some later close. + if (!title || title === q.data?.title) { + setRenaming(false); + return; + } try { await api.plexRenamePlaylist(playlistId, title); } catch (e) { @@ -533,12 +538,12 @@ export default function PlexPlaylistView({ onChange={(e) => setName(e.target.value)} onKeyDown={(e) => { if (e.key === "Enter") { - renameFocus.arm(); + renameFocus.arm(e); rename(); } // Unmounting the field fires no blur, so Escape really does cancel. if (e.key === "Escape") { - renameFocus.arm(); + renameFocus.arm(e); setRenaming(false); } }} diff --git a/frontend/src/components/PlexSearchBox.tsx b/frontend/src/components/PlexSearchBox.tsx index b61ad262..ebab47a9 100644 --- a/frontend/src/components/PlexSearchBox.tsx +++ b/frontend/src/components/PlexSearchBox.tsx @@ -140,7 +140,7 @@ export default function PlexSearchBox() { id={listId} role="listbox" aria-label={t("plex.suggest.aria")} - className={`${SURFACE.menu} absolute left-0 right-0 top-full z-20 mt-2 max-h-[70vh] overflow-y-auto animate-[popIn_0.16s_ease]`} + className={`${SURFACE.menu} absolute left-0 right-0 top-full z-menu mt-2 max-h-[70vh] overflow-y-auto animate-[popIn_0.16s_ease]`} > {suggestions.length === 0 && suggestQ.isFetching && (
{t("plex.loading")}
diff --git a/frontend/src/components/Scheduler.dom.test.tsx b/frontend/src/components/Scheduler.dom.test.tsx index 512d1980..77872713 100644 --- a/frontend/src/components/Scheduler.dom.test.tsx +++ b/frontend/src/components/Scheduler.dom.test.tsx @@ -165,6 +165,16 @@ describe("Scheduler · the job interval editor", () => { ), ); }); + + // E28 audit: a real Enter re-opened the editor — the keystroke's own activation landed on the trigger + // the focus had just returned to. The closing keys cancel their default action. + it("closing by a key does not press the trigger again", async () => { + await mount(); + for (const key of ["Enter", "Escape"]) { + fireEvent.click(screen.getByRole("button", { name: /scheduler.everyMin/ })); + expect(fireEvent.keyDown(intervalBox()!, { key })).toBe(false); + } + }); }); describe("Scheduler · the maintenance batch editor", () => { diff --git a/frontend/src/components/Scheduler.tsx b/frontend/src/components/Scheduler.tsx index d099a41e..bb97e033 100644 --- a/frontend/src/components/Scheduler.tsx +++ b/frontend/src/components/Scheduler.tsx @@ -196,8 +196,16 @@ function JobRow({ aria-errormessage={valid ? undefined : rangeId} onChange={(e) => setDraft(e.target.value)} onKeyDown={(e) => { - if (e.key === "Enter") save(); - if (e.key === "Escape") close(); + // preventDefault: close() returns the focus to the trigger before this keystroke + // is done, and a real Enter's own activation would press it again (useReturnFocus). + if (e.key === "Enter") { + e.preventDefault(); + save(); + } + if (e.key === "Escape") { + e.preventDefault(); + close(); + } }} /> {t("scheduler.minutes")} diff --git a/frontend/src/components/SettingsPanel.tsx b/frontend/src/components/SettingsPanel.tsx index 2b83de7a..e9ed0c04 100644 --- a/frontend/src/components/SettingsPanel.tsx +++ b/frontend/src/components/SettingsPanel.tsx @@ -598,7 +598,12 @@ function PlexWatchSync() {

{t("settings.plexSync.intro")}

- !busy && toggle.mutate(v)} /> + !busy && toggle.mutate(v)} + /> {busy ? (
diff --git a/frontend/src/components/VideoCard.tsx b/frontend/src/components/VideoCard.tsx index 043911a3..083e8c8c 100644 --- a/frontend/src/components/VideoCard.tsx +++ b/frontend/src/components/VideoCard.tsx @@ -110,7 +110,7 @@ function Actions({ // overlay for the thumbnailed views, and inline beside the actions for the one-line row. const INFO_REVEAL = "reveal-hover opacity-0 group-hover:opacity-100 group-focus-within:opacity-100 focus-visible:opacity-100 transition-opacity"; -const INFO_OVERLAY = clsx(INFO_REVEAL, "absolute bottom-1.5 left-1.5 z-10"); +const INFO_OVERLAY = clsx(INFO_REVEAL, "absolute bottom-1.5 left-1.5 z-base"); // Plain left-click opens the in-app player (the experiment); Ctrl/Cmd/middle-click // keep the native "open youtube.com in a new tab" behavior via the underlying href. diff --git a/frontend/src/components/WatchPage.dom.test.tsx b/frontend/src/components/WatchPage.dom.test.tsx index 629298e0..74ef5eff 100644 --- a/frontend/src/components/WatchPage.dom.test.tsx +++ b/frontend/src/components/WatchPage.dom.test.tsx @@ -122,6 +122,8 @@ describe("WatchPage media element (S3 · 5a)", () => { await findByText(/PDF · 2\.3 MB/); const dl = container.querySelector('a[href="/f.pdf"]'); expect(dl).not.toBeNull(); + // E28: the page's one action is the filled primary, not a muted outline. + expect(dl!.className).toContain("state-filled"); }); it("omits the Download for a view-only file link (S5d)", async () => { diff --git a/frontend/src/components/WatchPage.tsx b/frontend/src/components/WatchPage.tsx index e2f8a488..2625fc42 100644 --- a/frontend/src/components/WatchPage.tsx +++ b/frontend/src/components/WatchPage.tsx @@ -305,7 +305,8 @@ export default function WatchPage() { /* A generic uploaded file has no player — just an info card (name/type/size) and, if the link allows it, a Download. The backend serves the file force-download regardless, so gating the button on allow_download only mirrors the owner's ACL, matching the media - branch's download affordance below. */ + branch's download affordance below. It is the page's one action, so it is the filled + primary (E28), as the unlock card's Watch is. */
@@ -320,7 +321,12 @@ export default function WatchPage() { .join(" · ")}
{meta.allow_download && ( - } className="mt-5"> + } + className="mt-5" + > {T.download} )} diff --git a/frontend/src/components/plexCards.tsx b/frontend/src/components/plexCards.tsx index 13aacbf1..53881e81 100644 --- a/frontend/src/components/plexCards.tsx +++ b/frontend/src/components/plexCards.tsx @@ -89,7 +89,7 @@ export function PlexPosterCard({ decoding="async" className="w-full h-full object-cover group-hover:scale-105 transition-transform duration-200" /> - {/* Playable-cost tint (always shown — a quick "will it direct-play" cue). z-20 keeps the dot + {/* Playable-cost tint (always shown — a quick "will it direct-play" cue). z-menu keeps the dot ABOVE the full-card hover-reveal overlay below (absolute inset-0, no pointer-events-none): without it the overlay sits on top and swallows the cursor, so the dot's native `title` tooltip never fired on hover (a regression once the reveal overlay was added). */} @@ -100,7 +100,7 @@ export function PlexPosterCard({ ); @@ -181,7 +181,7 @@ export function PlexPosterCard({ {/* Quick actions (playable cards): Info + watched toggle — revealed on hover, always on touch. */} {isPlayable && ( -
+
{ @@ -206,7 +206,7 @@ export function PlexPosterCard({ )} {pct > 0 && ( -
+
)} diff --git a/frontend/src/components/ui/Carousel.tsx b/frontend/src/components/ui/Carousel.tsx index c6295e31..02ada021 100644 --- a/frontend/src/components/ui/Carousel.tsx +++ b/frontend/src/components/ui/Carousel.tsx @@ -99,7 +99,7 @@ export function Carousel({ // removes it on touch, where the strip scrolls by swipe — so an invisible edge button never eats a tap. // Round, dark translucent: they sit over the pictures (A2). const arrowBtn = - "reveal-hover absolute top-1/2 -translate-y-1/2 z-10 backdrop-blur-xs border border-white/15 shadow-lg " + + "reveal-hover absolute top-1/2 -translate-y-1/2 z-base backdrop-blur-xs border border-white/15 shadow-lg " + "opacity-0 group-hover/car:opacity-100 focus-visible:opacity-100"; return ( diff --git a/frontend/src/components/ui/PriorityStepper.tsx b/frontend/src/components/ui/PriorityStepper.tsx index c5da938d..887a165a 100644 --- a/frontend/src/components/ui/PriorityStepper.tsx +++ b/frontend/src/components/ui/PriorityStepper.tsx @@ -174,10 +174,10 @@ export function PriorityStepper({ onBlur={commitEdit} onKeyDown={(e) => { if (e.key === "Enter") { - editFocus.arm(); + editFocus.arm(e); commitEdit(); } else if (e.key === "Escape") { - editFocus.arm(); + editFocus.arm(e); setEditing(false); } }} diff --git a/frontend/src/components/ui/form.dom.test.tsx b/frontend/src/components/ui/form.dom.test.tsx new file mode 100644 index 00000000..ebd0299f --- /dev/null +++ b/frontend/src/components/ui/form.dom.test.tsx @@ -0,0 +1,29 @@ +// @vitest-environment jsdom +// +// E28 audit: the Switch was a bare 36×20 track — under the 24 floor on a mouse, never the 44 touch box +// DESIGN.md A3 lists it under, and its name was optional, so the Plex-sync switch shipped nameless. +import { afterEach, describe, expect, it, vi } from "vitest"; +import { cleanup, fireEvent, render, screen } from "@testing-library/react"; + +import { Switch } from "./form"; + +afterEach(cleanup); + +describe("Switch", () => { + it("is a named switch whose own box meets the 24 floor and grows to the touch target", () => { + const onChange = vi.fn(); + render(); + const sw = screen.getByRole("switch", { name: "Dark mode" }); + expect(sw.getAttribute("aria-checked")).toBe("false"); + expect(sw.className).toContain("h-6"); + expect(sw.className).toContain("pointer-coarse:min-h-(--touch-target)"); + fireEvent.click(sw); + expect(onChange).toHaveBeenCalledWith(true); + }); + + it("cannot be rendered without a name", () => { + // @ts-expect-error — `label` is required: a switch has no text of its own to be named by. + render( {}} />); + expect(screen.getByRole("switch")).toBeTruthy(); + }); +}); diff --git a/frontend/src/components/ui/form.tsx b/frontend/src/components/ui/form.tsx index 92f63d68..15f33fba 100644 --- a/frontend/src/components/ui/form.tsx +++ b/frontend/src/components/ui/form.tsx @@ -6,8 +6,10 @@ import { SURFACE } from "./surfaces"; // Shared form/settings primitives, factored out of the many panels that each re-declared // them (Settings, Config, Stats, admin pages, Setup wizard). Visual contract unchanged. -/** Pill on/off switch — the bare control; compose it with your own label/row. Pass `label` for - * the accessible name (the visible row text) since the control itself has no inner text. */ +/** Pill on/off switch — the bare control; compose it with your own label/row. `label` is the + * accessible name (the visible row text), required: the control has no inner text to be named by. + * The button is its own box around the 36×20 track: 24 tall on a mouse (the B1 floor) and the 44 + * touch target on a coarse pointer (A3), a real box rather than an invisible hit area. */ export function Switch({ checked, onChange, @@ -16,7 +18,7 @@ export function Switch({ }: { checked: boolean; onChange: (v: boolean) => void; - label?: string; + label: string; disabled?: boolean; }) { // FocusSafeButton: a switch is disabled by its caller while the change it made is saving, and a @@ -29,13 +31,17 @@ export function Switch({ aria-label={label} disabled={disabled} onClick={() => onChange(!checked)} - className={`w-9 h-5 rounded-full transition relative shrink-0 disabled:opacity-50 disabled:cursor-not-allowed ${checked ? "bg-accent" : "bg-border"}`} + className="group inline-flex h-6 min-w-9 shrink-0 items-center justify-center rounded-full disabled:cursor-not-allowed disabled:opacity-50 pointer-coarse:min-h-(--touch-target) pointer-coarse:min-w-(--touch-target)" > + className={`relative h-5 w-9 rounded-full transition ${checked ? "bg-accent" : "bg-border"}`} + > + + ); } diff --git a/frontend/src/components/ui/gridColumns.ts b/frontend/src/components/ui/gridColumns.ts index c0bcb34a..f082cf01 100644 --- a/frontend/src/components/ui/gridColumns.ts +++ b/frontend/src/components/ui/gridColumns.ts @@ -1,5 +1,5 @@ // VirtualGrid's column-count policy, extracted as a pure function so the viewport contract's UPPER -// end is unit-testable without a layout engine (the lower end, the 1024x768 floor, needs no policy — +// end is unit-testable without a layout engine (the lower end, the 768-wide tablet floor, needs no policy — // the grid simply drops to the columns that fit). // // An auto-fill grid answers "more room" with more columns, never bigger ones, so it always settles diff --git a/frontend/src/components/ui/useReturnFocus.dom.test.tsx b/frontend/src/components/ui/useReturnFocus.dom.test.tsx new file mode 100644 index 00000000..e874f2aa --- /dev/null +++ b/frontend/src/components/ui/useReturnFocus.dom.test.tsx @@ -0,0 +1,46 @@ +// @vitest-environment jsdom +// +// E28 audit: an in-place editor closed by a REAL Enter re-opened at once. The keydown commits and hands +// the focus back to the trigger button; the same keystroke's default action (the keypress activation) +// then lands on that button and clicks it. A synthetic keydown has no default action, so the first +// test passed while the live rename was broken. Arming now cancels the key's default action. +import { useState } from "react"; +import { afterEach, describe, expect, it } from "vitest"; +import { cleanup, fireEvent, render, screen } from "@testing-library/react"; + +import { useReturnFocus } from "./useReturnFocus"; + +afterEach(cleanup); + +function Editor() { + const [open, setOpen] = useState(false); + const focus = useReturnFocus(open); + return open ? ( + { + if (e.key === "Enter" || e.key === "Escape") { + focus.arm(e); + setOpen(false); + } + }} + /> + ) : ( + + ); +} + +describe("useReturnFocus", () => { + it("returns the focus to the trigger, and the closing key does not press it again", () => { + render(); + for (const key of ["Enter", "Escape"]) { + fireEvent.click(screen.getByRole("button", { name: "Rename" })); + // fireEvent returns false when a handler cancelled the event's default action. + expect(fireEvent.keyDown(screen.getByRole("textbox", { name: "name" }), { key })).toBe(false); + expect(document.activeElement).toBe(screen.getByRole("button", { name: "Rename" })); + } + }); +}); diff --git a/frontend/src/components/ui/useReturnFocus.ts b/frontend/src/components/ui/useReturnFocus.ts index 31849483..3622e95c 100644 --- a/frontend/src/components/ui/useReturnFocus.ts +++ b/frontend/src/components/ui/useReturnFocus.ts @@ -4,8 +4,13 @@ import { useCallback, useEffect, useRef } from "react"; // and back. Leaving it by Enter or Escape unmounts the focused field, and the browser drops the focus // on : the next Tab starts again at the top of the page (WCAG 2.4.3). This hands the focus back // to the trigger once the editor has CLOSED — after the commit, so the trigger is mounted and visible -// again whether it stayed in the tree or was swapped out. `arm()` it from the keys only: leaving by a +// again whether it stayed in the tree or was swapped out. `arm(e)` it from the keys only: leaving by a // click elsewhere means the user already put the focus where they wanted it. +// +// `arm` takes the key event and cancels its default action. The focus lands on the trigger BEFORE the +// same keystroke has finished: a real Enter's keypress then activates the trigger button and re-opens +// the editor it just closed (the playlist rename did exactly that; a synthetic keydown in a test has no +// default action, so only a real key showed it). export function useReturnFocus(open: boolean) { const target = useRef(null); const armed = useRef(false); @@ -14,7 +19,8 @@ export function useReturnFocus(open: armed.current = false; target.current?.focus(); }, [open]); - const arm = useCallback(() => { + const arm = useCallback((e: { preventDefault(): void }) => { + e.preventDefault(); armed.current = true; }, []); return { target, arm }; diff --git a/frontend/src/index.css b/frontend/src/index.css index f8d22495..634969c7 100644 --- a/frontend/src/index.css +++ b/frontend/src/index.css @@ -1035,11 +1035,10 @@ html[data-theme="light"] { .player-stage--max { position: fixed; inset: 0; - /* rail-level (40): this stage only has to beat the modal chrome it is nested inside (whose own - children top out at z-menu/20). The modal root is a z-overlay stacking context, so nothing - here escapes it — 9999 is the glass tuner's reserved level and claimed an authority this - layer neither has nor needs. */ - z-index: 40; + /* rail level: this stage only has to beat the modal chrome it is nested inside (whose own + children top out at the menu level). The modal root is a z-overlay stacking context, so nothing + here escapes it — a higher level would claim an authority this layer neither has nor needs. */ + z-index: var(--z-index-rail); } /* Thin, theme-aware scrollbars */ diff --git a/frontend/src/lib/feedView.ts b/frontend/src/lib/feedView.ts index 67d8cfdc..e99f39d5 100644 --- a/frontend/src/lib/feedView.ts +++ b/frontend/src/lib/feedView.ts @@ -23,7 +23,7 @@ export interface FeedViewSpec { * scan, so the stacked row keeps a cap; the one-line row doesn't need as tight a one. */ colMaxWidth: number | null; /** Multi-column views only: the most columns to lay out (null = as many as fit). The UPPER end of - * the 1024x768..ultrawide contract (E10 S4). An auto-fill grid always settles just above `minCol`, + * the tablet..ultrawide contract (E10 S4; tablet portrait since E28). An auto-fill grid always settles just above `minCol`, * so without a cap a wider screen buys more, smaller items rather than better ones — measured at * 3440px, `cards` laid out 11 columns of 264px. Each cap here is set so the item tops out near * 1.5x its own `minCol` on a 3440px ultrawide, which keeps each view recognisably itself (a capped diff --git a/frontend/src/lib/zLayer.contract.test.ts b/frontend/src/lib/zLayer.contract.test.ts new file mode 100644 index 00000000..3b52e605 --- /dev/null +++ b/frontend/src/lib/zLayer.contract.test.ts @@ -0,0 +1,34 @@ +import { fileURLToPath } from "node:url"; + +import { ESLint } from "eslint"; +import { describe, expect, it } from "vitest"; + +// The layer-scale rule in eslint.config.js (DESIGN.md Tokens: "No ad-hoc z-index"). A numeric `z-10` +// happened to equal `base` and passed every review; a `no-restricted-syntax` selector that matches +// nothing looks exactly like a clean codebase, so pin that it fires. +const FRONTEND = fileURLToPath(new URL("../../", import.meta.url)); + +async function refused(source: string): Promise { + const eslint = new ESLint({ cwd: FRONTEND }); + const [result] = await eslint.lintText(source, { filePath: "src/components/__probe.tsx" }); + return (result?.messages ?? []).filter((m) => m.message.startsWith("z-index off the layer scale")) + .length; +} + +describe("a z-index off the layer scale", () => { + // Assembled, so this file does not trip the very rule it probes. + const z = (v: string) => `z-${v}`; + + it("is refused as a number, an arbitrary value or a variant, in a string and a template", async () => { + expect(await refused(`export const a = "absolute ${z("10")} top-0";\n`)).toBe(1); + expect(await refused(`export const b = "hover:${z("[5]")}";\n`)).toBe(1); + expect(await refused("export const c = `x ${1} -" + z("20") + "`;\n")).toBe(1); + }); + + it("leaves a named level, z-0 and z-auto alone", async () => { + expect(await refused(`export const d = "${z("base")} ${z("menu")} ${z("tooltip")}";\n`)).toBe( + 0, + ); + expect(await refused(`export const e = "${z("0")} md:${z("auto")}";\n`)).toBe(0); + }); +});