fix(design): an audit of the S3d claims — the float shadow, filter hints, one action size per row (E28 S3d)
- A glass role's shadow is one token (shadow-float); the header search's drop-shadow utility had replaced its top highlight, and the debug pill's shadow-lg did the same. The surface-role lint rule now refuses a shadow size beside a role. - Plex: every value that filters carries a hint (genre, certificate, studio had none), and the facts panel's year and certificate filter like the header's. filterKeyword -> filterValue. - Downloads: the row resized its buttons with a [&_button] override that the save link escaped (28px beside 32px); the size is the primitive's md rung now.
This commit is contained in:
+2
-1
@@ -174,7 +174,8 @@ Also in force:
|
||||
violation. A call site names the ROLE (`SURFACE.panel` / `drawer` / `card` / `menu` / `popover` / `tooltip`,
|
||||
`ui/surfaces.ts`), which brings the tier AND the radius; a page section is a panel, and what floats over
|
||||
undimmed content (a menu, a hover card, a status popover, a toast, a tooltip) is `.glass-menu`. A bare glass
|
||||
class is a lint error.
|
||||
class is a lint error. A role's depth is its own shadow (the top highlight + the drop); a `shadow-*` size beside
|
||||
a role would replace both and is a lint error too — a field that floats takes `shadow-float`, the same shadow.
|
||||
- **Static vs pressable (owner, S3d):** a label that does nothing (a topic, a keyword, a status, a count) is a
|
||||
flat, tonal `Badge` with no edge; everything that does something has an edge (a chip, a trigger, a field).
|
||||
A genre that filters the library is a chip; the same genre where it can't is a badge.
|
||||
|
||||
@@ -170,11 +170,20 @@ const SURFACE_OWNERS = new Set([
|
||||
"fieldStyles.ts", // the header search floats over the page
|
||||
]);
|
||||
const GLASS_TIER = /^glass(-card|-menu)?$/;
|
||||
// A glass surface's depth is its own box-shadow, whose inset top highlight a `shadow-*` utility would
|
||||
// replace wholesale (the header search and the debug pill lost theirs). A shadow COLOUR only sets a
|
||||
// variable, and `shadow-float` IS the surface's shadow, so both pass.
|
||||
const SHADOW_SIZE = /^shadow-(xs|sm|md|lg|xl|2xl|\[.+\]|\(.+\))$/;
|
||||
const isShadowSize = (w) => SHADOW_SIZE.test(w.slice(w.lastIndexOf(":") + 1));
|
||||
const isSurfaceRole = (n) =>
|
||||
n?.type === "MemberExpression" && n.object.type === "Identifier" && n.object.name === "SURFACE";
|
||||
const surfaceRole = {
|
||||
meta: {
|
||||
type: "problem",
|
||||
messages: {
|
||||
raw: "A bare `{{tier}}` class: use a role from ui/surfaces.ts (SURFACE.panel / drawer / card / menu / popover / tooltip), which brings the radius too.",
|
||||
shadow:
|
||||
"`{{shadow}}` on a glass surface replaces its own shadow and drops the top highlight; the role's depth is the intended one.",
|
||||
},
|
||||
},
|
||||
create(context) {
|
||||
@@ -191,6 +200,20 @@ const surfaceRole = {
|
||||
const tier = words.find((w) => GLASS_TIER.test(w));
|
||||
if (tier) context.report({ node, messageId: "raw", data: { tier } });
|
||||
};
|
||||
// A role plus a shadow size in one class list: `${SURFACE.x} shadow-lg` or clsx(SURFACE.x, "shadow-lg").
|
||||
const shadowBeside = (node, parts) => {
|
||||
if (!parts.some(isSurfaceRole)) return;
|
||||
for (const part of parts) {
|
||||
const text =
|
||||
part.type === "TemplateElement"
|
||||
? part.value.cooked
|
||||
: part.type === "Literal" && typeof part.value === "string"
|
||||
? part.value
|
||||
: "";
|
||||
const shadow = text.split(/\s+/).find(isShadowSize);
|
||||
if (shadow) context.report({ node, messageId: "shadow", data: { shadow } });
|
||||
}
|
||||
};
|
||||
return {
|
||||
Literal(node) {
|
||||
check(node, node.value);
|
||||
@@ -198,6 +221,12 @@ const surfaceRole = {
|
||||
TemplateElement(node) {
|
||||
check(node, node.value.cooked);
|
||||
},
|
||||
TemplateLiteral(node) {
|
||||
shadowBeside(node, [...node.quasis, ...node.expressions]);
|
||||
},
|
||||
CallExpression(node) {
|
||||
shadowBeside(node, node.arguments);
|
||||
},
|
||||
};
|
||||
},
|
||||
};
|
||||
|
||||
@@ -153,7 +153,7 @@ function RecorderPill() {
|
||||
<div
|
||||
role="group"
|
||||
aria-label={t("recorder.pill.label")}
|
||||
className={`relative ${SURFACE.floating} h-12 pl-3 sm:pl-4 pr-1.5 flex items-center gap-1.5 sm:gap-2 shadow-lg ${
|
||||
className={`relative ${SURFACE.floating} h-12 pl-3 sm:pl-4 pr-1.5 flex items-center gap-1.5 sm:gap-2 ${
|
||||
hidden ? "invisible" : ""
|
||||
}`}
|
||||
>
|
||||
|
||||
@@ -160,7 +160,7 @@ export default function DownloadCenter() {
|
||||
if (!tabs.some((tb) => tb.id === tab)) setTab("queue");
|
||||
|
||||
const saveBtn = (job: DownloadJob) => (
|
||||
<IconLink href={api.downloadFileUrl(job.id)} label={t("downloads.actions.download")}>
|
||||
<IconLink href={api.downloadFileUrl(job.id)} label={t("downloads.actions.download")} size="md">
|
||||
<Download />
|
||||
</IconLink>
|
||||
);
|
||||
|
||||
@@ -101,6 +101,26 @@ describe("PlexInfo enriched detail (page)", () => {
|
||||
expect(onFilter).toHaveBeenCalledWith({ keywords: ["heist"] });
|
||||
});
|
||||
|
||||
// E28: every value that filters says so on hover — the genre, certificate and studio had no hint
|
||||
// while the keyword and the year did — and a value filters in the facts panel as in the header.
|
||||
it("every filtering value carries a filter hint", () => {
|
||||
wrap(<PlexInfo detail={detail} variant="page" onFilter={() => {}} />);
|
||||
for (const v of ["heist", "Crime", "R", "Studio A"])
|
||||
for (const el of screen.getAllByText(v))
|
||||
expect(el.closest("[title]")?.getAttribute("title")).toBe(`plex.info.filterValue:${v}`);
|
||||
});
|
||||
|
||||
it("the facts panel's year and certificate filter like the header's", () => {
|
||||
const onFilter = vi.fn();
|
||||
wrap(<PlexInfo detail={detail} variant="page" onFilter={onFilter} />);
|
||||
const [, factsCert] = screen.getAllByText("R");
|
||||
fireEvent.click(factsCert!);
|
||||
expect(onFilter).toHaveBeenLastCalledWith({ contentRatings: ["R"] });
|
||||
const [, factsYear] = screen.getAllByText("2010");
|
||||
fireEvent.click(factsYear!);
|
||||
expect(onFilter).toHaveBeenLastCalledWith({ yearMin: 2010, yearMax: 2010 });
|
||||
});
|
||||
|
||||
it("crew members open the person page on click", () => {
|
||||
const onOpenPerson = vi.fn();
|
||||
wrap(<PlexInfo detail={detail} variant="page" onOpenPerson={onOpenPerson} />);
|
||||
|
||||
@@ -156,16 +156,42 @@ function FactsPanel({
|
||||
const tmdb = detail.tmdb;
|
||||
const studios = tmdb?.studios?.length ? tmdb.studios : detail.studio ? [detail.studio] : [];
|
||||
const rows: { key: string; label: string; node: ReactNode }[] = [];
|
||||
if (detail.year != null)
|
||||
rows.push({ key: "year", label: t("plex.info.facts.release"), node: detail.year });
|
||||
// The same values filter here as in the header above (year, certificate, studios).
|
||||
const year = detail.year;
|
||||
if (year != null)
|
||||
rows.push({
|
||||
key: "year",
|
||||
label: t("plex.info.facts.release"),
|
||||
node: (
|
||||
<Filterable
|
||||
title={onFilter ? t("plex.info.filterYear", { year }) : undefined}
|
||||
onClick={onFilter ? () => onFilter({ yearMin: year, yearMax: year }) : undefined}
|
||||
className="hover:text-accent"
|
||||
>
|
||||
{year}
|
||||
</Filterable>
|
||||
),
|
||||
});
|
||||
if (detail.duration_seconds)
|
||||
rows.push({
|
||||
key: "runtime",
|
||||
label: t("plex.info.facts.runtime"),
|
||||
node: formatRuntime(detail.duration_seconds),
|
||||
});
|
||||
if (detail.content_rating)
|
||||
rows.push({ key: "cert", label: t("plex.info.facts.cert"), node: detail.content_rating });
|
||||
const cert = detail.content_rating;
|
||||
if (cert)
|
||||
rows.push({
|
||||
key: "cert",
|
||||
label: t("plex.info.facts.cert"),
|
||||
node: (
|
||||
<Filterable
|
||||
onClick={onFilter ? () => onFilter({ contentRatings: [cert] }) : undefined}
|
||||
className="hover:text-accent"
|
||||
>
|
||||
{cert}
|
||||
</Filterable>
|
||||
),
|
||||
});
|
||||
if (tmdb?.budget)
|
||||
rows.push({ key: "budget", label: t("plex.info.facts.budget"), node: money(tmdb.budget) });
|
||||
if (tmdb?.revenue)
|
||||
@@ -230,7 +256,6 @@ function KeywordsPanel({
|
||||
<Filterable
|
||||
key={k}
|
||||
tag
|
||||
title={onFilter ? t("plex.info.filterKeyword", { name: k }) : undefined}
|
||||
onClick={onFilter ? () => onFilter({ keywords: [k] }) : undefined}
|
||||
>
|
||||
{k}
|
||||
|
||||
@@ -140,4 +140,18 @@ describe("DownloadRow · keyboard", () => {
|
||||
expect(screen.getByTitle("play").hasAttribute("data-nav-open")).toBe(true);
|
||||
expect(screen.getByTitle("delete").hasAttribute("data-nav-open")).toBe(false);
|
||||
});
|
||||
|
||||
// E28: the row once resized its actions with a `[&_button]:size-8` override, which a save LINK in the
|
||||
// same row escaped (28px beside 32px buttons). The size is the primitive's own rung, set by IconBtn.
|
||||
it("sizes its actions through the primitive's rung, not a descendant override", () => {
|
||||
const { container } = render(
|
||||
<DownloadRow job={baseJob}>
|
||||
<IconBtn onClick={() => {}} title="play">
|
||||
p
|
||||
</IconBtn>
|
||||
</DownloadRow>,
|
||||
);
|
||||
expect(screen.getByTitle("play").className).toContain("size-8");
|
||||
expect(container.innerHTML).not.toContain("[&_button]");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -141,6 +141,7 @@ export function IconBtn({
|
||||
return (
|
||||
<IconButton
|
||||
variant={danger ? "danger" : "ghost"}
|
||||
size="md"
|
||||
onClick={onClick}
|
||||
data-nav-open={open ? "" : undefined}
|
||||
label={title}
|
||||
@@ -304,7 +305,7 @@ export function DownloadRow({
|
||||
{/* Row action buttons: the md rung here (the row stands alone), the sm rung in the modals, one
|
||||
IconBtn for both. `shrink-0` + no wrap keeps them on the title's line; the text column
|
||||
shrinks instead. */}
|
||||
<div className="flex items-start gap-1 shrink-0 [&_button]:size-8">{children}</div>
|
||||
<div className="flex items-start gap-1 shrink-0">{children}</div>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
@@ -22,8 +22,9 @@ import { Chip } from "./ui/Chip";
|
||||
// customize menu that toggles them. The prefs hook, backdrop helpers and epLabel live in plexDetail.ts.
|
||||
|
||||
// A metadata value that filters the grid when clicked (if onClick is wired), else plain text. Shared
|
||||
// by the movie info page (PlexInfo) and the unified grid / show pages (PlexBrowse). The optional
|
||||
// `title` gives a hover hint (e.g. "Filter by 1998"); callers that don't need it just omit it.
|
||||
// by the movie info page (PlexInfo) and the unified grid / show pages (PlexBrowse). A filtering value
|
||||
// always says so on hover: `title` is a caller's specific hint ("Show titles from 1998"), else the
|
||||
// generic "Filter by …" from the value's own text.
|
||||
export function Filterable({
|
||||
onClick,
|
||||
title,
|
||||
@@ -41,9 +42,15 @@ export function Filterable({
|
||||
onDark?: boolean;
|
||||
children: ReactNode;
|
||||
}) {
|
||||
const { t } = useTranslation();
|
||||
const hint =
|
||||
title ??
|
||||
(onClick && (typeof children === "string" || typeof children === "number")
|
||||
? t("plex.info.filterValue", { name: children })
|
||||
: undefined);
|
||||
if (tag)
|
||||
return onClick ? (
|
||||
<Chip variant="token" onClick={onClick} title={title}>
|
||||
<Chip variant="token" onClick={onClick} title={hint}>
|
||||
{children}
|
||||
</Chip>
|
||||
) : (
|
||||
@@ -53,7 +60,7 @@ export function Filterable({
|
||||
return (
|
||||
<Pressable
|
||||
onClick={onClick}
|
||||
title={title}
|
||||
title={hint}
|
||||
className={`${className ?? ""} cursor-pointer hover:text-accent transition`}
|
||||
>
|
||||
{children}
|
||||
|
||||
@@ -397,6 +397,7 @@ export function PlexShowView({
|
||||
<div className="mt-1 flex flex-wrap items-center gap-x-2 gap-y-0.5 text-sm text-muted">
|
||||
{show.year != null && (
|
||||
<Filterable
|
||||
title={onFilter ? t("plex.info.filterYear", { year: show.year }) : undefined}
|
||||
onClick={
|
||||
onFilter
|
||||
? () => onFilter({ yearMin: show.year, yearMax: show.year })
|
||||
@@ -407,15 +408,18 @@ export function PlexShowView({
|
||||
</Filterable>
|
||||
)}
|
||||
{show.content_rating && (
|
||||
<Filterable
|
||||
onClick={
|
||||
onFilter
|
||||
? () => onFilter({ contentRatings: [show.content_rating!] })
|
||||
: undefined
|
||||
}
|
||||
>
|
||||
· {show.content_rating}
|
||||
</Filterable>
|
||||
<span>
|
||||
·{" "}
|
||||
<Filterable
|
||||
onClick={
|
||||
onFilter
|
||||
? () => onFilter({ contentRatings: [show.content_rating!] })
|
||||
: undefined
|
||||
}
|
||||
>
|
||||
{show.content_rating}
|
||||
</Filterable>
|
||||
</span>
|
||||
)}
|
||||
{show.season_count != null && (
|
||||
<span>· {t("plex.seasons", { count: show.season_count })}</span>
|
||||
@@ -432,6 +436,9 @@ export function PlexShowView({
|
||||
onClick={
|
||||
onFilter ? () => onFilter({ ratingMin: Math.floor(score) }) : undefined
|
||||
}
|
||||
title={
|
||||
onFilter ? t("plex.info.filterRating", { n: Math.floor(score) }) : undefined
|
||||
}
|
||||
className={`inline-flex items-center gap-1 ${onFilter ? "cursor-pointer hover:text-accent" : "cursor-default"}`}
|
||||
>
|
||||
{/* decorative rating gold — intentionally NOT a status token */}
|
||||
|
||||
@@ -40,7 +40,8 @@ const BASE =
|
||||
|
||||
// `floating`: the field hovers over the scrolling page (the header search), so it keeps the glass-menu
|
||||
// surface under its edge instead of a see-through one (A2: what floats over content is glass). The
|
||||
// shadow is a utility so the focus ring's box-shadow composes with it instead of dropping it.
|
||||
// shadow is the `shadow-float` utility (highlight + drop, the menu's own) so the focus ring's box-shadow
|
||||
// composes with it instead of dropping it.
|
||||
export function fieldClass({
|
||||
size = "md",
|
||||
pill = false,
|
||||
@@ -51,6 +52,6 @@ export function fieldClass({
|
||||
BASE,
|
||||
SIZES[size],
|
||||
pill ? "rounded-full" : "rounded-lg",
|
||||
floating ? "glass-menu shadow-[0_18px_44px_-16px_rgba(0,0,0,0.6)]" : "bg-transparent",
|
||||
floating ? "glass-menu shadow-float" : "bg-transparent",
|
||||
);
|
||||
}
|
||||
|
||||
@@ -267,7 +267,7 @@
|
||||
"audience": "Audience",
|
||||
"keywords": "Keywords",
|
||||
"crew": "Crew",
|
||||
"filterKeyword": "Filter by \"{{name}}\"",
|
||||
"filterValue": "Filter by \"{{name}}\"",
|
||||
"facts": {
|
||||
"title": "Details",
|
||||
"release": "Released",
|
||||
|
||||
@@ -267,7 +267,7 @@
|
||||
"audience": "Közönség",
|
||||
"keywords": "Kulcsszavak",
|
||||
"crew": "Stáb",
|
||||
"filterKeyword": "Szűrés erre: „{{name}}”",
|
||||
"filterValue": "Szűrés erre: „{{name}}”",
|
||||
"facts": {
|
||||
"title": "Részletek",
|
||||
"release": "Megjelenés",
|
||||
|
||||
@@ -18,6 +18,12 @@
|
||||
makes the v4 opacity modifier (bg-accent/40 → color-mix(… var(--accent) 40% …)) pick
|
||||
up the active scheme's color. Same pattern as shadcn's v4 theme setup. */
|
||||
@theme inline {
|
||||
/* A floating glass surface's shadow: the top specular highlight + the drop. One token so a field that
|
||||
floats (the header search, `shadow-float`) keeps the highlight a menu has — a bare drop-shadow
|
||||
utility would replace both. */
|
||||
--shadow-float:
|
||||
inset 0 1px 0 color-mix(in srgb, #fff calc(var(--glass-inset) - 2%), transparent),
|
||||
0 18px 44px -16px rgba(0, 0, 0, 0.6);
|
||||
--color-bg: var(--bg);
|
||||
--color-surface: var(--surface);
|
||||
--color-card: var(--card);
|
||||
@@ -499,9 +505,7 @@ body {
|
||||
#fff var(--glass-edge),
|
||||
color-mix(in srgb, var(--border) 80%, transparent)
|
||||
);
|
||||
box-shadow:
|
||||
inset 0 1px 0 color-mix(in srgb, #fff calc(var(--glass-inset) - 2%), transparent),
|
||||
0 18px 44px -16px rgba(0, 0, 0, 0.6);
|
||||
box-shadow: var(--shadow-float);
|
||||
}
|
||||
|
||||
/* R9: standard interactive-row recipe. The hover tint menu/list rows already use, PLUS the
|
||||
|
||||
@@ -42,3 +42,25 @@ describe("a bare glass tier", () => {
|
||||
).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
// A role brings its own shadow, whose inset top highlight a `shadow-*` size would replace wholesale.
|
||||
describe("a shadow size beside a role", () => {
|
||||
const SUR = 'import clsx from "clsx";\nimport { SURFACE } from "./ui/surfaces";\n';
|
||||
const tpl = (cls: string) =>
|
||||
`${SUR}export const A = () => <div className={\`\${SURFACE.floating} ${cls}\`} />;\n`;
|
||||
const call = (role: string, cls: string) =>
|
||||
`${SUR}export const B = () => <div className={clsx(SURFACE.${role}, "${cls}")} />;\n`;
|
||||
|
||||
it("is refused in a template and in clsx, variants included", async () => {
|
||||
expect(await hits(tpl("h-12 shadow-lg"))).toBe(1);
|
||||
expect(await hits(call("menu", "hover:shadow-2xl"))).toBe(1);
|
||||
});
|
||||
|
||||
it("passes a shadow colour, shadow-float, and a shadow with no role beside it", async () => {
|
||||
expect(await hits(tpl("shadow-black/20"))).toBe(0);
|
||||
expect(await hits(call("popover", "shadow-float"))).toBe(0);
|
||||
expect(await hits('export const E = () => <div className="rounded-lg shadow-lg" />;\n')).toBe(
|
||||
0,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user