fix(web): fix the card inset at the root, not at the call sites
ci / bun-nix (pull_request) Successful in 51s
ci / docs-site (pull_request) Successful in 1m35s
ci / web (pull_request) Successful in 2m30s
ci / rust-arm64 (pull_request) Successful in 3m16s
ci / rust (pull_request) Failing after 9m12s
nix / flake (pull_request) Failing after 19m50s
ci / bun-nix (pull_request) Successful in 51s
ci / docs-site (pull_request) Successful in 1m35s
ci / web (pull_request) Successful in 2m30s
ci / rust-arm64 (pull_request) Successful in 3m16s
ci / rust (pull_request) Failing after 9m12s
nix / flake (pull_request) Failing after 19m50s
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.
This commit is contained in:
@@ -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<HTMLDivElement>
|
||||
>(({ className, ...props }, ref) => (
|
||||
<div
|
||||
ref={ref}
|
||||
className={cn("flex flex-col space-y-1.5 p-4 sm:p-6", className)}
|
||||
className={cn("flex flex-col space-y-1.5", INSET, className)}
|
||||
{...props}
|
||||
/>
|
||||
));
|
||||
@@ -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) => (
|
||||
<div
|
||||
ref={ref}
|
||||
className={cn(!flush && "p-4 pt-0 sm:p-6 sm:pt-0", className)}
|
||||
className={cn(!flush && INSET_AFTER_SIBLING, className)}
|
||||
{...props}
|
||||
/>
|
||||
));
|
||||
@@ -91,7 +116,7 @@ const CardFooter = React.forwardRef<
|
||||
>(({ className, ...props }, ref) => (
|
||||
<div
|
||||
ref={ref}
|
||||
className={cn("flex items-center p-4 pt-0 sm:p-6 sm:pt-0", className)}
|
||||
className={cn("flex items-center", INSET_AFTER_SIBLING, className)}
|
||||
{...props}
|
||||
/>
|
||||
));
|
||||
|
||||
@@ -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. */}
|
||||
<Card>
|
||||
<CardContent className="flex flex-1 items-center justify-between p-4 sm:pt-6">
|
||||
<CardContent className="flex flex-1 items-center justify-between">
|
||||
<span className="text-sm text-muted-foreground">
|
||||
{m.status_paired_count()}
|
||||
</span>
|
||||
@@ -72,7 +72,7 @@ export const DashboardView: FC<{
|
||||
</CardContent>
|
||||
</Card>
|
||||
<Card>
|
||||
<CardContent className="flex flex-1 items-center justify-between p-4 sm:pt-6">
|
||||
<CardContent className="flex flex-1 items-center justify-between">
|
||||
<span className="text-sm text-muted-foreground">
|
||||
{m.status_pin_pending()}
|
||||
</span>
|
||||
@@ -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,
|
||||
}) => (
|
||||
<Card>
|
||||
<CardContent className="flex flex-1 items-center justify-between p-4 sm:pt-6">
|
||||
<CardContent className="flex flex-1 items-center justify-between">
|
||||
<span className="flex items-center gap-2 text-sm text-muted-foreground">
|
||||
{icon}
|
||||
{label}
|
||||
|
||||
@@ -267,13 +267,13 @@ export const DisplayTabs: FC<{
|
||||
|
||||
<TabsContent value="configuration">
|
||||
<Card>
|
||||
<CardContent className="space-y-4 pt-6">{configuration}</CardContent>
|
||||
<CardContent className="space-y-4">{configuration}</CardContent>
|
||||
</Card>
|
||||
</TabsContent>
|
||||
|
||||
<TabsContent value="live">
|
||||
<Card>
|
||||
<CardContent className="pt-6">{live}</CardContent>
|
||||
<CardContent>{live}</CardContent>
|
||||
</Card>
|
||||
</TabsContent>
|
||||
</Tabs>
|
||||
|
||||
@@ -28,7 +28,7 @@ export const ConflictsCard: FC = () => {
|
||||
if (conflicts.length === 0) return null;
|
||||
return (
|
||||
<Card className="border-amber-600/40 dark:border-amber-500/40">
|
||||
<CardContent className="flex items-start gap-3 p-card pt-card sm:pt-card">
|
||||
<CardContent className="flex items-start gap-3">
|
||||
<AlertTriangle className="mt-0.5 size-5 shrink-0 text-amber-600 dark:text-amber-500" />
|
||||
<div className="min-w-0 flex-1 space-y-2">
|
||||
<p className="text-sm font-medium text-amber-600 dark:text-amber-500">
|
||||
|
||||
@@ -238,13 +238,10 @@ export const LogsCard: FC<{
|
||||
|
||||
return (
|
||||
<Card>
|
||||
{/* 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.) */}
|
||||
<CardContent className="flex flex-col gap-3 pt-4 sm:pt-6">
|
||||
{/* 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. */}
|
||||
<CardContent className="flex flex-col gap-3">
|
||||
<div className="flex flex-wrap items-center gap-2">
|
||||
<div className="flex items-center gap-1">
|
||||
{LEVELS.map((l) => (
|
||||
|
||||
@@ -135,7 +135,7 @@ export const PairedDevices: FC<{
|
||||
<h2 className="text-lg font-medium">{m.pairing_native_devices()}</h2>
|
||||
</CardHeader>
|
||||
|
||||
<CardContent className="p-6">
|
||||
<CardContent>
|
||||
<QueryState isLoading={isLoading} error={error} refetch={refetch}>
|
||||
{rows.length === 0 ? (
|
||||
m.pairing_native_empty()
|
||||
|
||||
@@ -55,7 +55,7 @@ export const JobProgressSection: FC<{
|
||||
if (!job.isError) return null;
|
||||
return (
|
||||
<Card className="ring-2 ring-destructive/60">
|
||||
<CardContent className="flex items-start gap-3 p-card pt-card sm:pt-card">
|
||||
<CardContent className="flex items-start gap-3">
|
||||
<XCircle className="mt-0.5 size-5 shrink-0 text-destructive" />
|
||||
<div className="min-w-0 flex-1">
|
||||
<p className="text-sm font-medium">{m.store_job_lost()}</p>
|
||||
@@ -92,7 +92,7 @@ export const JobProgressCard: FC<{
|
||||
className={failed ? "ring-2 ring-destructive/60" : undefined}
|
||||
aria-live="polite"
|
||||
>
|
||||
<CardContent className="space-y-3 p-card pt-card sm:pt-card">
|
||||
<CardContent className="space-y-3">
|
||||
<div className="flex items-start gap-3">
|
||||
{running ? (
|
||||
<Spinner className="mt-0.5 size-5 shrink-0" />
|
||||
|
||||
@@ -21,6 +21,42 @@ const meta = {
|
||||
export default meta;
|
||||
type Story = StoryObj<typeof meta>;
|
||||
|
||||
/**
|
||||
* 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: () => (
|
||||
<div className="grid gap-4 sm:grid-cols-2">
|
||||
<Card>
|
||||
<CardHeader>
|
||||
<CardTitle>With a header</CardTitle>
|
||||
</CardHeader>
|
||||
<CardContent className="text-sm text-muted-foreground">
|
||||
The body drops its top inset because the header above already supplied
|
||||
one.
|
||||
</CardContent>
|
||||
</Card>
|
||||
<Card>
|
||||
<CardContent className="text-sm text-muted-foreground">
|
||||
No header, so the body keeps its own top inset — automatically, with
|
||||
nothing for the call site to remember.
|
||||
</CardContent>
|
||||
</Card>
|
||||
</div>
|
||||
),
|
||||
};
|
||||
|
||||
export const HostCard: Story = {
|
||||
render: () => (
|
||||
<Card className="max-w-sm">
|
||||
|
||||
Reference in New Issue
Block a user