From 6c2c01e67b7e12954920a4f739e20af2ddec0240 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Sun, 27 Sep 2026 05:11:34 +0200 Subject: [PATCH] =?UTF-8?q?fix(design):=20the=20S3g=20conformance=20sweep?= =?UTF-8?q?=20=E2=80=94=20primitives=20at=20their=20rung=20(E28=20S3g)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The probe re-run over the 15 pages at 1920x1080 found every button, icon button, chip, field, tab, segment and menu row drawn by its primitive except the S3h Plex picture, a div with the button role. It is a Pressable now, and the button-primitive lint rule refuses an element with role="button" outside the control primitives. - SegmentedToggle defaults to the toolbar rung (28, text-xs): the old small default had put the pager's page sizes and the YouTube result count on 10px type beside 28px buttons. The Plex match toggle asks for the small rung explicitly. - The language badge on the rail uses text-3xs (was an 8px one-off); the Plex playlist's watched mark and the Home hero's length use MEDIA_BADGE instead of hand-copied classes. - A media picture shows the pointer whether it is a link or a button. - DESIGN.md: a Conformance chapter (the four-step check before a UI change is done, with the S3g baseline), the segmented rung, two log rows. CLAUDE.md points at the check. --- CLAUDE.md | 2 +- docs/DESIGN.md | 27 ++++++++++++++++++- frontend/eslint.config.js | 14 ++++++++++ frontend/src/components/Feed.tsx | 1 + frontend/src/components/Home.tsx | 7 +++-- frontend/src/components/LanguageSwitcher.tsx | 6 +++-- frontend/src/components/PlexPlaylistView.tsx | 7 ++--- frontend/src/components/PlexSidebar.tsx | 1 + .../src/components/plexCards.dom.test.tsx | 5 +++- frontend/src/components/plexCards.tsx | 17 ++---------- frontend/src/components/ui/Pager.tsx | 1 + .../ui/SegmentedToggle.dom.test.tsx | 11 ++++++++ .../src/components/ui/SegmentedToggle.tsx | 6 +++-- frontend/src/components/ui/mediaCardStyles.ts | 6 +++-- .../ui/pressableSelectable.contract.test.ts | 1 + .../src/lib/buttonPrimitive.contract.test.ts | 11 ++++++++ 16 files changed, 90 insertions(+), 33 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index fc9ddad4..0e2fb8ef 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -52,7 +52,7 @@ how it gets built. Concretely: - **Challenge unsound requests.** If a request is technically ill-advised and you can back it with real arguments, push back or propose an alternative — don't comply blindly. - **Verify UI in the browser before calling it done.** tsc/unit/DOM tests miss visual and interaction - bugs; do a real visual check of any UI change. + bugs; do a real visual check of any UI change, and run DESIGN.md's four-step Conformance check. - **Be terse everywhere else.** Outside those decision points, lead with the answer or decision. During implementation and debugging especially: no restating the same point reworded, no filler. diff --git a/docs/DESIGN.md b/docs/DESIGN.md index a2981ead..524ed61f 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -18,6 +18,27 @@ owner settles it. - A rule stated here should also be enforced in code (an eslint rule, a primitive's type, a test). A rule that lives only in prose drifts. That is the finding that started this document. +## Conformance — the check before a UI change is done + +A screen is measured against this document, not against taste. Every change that renders something new +runs this check before it is called done (E28 S3g): + +1. **Primitives at a named rung.** Each control goes through its primitive (lint refuses the raw element, + and a `
` with it), at a rung or role this document names: a `Button` size, an + `IconButton` size, a `Chip` size, the `SegmentedToggle` rung, a `SURFACE` role. The lint holds the + element, not the rung, so the rung is checked by eye and by measure. +2. **The pattern's chapter, re-read.** The change matches the chapter of every pattern it touches, or it + opens a **Pending** item and a log row. It never quietly adds a second look. +3. **Measured.** On the changed pages, the rendered elements are grouped by kind (button, icon button, + chip, field, tab, segment, menu row, surface) and by their computed look (radius, height, type, fill, + edge). Each distinct look maps to a rung or role named here; a look that maps to none is a violation + or a new decision. The S3g baseline across the 15 pages at 1920 × 1080: every button, icon button, + chip, field, tab, segment and menu row is drawn by its primitive. What the measure still counts as + hand-drawn is by design: text links (no primitive), surfaces painted by their `SURFACE` role (a class, + which the measure cannot trace to a component) and the playlist table's own rows. +4. **Seen.** A screenshot of every changed state, thin edges at 1:1; the tablet widths (1024 × 768, a + narrow split screen) when the layout can reflow. Phones are the E29 epic's, not this check's. + ## The design lab `npm run dev`, then open **`http://localhost:5173/design-lab`**. The lab renders each open decision's variants @@ -189,7 +210,9 @@ Also in force: - **Unavailable with a reason** stays focusable: `aria-disabled` plus the reason in the label or tooltip, never native `disabled`, which drops it from the tab order with its explanation. [WCAG 2.1.1] - A choice between a few values is `SegmentedToggle`; a long list or a user's own list is `FilterSelect`. - Never a native `` (lint-enforced). Its rung is the toolbar's, 28 with `text-xs`, so it lines up + with the triggers and the pager's buttons; the 24 rung with the smallest type is only a side panel's + captioned match toggle (S3g: the old small default had put a pager and a toolbar count on 10px type). - A single-select row of filter chips goes through `FilterChipGroup` (its options are `Chip`s). - A button that starts slow work shows `loading` (a spinner, `aria-busy`) and can't be pressed twice. [Nielsen #1] @@ -564,3 +587,5 @@ pick a density step; it never changes the anatomy or loses an action. [Nielsen # | 2026-09-27 | C2 as built | The side panel's drawer is `ui/Modal` `drawer-start`; drawer = a coarse pointer or below 768; the pin offered in its header ≥ 1024 | A drawer of its own inside `SidePanel` | Esc, Back, the scrim, the trap and the scroll lock are the dialog's; a second implementation is how two shells drift | | 2026-09-27 | Narrow rows by container | The download row, the playlist table and the Channels tag row stack by a container query; a fact line never wraps | Breakpoints on the window | The iPad zoomed in, split screen and a column beside a rail are narrow while the window is not; the zoomed download row broke "18.9 / MB" apart | | 2026-09-27 | Phones deferred | A phone-only layout (the hamburger nav, a folded feed band) waits for its own epic; E28 stays desktop + tablet | Fix the phone views inside E28 | Principle 2; the first phone attempts made the feed worse, and the epic's review grows with every sprint | +| 2026-09-27 | Conformance check (S3g) | A four-step check before a UI change is done: primitive at a named rung, the chapter re-read, the rendered page measured by kind and look, every state seen | A reviewer's eye per sprint | The S1 probe found 69 button looks; after S3 every control kind is drawn by its primitive, and the check keeps a new look from arriving unnoticed | +| 2026-09-27 | Segmented rung | `SegmentedToggle` defaults to the toolbar rung (28, `text-xs`); 24 only for a panel's captioned match toggle | The small default | Two toolbars had inherited 10px type from the default beside 28px buttons | diff --git a/frontend/eslint.config.js b/frontend/eslint.config.js index 8662f2a7..1db7a315 100644 --- a/frontend/eslint.config.js +++ b/frontend/eslint.config.js @@ -109,6 +109,7 @@ const buttonPrimitive = { type: "problem", messages: { raw: "A raw <{{name}}> outside the control primitives: use ui/Button (a labelled action), IconButton (a glyph), Pressable (a card or row that is the control), Chip or DragHandle.", + role: 'A <{{name}} role="button"> is a hand-built button (its own Enter/Space, no focus-safe disabled, no touch target): use ui/Pressable for a surface that is the control.', }, }, create(context) { @@ -121,6 +122,19 @@ const buttonPrimitive = { const name = node.name.name; if (name === "button" || name === "FocusSafeButton") context.report({ node, messageId: "raw", data: { name } }); + // S3g: the same button spelled as a div with a role slipped past the rule above (my own + // S3h Plex picture). A component (capitalised) may take a role for its own reasons. + else if ( + /^[a-z]/.test(name) && + node.attributes.some( + (a) => + a.type === "JSXAttribute" && + a.name.name === "role" && + a.value?.type === "Literal" && + a.value.value === "button", + ) + ) + context.report({ node, messageId: "role", data: { name } }); }, }; }, diff --git a/frontend/src/components/Feed.tsx b/frontend/src/components/Feed.tsx index 70fa3af6..b2ebdf78 100644 --- a/frontend/src/components/Feed.tsx +++ b/frontend/src/components/Feed.tsx @@ -482,6 +482,7 @@ export default function Feed({
onPlay(card)} - className="relative block h-full w-full overflow-hidden rounded-lg border border-border bg-card" + className="relative block h-full w-full cursor-pointer overflow-hidden rounded-lg border border-border bg-card" aria-label={`${started ? t("home.hero.continue") : t("home.hero.play")}: ${card.title}`} > {card.thumb ? ( @@ -567,9 +568,7 @@ function Hero({ ) : null} {card.duration_seconds != null && ( // It steps aside while the hover row covers its corner, as a card's length does. - + {formatDuration(card.duration_seconds)} )} diff --git a/frontend/src/components/LanguageSwitcher.tsx b/frontend/src/components/LanguageSwitcher.tsx index a811d9c6..b5c0f8e8 100644 --- a/frontend/src/components/LanguageSwitcher.tsx +++ b/frontend/src/components/LanguageSwitcher.tsx @@ -60,10 +60,12 @@ export default function LanguageSwitcher({ > {rail ? ( /* A rail row (F32), with the current-language badge (HU/EN) on the glyph's corner so the - active language reads at a glance. The ring matches the page to detach it from the icon. */ + active language reads at a glance. The ring matches the page to detach it from the icon; + its type is the rail count badge's (text-3xs), the smallest step on the scale (it was an + 8px one-off). */ - + {current.code.toUpperCase()} diff --git a/frontend/src/components/PlexPlaylistView.tsx b/frontend/src/components/PlexPlaylistView.tsx index e1d1752b..36c06df5 100644 --- a/frontend/src/components/PlexPlaylistView.tsx +++ b/frontend/src/components/PlexPlaylistView.tsx @@ -53,6 +53,7 @@ import { Input } from "./ui/Input"; import { useReturnFocus } from "./ui/useReturnFocus"; import { SURFACE } from "./ui/surfaces"; import { PageHeading } from "./ui/Headings"; +import { MEDIA_BADGE } from "./ui/mediaCardStyles"; // A playlist's items, grouped for readability: movies stay standalone, while a run of episodes from the // same show collapses into a season/show group so a whole series doesn't sprawl into an endless flat @@ -199,11 +200,7 @@ function EpisodeRow({ ep, ctx, dense }: { ep: PlexCard; ctx: Ctx; dense: boolean className={`relative shrink-0 overflow-hidden rounded-sm border border-border bg-surface ${dense ? "aspect-video w-14" : "aspect-video w-20"}`} > - {ep.status === "watched" && ( - - ✓ - - )} + {ep.status === "watched" && ✓} {inProgress &&
}
diff --git a/frontend/src/components/PlexSidebar.tsx b/frontend/src/components/PlexSidebar.tsx index bbf14531..1b90444f 100644 --- a/frontend/src/components/PlexSidebar.tsx +++ b/frontend/src/components/PlexSidebar.tsx @@ -55,6 +55,7 @@ function MatchToggle({ const { t } = useTranslation(); return ( { const root = container.firstElementChild as HTMLElement; expect(root.getAttribute("data-nav-index")).toBe("3"); const open = root.querySelector("[data-nav-open]")!; - expect(open.getAttribute("role")).toBe("button"); + // A real button (ui/Pressable), not a div wearing its role: Enter/Space and the touch target + // come from the primitive (S3g found the S3h picture hand-built). + expect(open.tagName).toBe("BUTTON"); + expect(open.getAttribute("type")).toBe("button"); fireEvent.click(open); expect(onOpen).toHaveBeenCalledTimes(1); }); diff --git a/frontend/src/components/plexCards.tsx b/frontend/src/components/plexCards.tsx index 75c67cb4..6df9b1e7 100644 --- a/frontend/src/components/plexCards.tsx +++ b/frontend/src/components/plexCards.tsx @@ -209,27 +209,14 @@ function PlexPicture({ const started = plays && pct > 0 && !!onRestart; return (
-
{ - if (e.key === "Enter" || e.key === " ") { - e.preventDefault(); - onOpen(); - } - }} - className={clsx(MEDIA_PICTURE, "cursor-pointer")} - > + {children}
{plays && !started && }
-
+ {started && (
{/* A handful of fixed sizes plus "All": segmented, the house control for a short fixed set. */} { diff --git a/frontend/src/components/ui/SegmentedToggle.dom.test.tsx b/frontend/src/components/ui/SegmentedToggle.dom.test.tsx index a20cd4ae..4bf5364a 100644 --- a/frontend/src/components/ui/SegmentedToggle.dom.test.tsx +++ b/frontend/src/components/ui/SegmentedToggle.dom.test.tsx @@ -32,6 +32,17 @@ describe("SegmentedToggle", () => { expect(onChange).toHaveBeenCalledWith("and"); }); + it("sits on the toolbar rung unless a panel's caption asks for the small one (S3g)", () => { + render(); + const plain = screen.getByRole("group", { name: "Default" }); + expect(plain.className).toContain("text-xs"); + expect(plain.querySelector("button")!.className).toContain("h-7"); + render( + , + ); + expect(screen.getByRole("group", { name: "Small" }).className).toContain("text-3xs"); + }); + it("exposes the group aria-label", () => { render(); expect(screen.getByRole("group", { name: "Match mode" })).toBeTruthy(); diff --git a/frontend/src/components/ui/SegmentedToggle.tsx b/frontend/src/components/ui/SegmentedToggle.tsx index 8ea681cc..e28b73bb 100644 --- a/frontend/src/components/ui/SegmentedToggle.tsx +++ b/frontend/src/components/ui/SegmentedToggle.tsx @@ -21,7 +21,7 @@ export default function SegmentedToggle({ label, title, fullWidth = false, - size = "sm", + size = "md", className, }: { options: readonly SegmentedOption[]; @@ -37,7 +37,9 @@ export default function SegmentedToggle({ title?: string; // Full-width track with flex-1 buttons (the scope bar); default is intrinsic width. fullWidth?: boolean; - // sm = text-3xs, compact padding (the captioned match toggles); md = text-xs (the scope bar). + // md (28, text-xs) is the toolbar rung: it lines up with the search pill, the triggers and an `sm` + // Button (DESIGN B1). sm (24, text-3xs) is only a side panel's captioned match toggle. md is the + // default because the old sm default slid a pager and a toolbar count onto 10px type (S3g). size?: "sm" | "md"; className?: string; }) { diff --git a/frontend/src/components/ui/mediaCardStyles.ts b/frontend/src/components/ui/mediaCardStyles.ts index 4efdbac2..b862911a 100644 --- a/frontend/src/components/ui/mediaCardStyles.ts +++ b/frontend/src/components/ui/mediaCardStyles.ts @@ -4,9 +4,11 @@ import { HOVER_REVEAL } from "./reveal"; // The one media card's classes (ui/MediaCard): the pieces a caller builds its picture from, kept in // a module of their own so the component file exports only components (fast refresh). -/** The picture's own box: rounded inside the frame, edged, lifting its shadow on hover. */ +/** The picture's own box: rounded inside the frame, edged, lifting its shadow on hover. It is always + * the card's open control, so it shows the hand whether it is a link (a video) or a button (a Plex + * title, a shelf tile), which a button would not by itself. */ export const MEDIA_PICTURE = - "relative block h-full w-full overflow-hidden rounded-xl border border-border bg-surface shadow-md transition-shadow group-hover:shadow-2xl"; + "relative block h-full w-full cursor-pointer overflow-hidden rounded-xl border border-border bg-surface shadow-md transition-shadow group-hover:shadow-2xl"; /** The picture's image: covers the box and zooms a little on hover. */ export const MEDIA_IMG = diff --git a/frontend/src/components/ui/pressableSelectable.contract.test.ts b/frontend/src/components/ui/pressableSelectable.contract.test.ts index 58964e8e..04c4d3ea 100644 --- a/frontend/src/components/ui/pressableSelectable.contract.test.ts +++ b/frontend/src/components/ui/pressableSelectable.contract.test.ts @@ -32,6 +32,7 @@ const PLAIN: Record = { "PlexPlaylistAdd.tsx": [1, "a checkbox row in a picker"], "PlexPlaylistView.tsx": [1, "a show's poster stack (aria-hidden)"], "PlexSearchBox.tsx": [1, "a listbox option"], + "plexCards.tsx": [1, "a card's picture (named by aria-label; its title is the caption)"], "Scheduler.tsx": [1, "the interval editor trigger"], "SettingsPanel.tsx": [1, "a colour swatch"], "SyncStatus.tsx": [1, "the status pill trigger"], diff --git a/frontend/src/lib/buttonPrimitive.contract.test.ts b/frontend/src/lib/buttonPrimitive.contract.test.ts index 9c3b2340..3cc4b8eb 100644 --- a/frontend/src/lib/buttonPrimitive.contract.test.ts +++ b/frontend/src/lib/buttonPrimitive.contract.test.ts @@ -42,4 +42,15 @@ describe("a hand-rolled button", () => { ), ).toBe(0); }); + + it("is refused spelled as an element with the button role; a component's role is its own", async () => { + expect(await hits('export const F = () =>
x
;\n')).toBe(1); + expect(await hits('export const G = () => x;\n')).toBe(1); + expect(await hits('export const H = () =>
x
;\n')).toBe(0); + expect( + await hits( + 'import { Pressable } from "./ui/Pressable";\nexport const I = () => x;\n', + ), + ).toBe(0); + }); });