From 46e8711c902871be6a0db60d6a06a48b4d551d1e Mon Sep 17 00:00:00 2001 From: BigSimmo <87357024+BigSimmo@users.noreply.github.com> Date: Fri, 7 Aug 2026 19:17:47 +0800 Subject: [PATCH] feat(design-system): give cn() tailwind-merge (#218) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `cn()` was a plain `classes.filter(Boolean).join(" ")`, so a later class could never override an earlier one — every size or colour override had to be worked around at the call site, and the winner was decided by Tailwind's stylesheet emission order rather than by intent. Five comments across four files documented that constraint. The substance is not the `cn()` body, which is a one-line change keeping its exact previous signature and falsy filtering. It is `src/lib/tailwind-merge.ts`: stock tailwind-merge classifies any unrecognised `text-` as a text COLOUR, and this repo's `@theme` defines scales it has never seen. `text-sm-minus`, `text-base-minus`, `text-lg-minus`, `text-2xl-minus`, `text-2xl-compact`, `text-3xl-minus` and `text-hero` were each measured being deleted when they met `text-[color:var(--text-muted)]` in the same call — the `eyebrowText` recipe is exactly that pair. `size-icon-*`, `tracking-*`, `leading-*`, `ease-*`, `animate-*` and the `pt-safe` family were not recognised at all, so they never merged. All of that is silent: no type error, no lint error. `--spacing-tap` is deliberately NOT declared. Tailwind emits `.min-h-tap` after every numeric `.min-h-*`, so the 48px tap token wins today at the 22 call sites that pair them; declaring it would hand the win to the later class and drop 18 production targets to 32/36/40/42px, which AGENTS.md forbids. The omission is documented in the config and pinned by a test. Every other family was measured across all 1 409 `cn()` call sites and introduces zero new class deletions. The 339 sites where twMerge drops `focus-visible:outline` beside `focus-visible:outline-2` are CSS-identical: compiling Tailwind shows both utilities emit the same two properties, with `--tw-outline-style` defaulting to `solid`. The five workaround comments now record that the constraint is lifted. The dodges themselves are kept — reverting `max-sm:hidden` or collapsing the three-branch chip changes which utilities render, and belongs in a change whose Chromium job is being read, not in a dependency swap. One of the five is only half lifted: tailwind-merge scores bare `border` and `border-t` as separate groups, so `document-search-results.tsx` still needs its `border-0`. Co-Authored-By: Claude Opus 5 --- docs/codebase-index.md | 1 + package-lock.json | 11 ++ package.json | 1 + .../document-search-results.tsx | 27 +-- .../master-search-header.tsx | 7 +- src/components/mode-home-template.tsx | 13 +- src/components/ui-primitives.tsx | 12 +- src/components/ui/page-header.tsx | 14 +- src/lib/tailwind-merge.ts | 103 +++++++++++ tests/tailwind-merge-config.test.ts | 164 ++++++++++++++++++ 10 files changed, 332 insertions(+), 21 deletions(-) create mode 100644 src/lib/tailwind-merge.ts create mode 100644 tests/tailwind-merge-config.test.ts diff --git a/docs/codebase-index.md b/docs/codebase-index.md index 257e099eb..32b9909d7 100644 --- a/docs/codebase-index.md +++ b/docs/codebase-index.md @@ -178,6 +178,7 @@ domain-extracted directory; imported as `@/lib/rag/rag*`). Other modules below r | `observability/` — `answer-slo.ts`, `cache-metrics.ts`, `spend-metrics.ts`, `error-tracking.ts`, `agent-monitoring.ts` | Deep-health SLO / cache-hit / answer-spend snapshots; privacy-safe Sentry error + DB-span scrubbers and metadata-only OpenAI agent monitoring (`docs/error-tracking.md`) | | `validation/` | `body.ts`, `query.ts`, `params.ts`, `http.ts`, `form-data.ts` | | `app-modes.ts`, `document-flow-routes.ts`, `local-project-identity.ts`, `local-server-utils.mjs` | Routing and project identity | +| `tailwind-merge.ts` | The `extendTailwindMerge` config behind `cn()` — declares this repo's custom `@theme` scales so twMerge does not misclassify them (`docs/design-system/TOKENS.md`) | --- diff --git a/package-lock.json b/package-lock.json index c7ffeb1dc..4dd493bb9 100644 --- a/package-lock.json +++ b/package-lock.json @@ -28,6 +28,7 @@ "react": "19.2.8", "react-dom": "19.2.8", "server-only": "^0.0.1", + "tailwind-merge": "^3.6.0", "zod": "^4.4.3" }, "devDependencies": { @@ -13073,6 +13074,16 @@ "dev": true, "license": "MIT" }, + "node_modules/tailwind-merge": { + "version": "3.6.0", + "resolved": "https://registry.npmjs.org/tailwind-merge/-/tailwind-merge-3.6.0.tgz", + "integrity": "sha512-uxL7qAVQriqRQPAyK3pj66VqskWqoZ37PW94jwOTwNfq/z9oyu1V+eqrZqtR2+fCiXdYOZe/Modt8GtvqNzu+w==", + "license": "MIT", + "funding": { + "type": "github", + "url": "https://github.com/sponsors/dcastil" + } + }, "node_modules/tailwindcss": { "version": "4.3.3", "resolved": "https://registry.npmjs.org/tailwindcss/-/tailwindcss-4.3.3.tgz", diff --git a/package.json b/package.json index 1f9aa567d..fa1757c5d 100644 --- a/package.json +++ b/package.json @@ -260,6 +260,7 @@ "react": "19.2.8", "react-dom": "19.2.8", "server-only": "^0.0.1", + "tailwind-merge": "^3.6.0", "zod": "^4.4.3" }, "overrides": { diff --git a/src/components/clinical-dashboard/document-search-results.tsx b/src/components/clinical-dashboard/document-search-results.tsx index 8fe17b6e2..cff345b94 100644 --- a/src/components/clinical-dashboard/document-search-results.tsx +++ b/src/components/clinical-dashboard/document-search-results.tsx @@ -364,13 +364,14 @@ function DocumentFilterPanel({ onOpenLibrary(); }} data-testid="document-filter-browse-library" - // `border-0 border-t`, not `border-t` alone. `cn` is a plain join, - // not tailwind-merge, so `floatingControl`'s own `border` (all four - // sides) survives an added `border-t` and the result is a fully - // bordered button whose colour is decided by Tailwind's emission - // order between two competing arbitrary utilities — the exact hazard - // the facet-chip branches in this file are written to avoid. Zeroing - // the box first leaves only the separating rule that was intended. + // `border-0 border-t`, not `border-t` alone. `cn` now runs through + // tailwind-merge (ledger #218), which lifts half of this: the + // competing arbitrary border COLOURS resolve last-wins instead of by + // Tailwind's emission order. The width half is not lifted — + // tailwind-merge scores bare `border` and `border-t` as different + // groups, so `floatingControl`'s all-sides `border` still survives an + // added `border-t` and the button would still be fully bordered. + // Zeroing the box first is still what leaves only the separating rule. className={cn( floatingControl, "min-h-tap justify-start gap-2 rounded-lg border-0 border-t border-[color:var(--border)] bg-transparent px-1 text-xs sm:min-h-10", @@ -631,10 +632,14 @@ function DocumentFilterPanel({ "inline-flex min-h-tap max-w-full items-center gap-1.5 rounded-md border px-2.5 text-2xs font-semibold shadow-[var(--shadow-inset)] transition motion-reduce:transition-none sm:min-h-9 sm:gap-1 sm:px-2 lg:min-h-8", "focus-visible:outline focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-[color:var(--focus)]", // Three mutually exclusive branches, not a base plus an - // override: `cn` is a plain join, so two competing - // `border-[color:…]` utilities would both reach the DOM - // and the winner would be decided by stylesheet order - // rather than by intent. + // override: `cn` was a plain join, so two competing + // `border-[color:…]` utilities both reached the DOM and + // the winner was decided by stylesheet order rather than + // by intent. That constraint is lifted (ledger #218) — + // `cn` merges, and a later border colour now wins + // deterministically. The branches are kept as they are + // because collapsing them changes which utilities render; + // that is a visual change, not a dependency swap. selected ? "border-[color:var(--clinical-accent)]/35 bg-[color:var(--clinical-accent-soft)] text-[color:var(--clinical-accent)]" : deadEnd diff --git a/src/components/clinical-dashboard/master-search-header.tsx b/src/components/clinical-dashboard/master-search-header.tsx index f1c09dd25..31c32d47a 100644 --- a/src/components/clinical-dashboard/master-search-header.tsx +++ b/src/components/clinical-dashboard/master-search-header.tsx @@ -2082,10 +2082,13 @@ export function MasterSearchHeader({ onKeyDown={handleModeTriggerKeyDown} className={cn( // Size utilities live in the per-variant branch, never the shared - // base: cn() is plain concat (no tailwind-merge), so keeping the + // base: cn() was plain concat (no tailwind-merge), so keeping the // default h-/w-/min-w- here too made the workflow overrides dead — // Tailwind v4 emits same-property utilities in canonical order and - // the base won at every breakpoint but lg:. + // the base won at every breakpoint but lg:. That constraint is + // lifted (ledger #218): cn() merges, so a base size would now lose + // to the branch that follows it. The split stays because one place + // per size is still the clearer shape, not because it is forced. "universal-header-mode-button inline-grid min-w-0 grid-cols-[auto_minmax(0,1fr)_auto] items-center gap-2 rounded-full border border-[color:var(--border)] bg-[color:var(--surface)] px-2.5 text-left transition hover:border-[color:var(--border-strong)] hover:bg-[color:var(--surface-subtle)] focus-visible:outline focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-[color:var(--focus)]", isWorkflowHeader ? "h-tap w-[min(11rem,calc(100vw-11rem))] sm:w-[12rem] sm:min-w-0 lg:w-[12.5rem]" diff --git a/src/components/mode-home-template.tsx b/src/components/mode-home-template.tsx index 55a515ef8..4f537fae6 100644 --- a/src/components/mode-home-template.tsx +++ b/src/components/mode-home-template.tsx @@ -103,10 +103,17 @@ export function ModeHomeHero({ * shell). That works for short empty homes, but centering a child taller than * the phone scrollport clips the top — unreachable at scrollTop 0. * - * Prefer this prop over className `justify-*` overrides: `cn()` concatenates - * and does not resolve Tailwind conflicts, so dual justify utilities are - * non-deterministic. Alignment classes are applied last and any stray + * Prefer this prop over className `justify-*` overrides: `cn()` used to + * concatenate without resolving Tailwind conflicts, so dual justify utilities + * were non-deterministic. Alignment classes are applied last and any stray * `justify-*` tokens in `className` are stripped. + * + * That constraint is lifted (ledger #218) — `cn()` now merges, and because the + * alignment class is applied last it would win on its own, prefixed variants + * (`sm:justify-*`) included. `withoutJustifyUtilities` is therefore belt-and- + * braces rather than load-bearing. It is kept: dropping it changes which + * utilities reach the DOM on every mode home, which wants its own visual proof, + * and the prop remains the supported way to set this alignment either way. */ export type ModeHomeMainAlign = "center" | "start" | "startOnPhone"; diff --git a/src/components/ui-primitives.tsx b/src/components/ui-primitives.tsx index aa6f58af8..1e6d03b97 100644 --- a/src/components/ui-primitives.tsx +++ b/src/components/ui-primitives.tsx @@ -10,6 +10,7 @@ import { validationStatusLabel, } from "@/lib/source-metadata"; import { classifySourceAuthority } from "@/lib/source-authority-registry"; +import { twMergeClinical } from "@/lib/tailwind-merge"; import type { ClinicalSourceMetadata } from "@/lib/types"; /** @@ -23,8 +24,17 @@ import type { ClinicalSourceMetadata } from "@/lib/types"; */ export type SourceMetadataInput = Partial | null; +/** + * Compose Tailwind classes, resolving conflicts last-wins. + * + * Falsy arguments are dropped exactly as before; what changed is that the result + * now goes through tailwind-merge, so a later class beats an earlier one instead + * of both being emitted and the generated stylesheet's order deciding. See + * `@/lib/tailwind-merge` for why the merge needs this repo's `@theme` scales + * declared to it, and what it silently deletes without them. + */ export function cn(...classes: Array) { - return classes.filter(Boolean).join(" "); + return twMergeClinical(classes.filter(Boolean).join(" ")); } export const transitionSurface = "transition-colors transition-shadow motion-reduce:transition-none"; diff --git a/src/components/ui/page-header.tsx b/src/components/ui/page-header.tsx index d954f7f4d..c4567d04e 100644 --- a/src/components/ui/page-header.tsx +++ b/src/components/ui/page-header.tsx @@ -146,11 +146,17 @@ export function PageHeader({ page header keeps one treatment. `max-sm:hidden`, not `hidden sm:grid`: `iconTilePremium` already - carries `grid`, and `cn()` in this repo is a plain join with no + carries `grid`, and `cn()` used to be a plain join with no tailwind-merge (ledger #218), so `hidden` + `sm:grid` + `grid` - would leave three display utilities to be resolved by stylesheet - order rather than by intent. One base display plus one max-width - override has nothing to race. */} + left three display utilities to be resolved by stylesheet order + rather than by intent. One base display plus one max-width + override had nothing to race. + + That constraint is lifted — `cn()` now merges, and `grid hidden + sm:grid` resolves to `hidden sm:grid`. This site keeps + `max-sm:hidden` anyway: swapping it back is a change to which + display utility renders, so it belongs in a change whose Chromium + job is being read, not in the dependency PR. */} {Icon ? (