From 0b252403cd668800f667b3c3cda73238c77a46e6 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Sun, 9 Aug 2026 17:23:09 +0200 Subject: [PATCH] fix(web): fix the card inset at the root, not at the call sites MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The broken inset on the Displays configuration card was the symptom. The cause is structural, and it had already been diagnosed at least twice in-tree without being fixed. Two faults, both in components/ui/card.tsx: 1. The padding was a RESPONSIVE COMPOUND: `p-4 pt-0 sm:p-6 sm:pt-0`. tailwind-merge resolves conflicts only within a variant, so any call-site override won at the base and lost at `sm:` — correct on a phone, wrong on every desktop. Measured on the Displays card before this change: padding-top 24px at 500px, 0px at 1440px. 2. `pt-0` encoded an assumption about a SIBLING that nothing enforced — "a CardHeader is above me and supplies the top inset". Delete the header, which is exactly what tabbing a page does since the tab label replaces the card title, and the top inset silently vanishes at ≥640px. Fix: - One single-variant utility, `p-padding-card` — the same `--spacing-padding-card` token @unom/ui's own Card uses, so nested cards finally agree on their inset. A single variant cannot half-lose an override. - Top inset is now self-correcting: `[&:not(:first-child)]:pt-0`. Ask the DOM instead of the author. A headerless CardContent keeps its inset with nothing to remember. Seven call sites had grown their own compensation in five dialects — `p-6`, `p-card pt-card sm:pt-card` (×3), `p-4 sm:pt-6` (×3), `pt-4 sm:pt-6`, and my own `pt-6` from the tabs commit. All removed; they are the symptom-fixes this replaces. LogsCard even carried a six-line comment correctly describing the trap and working around it locally — that comment is now three lines saying it no longer needs saying. `flush` stays: full-bleed content is a real intent, expressed as a prop the component honours rather than a utility that has to out-argue the one already there. Guarded by UI/Card → "Inset with and without header", a headered/headerless pair that has to look identical on every side. It must be checked at BOTH widths — a single width cannot show this class of bug, which is why it kept surviving. Verified by measuring computed padding at 500px and 1440px: first child 20px on all four sides, after-a-header 0px top and 20px elsewhere, identical at both widths. tsc clean, biome clean on every touched file, 9/9 server tests, build + i18n clean, 32/32 screenshots. --- web/src/components/ui/card.tsx | 41 +++++++++++++++++----- web/src/sections/Dashboard/view.tsx | 11 +++--- web/src/sections/Displays/DisplayCard.tsx | 4 +-- web/src/sections/Host/ConflictsCard.tsx | 2 +- web/src/sections/Logs/LogsCard.tsx | 11 +++--- web/src/sections/Pairing/PairedDevices.tsx | 2 +- web/src/sections/Store/JobProgress.tsx | 4 +-- web/src/stories/Card.stories.tsx | 36 +++++++++++++++++++ 8 files changed, 86 insertions(+), 25 deletions(-) diff --git a/web/src/components/ui/card.tsx b/web/src/components/ui/card.tsx index 257190a5..43a9b60f 100644 --- a/web/src/components/ui/card.tsx +++ b/web/src/components/ui/card.tsx @@ -27,13 +27,37 @@ const Card = ({ ); Card.displayName = "Card"; +/** + * The card inset, as ONE utility. + * + * It used to be `p-4 sm:p-6`, and that responsive pair is what made every padding override in this + * codebase unreliable: tailwind-merge resolves conflicts only *within* a variant, so a call-site + * `pt-6` beat the base `pt-0` and lost to `sm:pt-0` — correct on mobile, zero on desktop. Seven call + * sites had grown their own compensation for that in five different dialects. + * + * A single-variant token cannot half-lose. `--spacing-padding-card` is also what @unom/ui's own + * `Card` uses, so nested cards finally agree on their inset. + */ +const INSET = "p-padding-card"; + +/** + * Body/footer padding, minus the top when something already sits above. + * + * The old code hard-coded `pt-0` because "a CardHeader supplies the top inset" — an assumption about + * a SIBLING that nothing enforced. Delete the header (exactly what tabbing a page does, since the + * tab label replaces the card title) and the top inset silently vanished at ≥640px. Asking the DOM + * instead of the author makes it self-correcting: first child keeps its inset, later children drop + * it. + */ +const INSET_AFTER_SIBLING = `${INSET} [&:not(:first-child)]:pt-0`; + const CardHeader = React.forwardRef< HTMLDivElement, React.HTMLAttributes >(({ className, ...props }, ref) => (
)); @@ -67,11 +91,12 @@ CardDescription.displayName = "CardDescription"; * Card body. Pass `flush` for content that should meet the card's edges — a full-bleed table, most * commonly — instead of trying to cancel the padding from the outside. * - * `className="p-0"` does NOT work for that: tailwind-merge only resolves conflicts *within the same - * variant*, so `p-0` cancels `p-4` but leaves `sm:p-6` standing, and the padding silently returns at - * ≥640px. Every call site that tried it ended up with a doubled inset once a `CardHeader` (which - * brings its own `sm:p-6`) was nested inside — visible as one card whose title sits 24px further in - * than its neighbours'. + * Do NOT reach for `className="p-0"`: `flush` exists precisely so that intent is expressed as a prop + * the component honours, rather than as a utility that has to out-argue the one already there. + * + * Conversely, you no longer need to ADD top padding when there is no header — that is automatic now. + * If you find yourself writing `pt-*` on a CardContent, the layout is telling you something else is + * wrong. */ const CardContent = React.forwardRef< HTMLDivElement, @@ -79,7 +104,7 @@ const CardContent = React.forwardRef< >(({ className, flush = false, ...props }, ref) => (
)); @@ -91,7 +116,7 @@ const CardFooter = React.forwardRef< >(({ className, ...props }, ref) => (
)); diff --git a/web/src/sections/Dashboard/view.tsx b/web/src/sections/Dashboard/view.tsx index 792b3994..82da0cbc 100644 --- a/web/src/sections/Dashboard/view.tsx +++ b/web/src/sections/Dashboard/view.tsx @@ -62,7 +62,7 @@ export const DashboardView: FC<{ only the GameStream certs read as "0 paired" on a host every one of whose clients was in fact paired. */} - + {m.status_paired_count()} @@ -72,7 +72,7 @@ export const DashboardView: FC<{ - + {m.status_pin_pending()} @@ -206,7 +206,10 @@ export const DashboardView: FC<{ * else except the host log. */ const AudioWiringCard: FC<{ audio: AudioWiring }> = ({ audio }) => { - const badge: { variant: "success" | "secondary" | "destructive"; text: string } = + const badge: { + variant: "success" | "secondary" | "destructive"; + text: string; + } = audio.readiness === "full" ? { variant: "success", text: m.audio_ready() } : audio.readiness === "audio_only" @@ -257,7 +260,7 @@ const StatCard: FC<{ icon: ReactNode; label: string; on: boolean }> = ({ on, }) => ( - + {icon} {label} diff --git a/web/src/sections/Displays/DisplayCard.tsx b/web/src/sections/Displays/DisplayCard.tsx index 5f430c4b..cbd86c13 100644 --- a/web/src/sections/Displays/DisplayCard.tsx +++ b/web/src/sections/Displays/DisplayCard.tsx @@ -267,13 +267,13 @@ export const DisplayTabs: FC<{ - {configuration} + {configuration} - {live} + {live} diff --git a/web/src/sections/Host/ConflictsCard.tsx b/web/src/sections/Host/ConflictsCard.tsx index 12c845eb..5c47ee33 100644 --- a/web/src/sections/Host/ConflictsCard.tsx +++ b/web/src/sections/Host/ConflictsCard.tsx @@ -28,7 +28,7 @@ export const ConflictsCard: FC = () => { if (conflicts.length === 0) return null; return ( - +

diff --git a/web/src/sections/Logs/LogsCard.tsx b/web/src/sections/Logs/LogsCard.tsx index c0dc62b1..b4f177fb 100644 --- a/web/src/sections/Logs/LogsCard.tsx +++ b/web/src/sections/Logs/LogsCard.tsx @@ -238,13 +238,10 @@ export const LogsCard: FC<{ return ( - {/* This card has no CardHeader, so it has to put the top padding back itself — and it - must do so at BOTH breakpoints. `CardContent` is `p-4 pt-0 sm:p-6 sm:pt-0`, and - tailwind-merge only resolves conflicts within the same variant: a bare `pt-6` cancels - `pt-0` but leaves `sm:pt-0` standing, so the padding was 24px on a phone and 0 on a - desktop, with the filter row touching the card's edge. (Same trap the `p-0` note in - components/ui/card.tsx describes, in the other direction.) */} - + {/* No CardHeader here, and that no longer needs saying: CardContent keeps its top inset + unless something precedes it. This card used to restore it by hand at both + breakpoints. */} +

{LEVELS.map((l) => ( diff --git a/web/src/sections/Pairing/PairedDevices.tsx b/web/src/sections/Pairing/PairedDevices.tsx index 90af41db..73fee68c 100644 --- a/web/src/sections/Pairing/PairedDevices.tsx +++ b/web/src/sections/Pairing/PairedDevices.tsx @@ -135,7 +135,7 @@ export const PairedDevices: FC<{

{m.pairing_native_devices()}

- + {rows.length === 0 ? ( m.pairing_native_empty() diff --git a/web/src/sections/Store/JobProgress.tsx b/web/src/sections/Store/JobProgress.tsx index 60c22e77..43ea7cc8 100644 --- a/web/src/sections/Store/JobProgress.tsx +++ b/web/src/sections/Store/JobProgress.tsx @@ -55,7 +55,7 @@ export const JobProgressSection: FC<{ if (!job.isError) return null; return ( - +

{m.store_job_lost()}

@@ -92,7 +92,7 @@ export const JobProgressCard: FC<{ className={failed ? "ring-2 ring-destructive/60" : undefined} aria-live="polite" > - +
{running ? ( diff --git a/web/src/stories/Card.stories.tsx b/web/src/stories/Card.stories.tsx index 21b79dbb..147a83f1 100644 --- a/web/src/stories/Card.stories.tsx +++ b/web/src/stories/Card.stories.tsx @@ -21,6 +21,42 @@ const meta = { export default meta; type Story = StoryObj; +/** + * The inset contract — the thing this card got wrong most often. + * + * `CardContent` drops its top padding only when something already sits above it. The pair below is + * the regression guard: both cards must show the same inset on every side, and the headerless one + * must not have its first line touching the top edge. + * + * It used to be wrong invisibly, and only on desktop. The padding was `p-4 pt-0 sm:p-6 sm:pt-0`, so + * a headerless card had to restore the top inset itself — and a call-site `pt-6` beat the base + * `pt-0` while losing to `sm:pt-0`, because tailwind-merge resolves conflicts only within a variant. + * Right on a phone, zero on a desktop. Seven call sites had grown their own workaround for it. + * + * ⚠ Check this at BOTH viewport widths. A single width cannot show that class of bug. + */ +export const InsetWithAndWithoutHeader: Story = { + render: () => ( +
+ + + With a header + + + The body drops its top inset because the header above already supplied + one. + + + + + No header, so the body keeps its own top inset — automatically, with + nothing for the call site to remember. + + +
+ ), +}; + export const HostCard: Story = { render: () => (