fix(design): the S3g conformance sweep — primitives at their rung (E28 S3g)
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.
This commit is contained in:
@@ -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 } });
|
||||
},
|
||||
};
|
||||
},
|
||||
|
||||
@@ -482,6 +482,7 @@ export default function Feed({
|
||||
<div className="ml-auto flex items-center gap-1.5 text-xs text-muted shrink-0">
|
||||
<span aria-hidden="true">{t("feed.yt.show")}</span>
|
||||
<SegmentedToggle
|
||||
size="md"
|
||||
ariaLabel={t("feed.yt.show")}
|
||||
title={t("feed.yt.showHint")}
|
||||
value={String(ytCount)}
|
||||
|
||||
@@ -57,6 +57,7 @@ import { MediaResumeButtons } from "./MediaResumeButtons";
|
||||
import { MediaCard, MediaPlayGlyph } from "./ui/MediaCard";
|
||||
import {
|
||||
MEDIA_BAR,
|
||||
MEDIA_BADGE,
|
||||
MEDIA_HOVER_LAYER,
|
||||
MEDIA_IMG,
|
||||
MEDIA_PICTURE,
|
||||
@@ -559,7 +560,7 @@ function Hero({
|
||||
>
|
||||
<Pressable
|
||||
onClick={() => 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.
|
||||
<span
|
||||
className={`absolute bottom-1.5 right-1.5 rounded-sm bg-black/80 px-1.5 py-0.5 text-2xs font-medium text-white tabular-nums ${HOVER_YIELD}`}
|
||||
>
|
||||
<span className={`${MEDIA_BADGE} bottom-1.5 right-1.5 ${HOVER_YIELD}`}>
|
||||
{formatDuration(card.duration_seconds)}
|
||||
</span>
|
||||
)}
|
||||
|
||||
@@ -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). */
|
||||
<span className="relative">
|
||||
<Globe className={RAIL_GLYPH} />
|
||||
<span className="absolute -bottom-1.5 -right-2.5 text-[0.5rem] font-bold leading-none px-1 py-px rounded-sm bg-accent text-accent-fg ring-2 ring-bg">
|
||||
<span className="absolute -bottom-1.5 -right-2.5 text-3xs font-bold leading-none px-1 py-px rounded-sm bg-accent text-accent-fg ring-2 ring-bg">
|
||||
{current.code.toUpperCase()}
|
||||
</span>
|
||||
</span>
|
||||
|
||||
@@ -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"}`}
|
||||
>
|
||||
<img src={ep.thumb} alt="" loading="lazy" className="h-full w-full object-cover" />
|
||||
{ep.status === "watched" && (
|
||||
<span className="absolute right-0.5 top-0.5 rounded-sm bg-black/70 px-1 text-[0.5625rem] text-white">
|
||||
✓
|
||||
</span>
|
||||
)}
|
||||
{ep.status === "watched" && <span className={`${MEDIA_BADGE} right-0.5 top-0.5`}>✓</span>}
|
||||
{inProgress && <div className="absolute inset-x-0 bottom-0 h-0.5 bg-accent" />}
|
||||
<div className="absolute inset-0 grid place-items-center bg-black/40 opacity-0 transition group-hover/thumb:opacity-100">
|
||||
<Play className="h-4 w-4 text-white" fill="currentColor" />
|
||||
|
||||
@@ -55,6 +55,7 @@ function MatchToggle({
|
||||
const { t } = useTranslation();
|
||||
return (
|
||||
<SegmentedToggle
|
||||
size="sm"
|
||||
label={t("plex.filter.matchLabel")}
|
||||
ariaLabel={t("plex.filter.matchLabel")}
|
||||
options={[
|
||||
|
||||
@@ -270,7 +270,10 @@ describe("Plex cards in a keyboard-walkable list", () => {
|
||||
const root = container.firstElementChild as HTMLElement;
|
||||
expect(root.getAttribute("data-nav-index")).toBe("3");
|
||||
const open = root.querySelector<HTMLElement>("[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);
|
||||
});
|
||||
|
||||
@@ -209,27 +209,14 @@ function PlexPicture({
|
||||
const started = plays && pct > 0 && !!onRestart;
|
||||
return (
|
||||
<div className={mediaPictureBox(aspect)}>
|
||||
<div
|
||||
onClick={onOpen}
|
||||
role="button"
|
||||
tabIndex={0}
|
||||
aria-label={label}
|
||||
data-nav-open=""
|
||||
onKeyDown={(e) => {
|
||||
if (e.key === "Enter" || e.key === " ") {
|
||||
e.preventDefault();
|
||||
onOpen();
|
||||
}
|
||||
}}
|
||||
className={clsx(MEDIA_PICTURE, "cursor-pointer")}
|
||||
>
|
||||
<Pressable onClick={onOpen} aria-label={label} data-nav-open="" className={MEDIA_PICTURE}>
|
||||
<img src={thumb} alt="" loading="lazy" decoding="async" className={MEDIA_IMG} />
|
||||
{children}
|
||||
<div className={MEDIA_HOVER_LAYER}>
|
||||
{plays && !started && <MediaPlayGlyph small={small} />}
|
||||
</div>
|
||||
<MediaProgress pct={pct} />
|
||||
</div>
|
||||
</Pressable>
|
||||
{started && (
|
||||
<div
|
||||
className={clsx(
|
||||
|
||||
@@ -113,6 +113,7 @@ export default function Pager({
|
||||
</span>
|
||||
{/* A handful of fixed sizes plus "All": segmented, the house control for a short fixed set. */}
|
||||
<SegmentedToggle
|
||||
size="md"
|
||||
ariaLabel={sizeLabel}
|
||||
value={String(size)}
|
||||
onChange={(v) => {
|
||||
|
||||
@@ -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(<SegmentedToggle options={OPTS} value="or" onChange={vi.fn()} ariaLabel="Default" />);
|
||||
const plain = screen.getByRole("group", { name: "Default" });
|
||||
expect(plain.className).toContain("text-xs");
|
||||
expect(plain.querySelector("button")!.className).toContain("h-7");
|
||||
render(
|
||||
<SegmentedToggle options={OPTS} value="or" onChange={vi.fn()} ariaLabel="Small" size="sm" />,
|
||||
);
|
||||
expect(screen.getByRole("group", { name: "Small" }).className).toContain("text-3xs");
|
||||
});
|
||||
|
||||
it("exposes the group aria-label", () => {
|
||||
render(<SegmentedToggle options={OPTS} value="or" onChange={vi.fn()} ariaLabel="Match mode" />);
|
||||
expect(screen.getByRole("group", { name: "Match mode" })).toBeTruthy();
|
||||
|
||||
@@ -21,7 +21,7 @@ export default function SegmentedToggle<T extends string>({
|
||||
label,
|
||||
title,
|
||||
fullWidth = false,
|
||||
size = "sm",
|
||||
size = "md",
|
||||
className,
|
||||
}: {
|
||||
options: readonly SegmentedOption<T>[];
|
||||
@@ -37,7 +37,9 @@ export default function SegmentedToggle<T extends string>({
|
||||
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;
|
||||
}) {
|
||||
|
||||
@@ -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 =
|
||||
|
||||
@@ -32,6 +32,7 @@ const PLAIN: Record<string, [number, string]> = {
|
||||
"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"],
|
||||
|
||||
@@ -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 = () => <div role="button" tabIndex={0}>x</div>;\n')).toBe(1);
|
||||
expect(await hits('export const G = () => <span role="button">x</span>;\n')).toBe(1);
|
||||
expect(await hits('export const H = () => <div role="group">x</div>;\n')).toBe(0);
|
||||
expect(
|
||||
await hits(
|
||||
'import { Pressable } from "./ui/Pressable";\nexport const I = () => <Pressable role="button">x</Pressable>;\n',
|
||||
),
|
||||
).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user