From 71603479f73a3724eb54966a0d493da04aa04300 Mon Sep 17 00:00:00 2001 From: YaelAnaya Date: Tue, 8 Sep 2026 19:01:29 -0700 Subject: [PATCH 01/19] refactor: move tests out of src and trim the harness Tests live under tests/ (components, theme, gates, harness) and vitest runs two projects: unit (jsdom) and package (node, for the packed-consumer test to come). tests/setup.ts keeps only what a current test needs: the drained macrotask after cleanup, sonner's dismiss, and the pointer-capture and scrollIntoView stubs. The FormData, createObjectURL and localStorage workarounds came from VisionSet application code that no longer lives here. --- eslint.config.js | 11 +- package.json | 5 +- {src => tests}/components/combobox.test.tsx | 2 +- {src => tests}/components/components.test.tsx | 40 ++-- {src => tests}/components/sonner.test.tsx | 2 +- {src => tests}/gates/design.test.ts | 2 +- {src => tests}/gates/index.test.ts | 0 {src => tests}/gates/tokens.test.ts | 2 +- {src => tests}/harness.test.tsx | 2 +- tests/setup.ts | 48 +++++ {src => tests}/theme/imports.test.ts | 0 {src => tests}/theme/statusTone.test.ts | 2 +- {src => tests}/theme/tokens.test.ts | 7 +- tsconfig.build.json | 12 +- tsconfig.json | 18 +- vitest.config.ts | 87 ++++----- vitest.setup.ts | 171 ------------------ 17 files changed, 141 insertions(+), 270 deletions(-) rename {src => tests}/components/combobox.test.tsx (97%) rename {src => tests}/components/components.test.tsx (95%) rename {src => tests}/components/sonner.test.tsx (97%) rename {src => tests}/gates/design.test.ts (99%) rename {src => tests}/gates/index.test.ts (100%) rename {src => tests}/gates/tokens.test.ts (99%) rename {src => tests}/harness.test.tsx (94%) create mode 100644 tests/setup.ts rename {src => tests}/theme/imports.test.ts (100%) rename {src => tests}/theme/statusTone.test.ts (92%) rename {src => tests}/theme/tokens.test.ts (98%) delete mode 100644 vitest.setup.ts diff --git a/eslint.config.js b/eslint.config.js index 37d0adf..445131a 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -3,16 +3,13 @@ import reactHooks from "eslint-plugin-react-hooks"; import tseslint from "typescript-eslint"; export default tseslint.config( - // Flat-config ignores are relative to this file's directory, so the generated client must be - // named at its real path — "generated/" silently matches nothing now that it lives under src/. - { ignores: ["dist/", "src/generated/"] }, + { ignores: ["dist/", "examples/catalog/dist/"] }, js.configs.recommended, ...tseslint.configs.recommended, { - // The Rules of Hooks over the whole package rather than scoped to a React - // subdirectory: every module here is a component or is imported by one, so - // the scope is the package. - files: ["src/**/*.ts", "src/**/*.tsx"], + // The Rules of Hooks everywhere a component can be written: the package, + // its tests, and the catalog. + files: ["src/**/*.{ts,tsx}", "tests/**/*.{ts,tsx}", "examples/**/*.{ts,tsx}"], plugins: { "react-hooks": reactHooks }, rules: { "react-hooks/rules-of-hooks": "error", diff --git a/package.json b/package.json index 38783ec..7d3074a 100644 --- a/package.json +++ b/package.json @@ -27,9 +27,10 @@ ], "scripts": { "build": "tsc -p tsconfig.build.json", - "test": "vitest run", + "test": "vitest run --project unit", + "test:package": "vitest run --project package", "typecheck": "tsc -p tsconfig.json --noEmit", - "lint": "eslint src && pnpm run typecheck", + "lint": "eslint . && pnpm run typecheck", "format": "prettier --write .", "format:check": "prettier --check ." }, diff --git a/src/components/combobox.test.tsx b/tests/components/combobox.test.tsx similarity index 97% rename from src/components/combobox.test.tsx rename to tests/components/combobox.test.tsx index 70ab6e1..8379c64 100644 --- a/src/components/combobox.test.tsx +++ b/tests/components/combobox.test.tsx @@ -8,7 +8,7 @@ import { ComboboxInput, ComboboxItem, ComboboxList, -} from "./combobox"; +} from "../../src/components/combobox"; const FRUIT = ["apple", "banana", "cherry"]; diff --git a/src/components/components.test.tsx b/tests/components/components.test.tsx similarity index 95% rename from src/components/components.test.tsx rename to tests/components/components.test.tsx index 6e3cbcd..14df440 100644 --- a/src/components/components.test.tsx +++ b/tests/components/components.test.tsx @@ -18,25 +18,37 @@ import userEvent from "@testing-library/user-event"; import type { JSX } from "react"; import { describe, expect, it } from "vitest"; -import { Alert, AlertDescription, AlertTitle } from "./alert"; -import { Badge } from "./badge"; -import { Button } from "./button"; -import { Card, CardTitle } from "./card"; -import { Dialog, DialogContent, DialogDescription, DialogTitle } from "./dialog"; +import { Alert, AlertDescription, AlertTitle } from "../../src/components/alert"; +import { Badge } from "../../src/components/badge"; +import { Button } from "../../src/components/button"; +import { Card, CardTitle } from "../../src/components/card"; +import { Dialog, DialogContent, DialogDescription, DialogTitle } from "../../src/components/dialog"; import { DropdownMenu, DropdownMenuContent, DropdownMenuItem, DropdownMenuTrigger, -} from "./dropdown-menu"; -import { Progress } from "./progress"; -import { FieldError } from "./field"; -import { Input } from "./input"; -import { Label } from "./label"; -import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from "./select"; -import { Sheet, SheetContent, SheetDescription, SheetHeader, SheetTitle } from "./sheet"; -import { Table, TableBody, TableHead, TableHeader, TableRow } from "./table"; -import { Tabs, TabsContent, TabsList, TabsTrigger } from "./tabs"; +} from "../../src/components/dropdown-menu"; +import { Progress } from "../../src/components/progress"; +import { FieldError } from "../../src/components/field"; +import { Input } from "../../src/components/input"; +import { Label } from "../../src/components/label"; +import { + Select, + SelectContent, + SelectItem, + SelectTrigger, + SelectValue, +} from "../../src/components/select"; +import { + Sheet, + SheetContent, + SheetDescription, + SheetHeader, + SheetTitle, +} from "../../src/components/sheet"; +import { Table, TableBody, TableHead, TableHeader, TableRow } from "../../src/components/table"; +import { Tabs, TabsContent, TabsList, TabsTrigger } from "../../src/components/tabs"; describe("Button", () => { it("keeps an explicit type", () => { diff --git a/src/components/sonner.test.tsx b/tests/components/sonner.test.tsx similarity index 97% rename from src/components/sonner.test.tsx rename to tests/components/sonner.test.tsx index 069795a..6b7716e 100644 --- a/src/components/sonner.test.tsx +++ b/tests/components/sonner.test.tsx @@ -1,7 +1,7 @@ import { render, waitFor } from "@testing-library/react"; import { afterEach, describe, expect, it } from "vitest"; import { toast } from "sonner"; -import { Toaster } from "./sonner"; +import { Toaster } from "../../src/components/sonner"; afterEach(() => document.documentElement.classList.remove("dark")); diff --git a/src/gates/design.test.ts b/tests/gates/design.test.ts similarity index 99% rename from src/gates/design.test.ts rename to tests/gates/design.test.ts index 7ad79c7..2639e17 100644 --- a/src/gates/design.test.ts +++ b/tests/gates/design.test.ts @@ -5,7 +5,7 @@ import { spawnSync } from "node:child_process"; import { fileURLToPath } from "node:url"; import { expect, test } from "vitest"; -import { competingStatusPaletteIn, statusPaletteIn } from "./index.js"; +import { competingStatusPaletteIn, statusPaletteIn } from "../../src/gates/index.js"; const REPO = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "..", ".."); const read = (rel: string) => readFileSync(path.join(REPO, rel), "utf8"); diff --git a/src/gates/index.test.ts b/tests/gates/index.test.ts similarity index 100% rename from src/gates/index.test.ts rename to tests/gates/index.test.ts diff --git a/src/gates/tokens.test.ts b/tests/gates/tokens.test.ts similarity index 99% rename from src/gates/tokens.test.ts rename to tests/gates/tokens.test.ts index 6523cdf..313e08b 100644 --- a/src/gates/tokens.test.ts +++ b/tests/gates/tokens.test.ts @@ -12,7 +12,7 @@ import { spawnSync } from "node:child_process"; import { fileURLToPath } from "node:url"; import { expect, test } from "vitest"; -import { brandUsagesIn, colouredClassesIn } from "./index.js"; +import { brandUsagesIn, colouredClassesIn } from "../../src/gates/index.js"; const REPO = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "..", ".."); diff --git a/src/harness.test.tsx b/tests/harness.test.tsx similarity index 94% rename from src/harness.test.tsx rename to tests/harness.test.tsx index 3b84b1e..31d1a41 100644 --- a/src/harness.test.tsx +++ b/tests/harness.test.tsx @@ -17,7 +17,7 @@ import { render } from "@testing-library/react"; import { expect, it, vi } from "vitest"; -import { Dialog, DialogContent, DialogTitle } from "./components/dialog"; +import { Dialog, DialogContent, DialogTitle } from "../src/components/dialog"; const unmountFocusReturned = vi.fn(); diff --git a/tests/setup.ts b/tests/setup.ts new file mode 100644 index 0000000..0d0f1c9 --- /dev/null +++ b/tests/setup.ts @@ -0,0 +1,48 @@ +/** + * The jsdom harness every unit test runs under. Three things, each held by a + * test or a component in this repository; a workaround nothing here needs does + * not belong in this file. + */ + +import { cleanup } from "@testing-library/react"; +import { toast } from "sonner"; +import { afterEach } from "vitest"; + +/** + * Unmount between tests, and let the unmount finish. + * + * `@testing-library/react` registers `cleanup` itself only when `afterEach` is + * a global; this suite runs with `globals: false`, so it is registered here. + * + * The drained macrotask is for work an unmount schedules rather than does. + * Radix's focus scope returns focus from a `setTimeout(…, 0)` registered by its + * unmount cleanup; after a file's last test that timer races vitest's + * environment teardown and, when teardown wins, dispatches an event into a + * closed document — an uncaught `TypeError` under a green test count. A timer + * scheduled here, after Radix's and for the same duration, fires after it. + * `tests/harness.test.tsx` holds the property. + */ +afterEach(async () => { + cleanup(); + await new Promise((resolve) => setTimeout(resolve, 0)); +}); + +/** + * Sonner's toast store is module-global and outlives `cleanup()`, and a freshly + * mounted `` replays every toast still in it. `dismiss()` with no id + * marks every active toast dismissed synchronously. `tests/components/sonner.test.tsx` + * fires a toast in each of its tests and would otherwise see the previous one. + */ +afterEach(() => toast.dismiss()); + +/** + * Two DOM methods jsdom does not implement, which Radix's `Select` and + * `DropdownMenu` call while opening. Without them the open throws inside Radix + * and the test reads as "the option is not there". + */ +if (typeof Element !== "undefined") { + Element.prototype.hasPointerCapture ??= () => false; + Element.prototype.setPointerCapture ??= () => undefined; + Element.prototype.releasePointerCapture ??= () => undefined; + Element.prototype.scrollIntoView ??= () => undefined; +} diff --git a/src/theme/imports.test.ts b/tests/theme/imports.test.ts similarity index 100% rename from src/theme/imports.test.ts rename to tests/theme/imports.test.ts diff --git a/src/theme/statusTone.test.ts b/tests/theme/statusTone.test.ts similarity index 92% rename from src/theme/statusTone.test.ts rename to tests/theme/statusTone.test.ts index a7e311f..d941dc3 100644 --- a/src/theme/statusTone.test.ts +++ b/tests/theme/statusTone.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from "vitest"; -import { STATUS_INK, TONE_BORDER, TONE_FILL } from "./statusTone"; +import { STATUS_INK, TONE_BORDER, TONE_FILL } from "../../src/theme/statusTone"; describe("status tones", () => { it("names one Tailwind family per status and never a retired token", () => { diff --git a/src/theme/tokens.test.ts b/tests/theme/tokens.test.ts similarity index 98% rename from src/theme/tokens.test.ts rename to tests/theme/tokens.test.ts index fdbd2b2..08bb437 100644 --- a/src/theme/tokens.test.ts +++ b/tests/theme/tokens.test.ts @@ -19,9 +19,12 @@ import { fileURLToPath } from "node:url"; import { describe, expect, it } from "vitest"; -import { cssVar, DARK_THEME, LIGHT_THEME, THEME } from "./tokens"; +import { cssVar, DARK_THEME, LIGHT_THEME, THEME } from "../../src/theme/tokens"; -const STYLESHEET = readFileSync(fileURLToPath(new URL("./styles.css", import.meta.url)), "utf8"); +const STYLESHEET = readFileSync( + fileURLToPath(new URL("../../src/theme/styles.css", import.meta.url)), + "utf8", +); /** * Whitespace and quote style are presentation: a value that wraps, and a font diff --git a/tsconfig.build.json b/tsconfig.build.json index c89fcc2..525375c 100644 --- a/tsconfig.build.json +++ b/tsconfig.build.json @@ -1,7 +1,11 @@ { - // What ships. Test files are compiled by `typecheck` and by vitest, but they - // must not land in `dist/`: a consumer resolving `@robomous/ui-core` should - // never be one `import` away from a testing-library dependency. + // What ships: `src` only, declarations included. Tests live outside `src`, + // so nothing here has to be excluded. "extends": "./tsconfig.json", - "exclude": ["src/**/*.test.ts", "src/**/*.test.tsx", "src/testing"] + "compilerOptions": { + "declaration": true, + "outDir": "dist", + "rootDir": "src" + }, + "include": ["src"] } diff --git a/tsconfig.json b/tsconfig.json index 5bb6f8d..77fe9ce 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -1,4 +1,6 @@ { + // Typechecking: the shipped source and the tests together. What ships is + // compiled by tsconfig.build.json, which narrows this to `src`. "compilerOptions": { "target": "ES2022", "module": "ESNext", @@ -10,20 +12,10 @@ "noFallthroughCasesInSwitch": true, "isolatedModules": true, "skipLibCheck": true, - // TypeScript 6 stopped pulling every installed @types package into the - // ambient scope; a package that is needed must now be named. `node` is here - // for `src/gates/index.ts`, which reads the stylesheet off disk, and for the - // test files that scan the tree — which is the same reason the package - // declares @types/node at all. + // TypeScript 6 pulls in only the @types packages that are named. `node` is + // for the tests that read files and spawn processes. "types": ["node"], - "declaration": true, - "outDir": "dist", - // TypeScript 6 stopped inferring the common source directory when that - // inference shapes the output layout (error TS5011). `src` is what 5.x - // inferred; stating it changes nothing about `dist/` and is what the - // compiler now requires. - "rootDir": "src", "jsx": "react-jsx" }, - "include": ["src"] + "include": ["src", "tests"] } diff --git a/vitest.config.ts b/vitest.config.ts index f6fb7fa..28a5f3f 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -1,12 +1,12 @@ /** - * The component-test harness. + * Two projects, one command each. * - * These are ordinary DOM components whose behaviour *is* markup and roles — - * the `className` merge that makes an override real, the `asChild` that keeps a - * link a link, the value a `Progress` announces — so jsdom is the environment - * that can see any of it. The gates run here too, under - * `// @vitest-environment node`, because they read files rather than render - * anything. One suite, one command. + * `unit` renders the components into jsdom and asserts the behaviour a screen + * would silently lose. `package` builds and packs this repository, installs the + * tarball into a throwaway consumer and compiles it with a real Tailwind — the + * only check here that sees the package the way a consumer does. It is slow and + * touches the network, so `pnpm test` runs `unit` alone and `pnpm test:package` + * runs the other. */ import { availableParallelism } from "node:os"; @@ -15,57 +15,42 @@ import react from "@vitejs/plugin-react"; import { defineConfig } from "vitest/config"; /** - * How many test files run at once, and why it is a quarter of the cores rather - * than vitest's own default. - * - * Vitest's `forks` pool defaults to roughly one worker per logical core, and a - * worker here is not cheap: each one stands up a whole jsdom and a React root - * before it asserts anything. At one per core the suite oversubscribes the - * machine it is running on, and the tests that then miss their deadline are not - * the slow ones — they are whichever happened to be holding a core when the - * machine ran out, which is a failure that moves between runs and reads as - * flakiness rather than as load. - * - * Derived rather than fixed, because this is the command a contributor runs on - * whatever they have: a number chosen for a twenty-core desktop would throttle a - * four-core laptop. The divisor is four rather than two because the count that - * held was half the *physical* cores on a hyperthreaded box, and - * `availableParallelism` reports logical ones. The floor of two keeps a two-core - * CI runner where it already is — GitHub's runners derive below the floor, so the - * cap changes nothing there. - * - * The cap costs close to nothing, which is worth stating because it sounds like it - * should not be. A suite's wall time is bounded by its slowest *file*, not by how - * much CPU it can occupy, so workers past the point where each one carries less - * work than the critical path spend most of their lives waiting for it. What the - * cap removes is the contention that made every *other* file miss a deadline. + * A quarter of the logical cores, floor two. Each worker stands up a jsdom and + * a React root; one per core oversubscribes the machine and the tests that then + * miss their deadline are whichever happened to hold a core, which reads as + * flakiness rather than as load. A suite's wall time is bounded by its slowest + * file, so the cap costs close to nothing. */ const MAX_WORKERS = Math.max(2, Math.floor(availableParallelism() / 4)); export default defineConfig({ - plugins: [react()], test: { - environment: "jsdom", - // Every suite, including the gates' own tests, lives under src/. - include: ["src/**/*.test.{ts,tsx}"], - // Explicit imports from "vitest" in every test file. Globals would make a + // Explicit imports from "vitest" in every test file: globals would make a // test file's dependencies invisible. globals: false, - setupFiles: ["./vitest.setup.ts"], maxWorkers: MAX_WORKERS, - /** - * Headroom, not a loosening. - * - * Vitest's 5000ms default is a bound for a pure function. A test here mounts - * a component into jsdom and drives it through `userEvent`, which waits on - * real timers between synthetic events, and the gates shell out to - * `git ls-files` and read every tracked source. Neither is slow, but neither - * is bounded the way a pure function is, and a deadline tuned for the fast - * case turns a loaded machine into a red suite. - * - * It works only alongside the cap above. On its own a bigger timeout would - * just move the load at which contention starts reading as failure. - */ - testTimeout: 15_000, + projects: [ + { + plugins: [react()], + test: { + name: "unit", + environment: "jsdom", + include: ["tests/**/*.test.{ts,tsx}"], + exclude: ["tests/package/**"], + setupFiles: ["./tests/setup.ts"], + // Headroom for userEvent, which waits on real timers between events. + testTimeout: 15_000, + }, + }, + { + test: { + name: "package", + environment: "node", + include: ["tests/package/**/*.test.ts"], + testTimeout: 600_000, + hookTimeout: 600_000, + }, + }, + ], }, }); diff --git a/vitest.setup.ts b/vitest.setup.ts deleted file mode 100644 index 9d4f516..0000000 --- a/vitest.setup.ts +++ /dev/null @@ -1,171 +0,0 @@ -/** - * Unmount between tests, and let the unmount finish. - * - * `@testing-library/react` registers `cleanup` itself when a runner exposes - * `afterEach` as a global. This suite runs with `globals: false` — imports in a - * test file should say what the file uses — so the hook has to be registered by - * hand. Without it every render accumulates in one `document.body` and a query - * like `getByRole("alert")` starts failing with "found multiple elements" in - * whichever test happens to run second. That is a harness bug that reads exactly - * like a component bug, which is why it is worth a file and a comment. - * - * The drained macrotask is for work an unmount schedules rather than does. - * Radix's focus scope returns focus from a `setTimeout(…, 0)` registered by its - * unmount cleanup, so after `cleanup()` that timer is still pending. It fires - * into the next test's first millisecond, and after a file's last test it races - * the environment teardown: vitest tears jsdom down inside the worker's `stop` - * message handler, libuv serves I/O callbacks before the next timers phase, and a - * starved worker therefore wakes to the teardown first. The timer then builds a - * `CustomEvent` from Node's restored global and dispatches it on a jsdom element, - * which jsdom rejects — an uncaught `TypeError` that fails the run with every - * test green. A timer scheduled here, after the radix one and for the same - * duration, fires after it, so awaiting it is the guarantee that the document the - * unmount targets is still there. `harness.test.tsx` asserts the property. - */ - -import { cleanup } from "@testing-library/react"; -import { toast } from "sonner"; -import { afterEach } from "vitest"; - -afterEach(async () => { - cleanup(); - await new Promise((resolve) => setTimeout(resolve, 0)); -}); - -/** - * Sonner's toast store is module-global and outlives `cleanup()` — unmounting a - * `` removes its DOM, not the store's memory of what was announced. - * Since sonner 2.0.8 a freshly mounted `` replays every toast still - * in that store, so a toast fired in one test reappears beside the next test's - * own — `getByText` on a toast message then fails with "found multiple - * elements", in whichever test happens to announce the same thing second. - * `dismiss()` with no id marks every active toast dismissed synchronously, and - * the replay path skips dismissed ids. - */ -afterEach(() => toast.dismiss()); - -/** - * Two DOM methods jsdom does not implement, which Radix's `Select` and - * `DropdownMenu` call while opening. - * - * `hasPointerCapture` is part of the Pointer Events API — jsdom implements the - * events and not the capture model — and `scrollIntoView` is simply absent. - * Neither is a behaviour worth simulating: the first answers "is this pointer - * captured", which in a test is always no, and the second scrolls a viewport that - * does not exist. - * - * Without them, opening a `Select` throws inside Radix and the test reads as "the - * option is not there" — a component failure for a harness gap, which is the most - * misleading shape a test failure has. - */ -if (typeof Element !== "undefined") { - Element.prototype.hasPointerCapture ??= () => false; - Element.prototype.setPointerCapture ??= () => undefined; - Element.prototype.releasePointerCapture ??= () => undefined; - Element.prototype.scrollIntoView ??= () => undefined; -} - -/** - * Give `FormData` back to the realm that owns `fetch`. - * - * vitest's jsdom environment replaces `globalThis.FormData` with jsdom's class - * while leaving `fetch`, `Request` and `Response` as Node's (undici). The two do - * not recognise each other: `new Request(url, { body: })` - * silently **stringifies** it, so a multipart upload arrives as - * `text/plain: "[object FormData]"`. - * - * That is a realm mismatch and not a product bug — in a browser both come from the - * same place — but it makes an upload untestable, and worse, it makes a *correct* - * upload look exactly like the `[object File]` bug a missing `bodySerializer` - * produces. So the realms are reconciled here rather than worked around in each - * test. - * - * undici's class is not importable (undici is not a dependency), so it is taken - * from an instance: parsing a form-encoded `Response` produces one, and its - * constructor is the class `Request` will accept. - */ -const nodeFormData = ( - await new Response("k=v", { - headers: { "content-type": "application/x-www-form-urlencoded" }, - }).formData() -).constructor as typeof FormData; - -globalThis.FormData = nodeFormData; - -/** - * `URL.createObjectURL`, which jsdom does not implement at all. - * - * There is no blob URL scheme in jsdom and no object-URL registry behind it, so - * the call is simply absent — and because `AssetThumbnail` makes it inside an - * async effect, the `TypeError` lands *after* the test that caused it has passed. - * That is the worst shape a failure has: vitest reports "83 passed, 1 error" and - * names a test that was already green. It reproduced only in CI at first, because - * whether the effect resolves before the environment is torn down is a race. - * - * The stand-in is a counter rather than a no-op, so a test can still assert that a - * URL was handed out and that it was revoked — the leak this component exists to - * avoid is exactly the kind a silent no-op would hide. - */ -let objectUrls = 0; -const revoked = new Set(); - -// Assigned on a `typeof` check rather than with `??=`: a jsdom release that -// declares the property and leaves it `undefined` would satisfy neither `??=` nor -// a call, and the difference is invisible until CI. -if (typeof URL.createObjectURL !== "function") { - URL.createObjectURL = () => `blob:ui-core/${++objectUrls}`; -} -if (typeof URL.revokeObjectURL !== "function") { - URL.revokeObjectURL = (url: string) => { - revoked.add(url); - }; -} - -/** - * `localStorage`, which Node 26 declares on the global and leaves `undefined`. - * - * The fifth gap of this species in this file, and the first that comes from the - * runtime rather than from jsdom. Node declares the property while - * `--localstorage-file` is absent, so vitest's jsdom environment finds the key - * already taken and never installs jsdom's own — the property reads as - * `undefined` and `localStorage.getItem(...)` is a `TypeError` on a property - * read, not a storage that accepts writes and forgets them. - * - * `sessionStorage` is declared by Node too, and vitest skips it for the same - * reason — so that global is *Node's* `Storage`, not jsdom's. It is unaffected - * only because Node's `sessionStorage` works with no flag while its - * `localStorage` is `undefined` without `--localstorage-file`. That is why - * `data/session.ts` and its tests pass under both majors while - * `data/railState.ts` and `data/prefs.ts` do not. Any explanation predicting - * both would break is wrong. - * - * The product code needs nothing: `storage()` in both modules probes with a write - * and answers `null` from its `catch`, so the caller already gets the default — - * the behaviour that guard exists for, arriving through a cause nobody - * anticipated. What has no `localStorage` is the eight test *bodies*, which reach - * the property directly, as a browser lets them. - * - * Assigned on a `typeof` check rather than with `??=`: a property that is - * declared and `undefined` satisfies neither `??=` nor a call, which is exactly - * this case — the same point the `URL.createObjectURL` note above makes about a - * jsdom release. - * - * The stand-in is jsdom's real `Storage` rather than a hand-rolled `Map`, because - * `railState.test.ts` asserts the availability probe **leaves nothing behind**: - * a stub that swallowed writes would turn that test green while proving nothing. - * A `url` is mandatory — jsdom's default `about:blank` is an opaque origin and - * throws `SecurityError` on the first access. Setup runs per test file, so each - * worker gets its own empty store and nothing is shared across them, so long as - * files stay isolated, which is vitest's default. Passing `--localstorage-file` - * is not the fix, tempting as it looks for the `ExperimentalWarning` this block - * leaves behind: it makes the `typeof` check above pass, so this block is - * skipped, and the suite runs against Node's one process-wide on-disk store — - * `railState.test.ts`'s `afterEach` `clear()` would wipe a concurrently running - * file's data. - */ -if (typeof globalThis.localStorage?.setItem !== "function") { - const { JSDOM } = await import("jsdom"); - globalThis.localStorage = new JSDOM("", { - url: "http://localhost", - }).window.localStorage; -} From aaf5b21dbdb834d7544a843b874a38fd6aca3734 Mon Sep 17 00:00:00 2001 From: YaelAnaya Date: Tue, 8 Sep 2026 19:04:38 -0700 Subject: [PATCH 02/19] feat(theme): semantic status and overlay tokens; close the colour namespace; remove gates success, warning and info are roles in styles.css with light and dark values, exposed as bg-/text-/border-success and friends; overlay names the scrim behind a Dialog or Sheet. @theme resets --color-* so Tailwind's default palette is not part of the vocabulary, and --color-brand is no longer exposed: brand stays a CSS variable for identity only. chart-* tokens had no consumer and are gone; sidebar-* stays because two product shells use it. With the palette closed, statusPaletteIn, competingStatusPaletteIn and brandUsagesIn have nothing left to catch, and colouredClassesIn is held by an ESLint no-restricted-syntax rule instead. src/gates, the ./gates export and statusTone.ts (STATUS_INK, TONE_FILL, TONE_BORDER, StatusTone) are removed; docs/MIGRATION-0.3.md will carry the consumer migration. --- eslint.config.js | 21 ++- package.json | 6 +- src/components/badge.tsx | 8 +- src/components/dialog.tsx | 2 +- src/components/sheet.tsx | 2 +- src/gates/index.ts | 165 ------------------- src/index.ts | 15 +- src/theme/statusTone.ts | 52 ------ src/theme/styles.css | 188 +++++++++++----------- src/theme/tokens.ts | 43 +++-- tests/components/components.test.tsx | 5 +- tests/gates/design.test.ts | 229 --------------------------- tests/gates/index.test.ts | 11 -- tests/gates/tokens.test.ts | 215 ------------------------- tests/theme/statusTone.test.ts | 24 --- tests/theme/tokens.test.ts | 223 ++++++++------------------ 16 files changed, 209 insertions(+), 1000 deletions(-) delete mode 100644 src/gates/index.ts delete mode 100644 src/theme/statusTone.ts delete mode 100644 tests/gates/design.test.ts delete mode 100644 tests/gates/index.test.ts delete mode 100644 tests/gates/tokens.test.ts delete mode 100644 tests/theme/statusTone.test.ts diff --git a/eslint.config.js b/eslint.config.js index 445131a..8cec579 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -2,18 +2,35 @@ import js from "@eslint/js"; import reactHooks from "eslint-plugin-react-hooks"; import tseslint from "typescript-eslint"; +/** + * A Tailwind arbitrary value whose content is a colour: `bg-[#fff]`, + * `text-[rgb(0 0 0)]`, `ring-[var(--x)]`. The colour namespace is closed in + * `src/theme/styles.css`, so a named palette utility already produces nothing; + * this is the one road left for a literal to enter a class string, and it is + * held by the linter rather than by a scanner of our own. A consumer that wants + * the same rule copies these two selectors. + */ +const COLOUR_IN_CLASS = String.raw`-\[\s*(?:#|rgba?\(|hsla?\(|oklch\(|var\(--)`; +const COLOUR_MESSAGE = + "Colour belongs to the token contract: add a role to src/theme/styles.css and use its " + + "utility (bg-primary, text-warning), never a literal inside a class."; + export default tseslint.config( { ignores: ["dist/", "examples/catalog/dist/"] }, js.configs.recommended, ...tseslint.configs.recommended, { - // The Rules of Hooks everywhere a component can be written: the package, - // its tests, and the catalog. + // Everywhere a component can be written: the package, its tests, the catalog. files: ["src/**/*.{ts,tsx}", "tests/**/*.{ts,tsx}", "examples/**/*.{ts,tsx}"], plugins: { "react-hooks": reactHooks }, rules: { "react-hooks/rules-of-hooks": "error", "react-hooks/exhaustive-deps": "error", + "no-restricted-syntax": [ + "error", + { selector: `Literal[value=/${COLOUR_IN_CLASS}/]`, message: COLOUR_MESSAGE }, + { selector: `TemplateElement[value.raw=/${COLOUR_IN_CLASS}/]`, message: COLOUR_MESSAGE }, + ], }, }, ); diff --git a/package.json b/package.json index 7d3074a..bd933ae 100644 --- a/package.json +++ b/package.json @@ -15,11 +15,7 @@ "types": "./dist/index.d.ts", "import": "./dist/index.js" }, - "./styles.css": "./src/theme/styles.css", - "./gates": { - "types": "./dist/gates/index.d.ts", - "import": "./dist/gates/index.js" - } + "./styles.css": "./src/theme/styles.css" }, "files": [ "dist", diff --git a/src/components/badge.tsx b/src/components/badge.tsx index a7ed44a..4541dd9 100644 --- a/src/components/badge.tsx +++ b/src/components/badge.tsx @@ -13,11 +13,13 @@ const badgeVariants = cva( secondary: "bg-secondary text-secondary-foreground [a]:hover:bg-secondary/80", destructive: "bg-destructive/10 text-destructive focus-visible:ring-destructive/20 dark:bg-destructive/20 dark:focus-visible:ring-destructive/40 [a]:hover:bg-destructive/20", + // The status recipe: a soft surface and readable ink on one role. The + // role itself flips between themes, so no `dark:` restatement is needed. success: - "bg-emerald-500/10 text-emerald-700 focus-visible:ring-emerald-500/20 dark:bg-emerald-400/10 dark:text-emerald-400 dark:focus-visible:ring-emerald-400/40 [a]:hover:bg-emerald-500/20 dark:[a]:hover:bg-emerald-400/20", + "bg-success/10 text-success focus-visible:ring-success/20 dark:focus-visible:ring-success/40 [a]:hover:bg-success/20", warning: - "bg-amber-500/10 text-amber-700 focus-visible:ring-amber-500/20 dark:bg-amber-400/10 dark:text-amber-400 dark:focus-visible:ring-amber-400/40 [a]:hover:bg-amber-500/20 dark:[a]:hover:bg-amber-400/20", - info: "bg-sky-500/10 text-sky-700 focus-visible:ring-sky-500/20 dark:bg-sky-400/10 dark:text-sky-400 dark:focus-visible:ring-sky-400/40 [a]:hover:bg-sky-500/20 dark:[a]:hover:bg-sky-400/20", + "bg-warning/10 text-warning focus-visible:ring-warning/20 dark:focus-visible:ring-warning/40 [a]:hover:bg-warning/20", + info: "bg-info/10 text-info focus-visible:ring-info/20 dark:focus-visible:ring-info/40 [a]:hover:bg-info/20", quiet: "bg-muted text-muted-foreground [a]:hover:bg-muted/80", outline: "border-border text-foreground [a]:hover:bg-muted [a]:hover:text-muted-foreground", ghost: "hover:bg-muted hover:text-muted-foreground dark:hover:bg-muted/50", diff --git a/src/components/dialog.tsx b/src/components/dialog.tsx index 79776c5..a7a6783 100644 --- a/src/components/dialog.tsx +++ b/src/components/dialog.tsx @@ -29,7 +29,7 @@ function DialogOverlay({ ({ line, at: index + 1 })) - .filter(({ line }) => !COMMENT.test(line) && STATUS_PALETTE.test(line)) - .map(({ line, at }) => `${file}:${at}: ${line.trim()}`); -} - -/** - * A colour family that competes with the status palette for the same job — - * "a warning", "a success" — and so could stand in for it undetected. Unlike - * the palette itself these have no allowed home anywhere in a consumer. - */ -const COMPETING_PALETTE = /\b(?:bg|text|border)-(?:green|lime|teal|yellow|orange|blue|cyan|red)-\d/; - -/** Every `file:line` in `text` reaching for a colour family that competes with the status palette. */ -export function competingStatusPaletteIn(file: string, text: string): string[] { - return text - .split("\n") - .map((line, index) => ({ line, at: index + 1 })) - .filter(({ line }) => !COMMENT.test(line) && COMPETING_PALETTE.test(line)) - .map(({ line, at }) => `${file}:${at}: ${line.trim()}`); -} - -// ---- colour discipline (from tests/scripts/design_tokens.test.mjs) ---- - -/** - * A Tailwind arbitrary value whose content is a colour. - * - * Assembled from fragments so this file does not match itself. The `-` before - * the bracket is what makes it a *utility* rather than an array index or a - * TypeScript tuple type. - */ -const HEX = ["#", "[0-9a-fA-F]{3,8}"].join(""); -const LITERAL = String.raw`(?:${HEX}|rgba?\(|hsla?\(|oklch\()`; -// A raw colour right inside the bracket… -const ARBITRARY_COLOUR = new RegExp(String.raw`-\[\s*(?:${LITERAL}|var\(\s*--)`); -// The one colour-mix the preset writes mixes tokens only: -// `color-mix(in_oklch,var(--secondary),var(--foreground)_5%)`. Any other -// argument shape — a literal, a named colour, a bare number — is a colour. -const TOKEN_MIX = String.raw`color-mix\(in_[a-z0-9-]+(?:,var\(--[a-z0-9-]+\)(?:_\d+(?:\.\d+)?%)?)+\)`; -const BRACKET_MIX = new RegExp(String.raw`-\[\s*color-mix\([^\]]*\]`); -const ALLOWED_MIX = new RegExp(String.raw`-\[\s*${TOKEN_MIX}\]`); - -/** Every `file:line` in `text` that puts a colour inside a Tailwind class. */ -export function colouredClassesIn(file: string, text: string): string[] { - return text - .split("\n") - .map((line, index) => ({ line, at: index + 1 })) - .filter( - ({ line }) => - !COMMENT.test(line) && - (ARBITRARY_COLOUR.test(line) || (BRACKET_MIX.test(line) && !ALLOWED_MIX.test(line))), - ) - .map(({ line, at }) => `${file}:${at}: ${line.trim()}`); -} - -/** - * `DESIGN.md` "Where the brand is": the brand is identity, not a functional-UI - * colour — the wordmark and the styleguide swatch that shows it off, nothing - * a person acts on. This is not a headcount: the gate does not exist to hold - * a count of sites, it exists so brand can never migrate onto a control (a - * button, a progress fill, anything with a function) instead of staying the - * one place it is allowed to just be seen. A pure function over one file's - * text, so the gate is provable with fabricated input, and `COMMENT` keeps - * the styles.css line that *states* the rule from counting as a usage of it. - */ -const BRAND_UTILITY = /\b(?:bg|text|border|ring|fill|stroke)-brand\b/; - -/** Every line in `text` that paints with the brand colour. */ -export function brandUsagesIn( - file: string, - text: string, -): { file: string; at: number; text: string }[] { - return text - .split("\n") - .map((line, index) => ({ line, at: index + 1 })) - .filter(({ line }) => !COMMENT.test(line) && BRAND_UTILITY.test(line)) - .map(({ line, at }) => ({ file, at, text: line.trim() })); -} - -// ---- stylesheet parsing (from src/theme/tokens.test.ts, translated TS→JS) ---- - -/** Whitespace is presentation; a value that wraps is the same value — used internally by `rawDeclarations`. */ -function normalize(value: string): string { - return value.replace(/\s+/g, " ").trim(); -} - -/** - * The `{ … }` body belonging to the block whose header (e.g. `:root {` or - * `.dark {`) appears first in the file, matched by brace depth rather than by - * the next `\n}` so a nested `calc(...)` or `color-mix(...)` cannot fool it. - */ -export function blockBody(css: string, header: string): string { - const headerAt = css.indexOf(header); - if (headerAt === -1) throw new Error(`stylesheet has no ${JSON.stringify(header)} block`); - const braceAt = css.indexOf("{", headerAt); - let depth = 1; - let i = braceAt + 1; - while (depth > 0 && i < css.length) { - if (css[i] === "{") depth++; - else if (css[i] === "}") depth--; - i++; - } - return css.slice(braceAt + 1, i - 1); -} - -/** Every `--name: value;` declaration in a block, keyed WITH the leading `--`. */ -export function rawDeclarations(block: string): Map { - const map = new Map(); - for (const match of block.matchAll(/(--[\w-]+):\s*([^;]+);/g)) { - map.set(match[1], normalize(match[2])); - } - return map; -} - -/** The same, keyed WITHOUT the leading `--` — the shape `LIGHT_THEME` uses. */ -export function declarations(block: string): Map { - const map = new Map(); - for (const [name, value] of rawDeclarations(block)) { - map.set(name.slice(2), value); - } - return map; -} - -// ---- foundation facts ---- - -/** - * The foundation token names, read off the shipped stylesheet's `:root` so - * they can never drift from what actually runs. `radius` is a geometry - * setting, not a colour token, and is excluded — it matches LIGHT_THEME's keys. - */ -export function foundationTokenNames(): string[] { - const css = readFileSync(path.join(PKG, "src/theme/styles.css"), "utf8"); - return [...declarations(blockBody(css, ":root {")).keys()].filter((n) => n !== "radius"); -} diff --git a/src/index.ts b/src/index.ts index b27abac..bc3117b 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,8 +1,8 @@ /** * `@robomous/ui-core` — the Robomous design system. * - * Twenty-one React components this package owns outright, the design tokens - * they resolve through, and the status-tone vocabulary. A consumer imports + * Twenty-one React components this package owns outright, over Radix and Base + * UI behaviour, and the one stylesheet they resolve through. A consumer imports * exactly: * * ```ts @@ -11,20 +11,15 @@ * ``` * * The public surface is listed explicitly rather than `export *`, so what this - * package promises stays auditable. The scanners behind this repository's own - * design rules are published too, for a consumer to run over its own sources: - * `import { ... } from "@robomous/ui-core/gates"`. DESIGN.md names each rule - * and the gate that holds it. + * package promises can be read off this one file. Status colour is a semantic + * utility (`bg-success`, `text-warning`, `border-info`), not an export. */ -// The design tokens, and their prose contract in DESIGN.md. +// The runtime mirror of the stylesheet's tokens, for a caller that cannot read CSS. export { cssVar, DARK_THEME, LIGHT_THEME, THEME } from "./theme/tokens.js"; export { cn } from "cn"; -// The one home for status colour outside the Badge. -export { STATUS_INK, TONE_BORDER, TONE_FILL, type StatusTone } from "./theme/statusTone.js"; - // The components — Radix and Base UI behaviour, iconed with lucide. export { Button, buttonVariants } from "./components/button.js"; export { diff --git a/src/theme/statusTone.ts b/src/theme/statusTone.ts deleted file mode 100644 index 028fafb..0000000 --- a/src/theme/statusTone.ts +++ /dev/null @@ -1,52 +0,0 @@ -/** - * The one home for status colour outside the `Badge`. - * - * `badge.tsx` earns its own emerald/amber/sky because a chip's `/10` surface - * and hairline are a treatment nothing else draws. Everything else that paints - * a status — a dot, a timeline cell, an inline icon or word — reads its colour - * from here instead of naming a Tailwind family at its own call site, so a - * sixth place inventing a fourth spelling of "warning" is a type error, not a - * diff nobody notices. - */ -export type StatusTone = "neutral" | "accent" | "success" | "warning" | "destructive"; - -/** - * A tone's border and its fill, as whole utility names. - * - * Whole names rather than a `border-${tone}` template, because Tailwind scans - * source *text*: a class assembled at runtime is a class the build never saw and - * therefore a rule that is never emitted. The failure is silent and looks like a - * styling mistake, which is why these are written out. - * - * A status **dot** and a timeline **cell** are solid marks, not `Badge`s — a - * full-strength `*-500` fill is right there, where the `Badge`'s `/10` surface - * would vanish at 4px. - */ -export const TONE_BORDER: Record = { - neutral: "border-border", - accent: "border-primary", - success: "border-emerald-500 dark:border-emerald-400", - warning: "border-amber-500 dark:border-amber-400", - destructive: "border-destructive", -}; - -export const TONE_FILL: Record = { - neutral: "bg-muted-foreground", - accent: "bg-primary", - success: "bg-emerald-500 dark:bg-emerald-400", - warning: "bg-amber-500 dark:bg-amber-400", - destructive: "bg-destructive", -}; - -/** - * Ink for an icon or a run of inline text carrying a status — a `warning` - * triangle beside a sentence, not a dot or a cell, so it wants a legible - * foreground rather than `TONE_FILL`'s solid mark. `info` has no `StatusTone` - * counterpart: it names sky for surfaces that are informational without being - * any of `neutral`/`accent`/`success`/`warning`/`destructive`. - */ -export const STATUS_INK = { - success: "text-emerald-700 dark:text-emerald-400", - warning: "text-amber-700 dark:text-amber-400", - info: "text-sky-700 dark:text-sky-400", -} as const; diff --git a/src/theme/styles.css b/src/theme/styles.css index 8bf2bb0..9ea1bc2 100644 --- a/src/theme/styles.css +++ b/src/theme/styles.css @@ -1,30 +1,31 @@ /* - * @robomous/ui-core — the design system's stylesheet. + * @robomous/ui-core — the design system's stylesheet, and its one visual contract. * - * **This file is the design system's one home for a colour.** Tailwind v4 is - * CSS-first, so this is not a mirror of a config — it *is* the config, and - * every utility in this package and in every consuming app resolves through - * the names below. `brand` is Robomous's own identity colour, justified in - * `DESIGN.md`, and it is declared the way every other name here is: a value in - * `:root`, a dark counterpart in `.dark`, exposure through `@theme inline`. - * Consumers add their extensions in their own stylesheet after importing this - * one. `tokens.ts` is the TypeScript mirror of this file, for callers that - * cannot read CSS, and `tokens.test.ts` asserts the two agree declaration for - * declaration. + * Tailwind v4 is CSS-first, so this file is not a mirror of a config: it *is* + * the config. Every utility in this package and in every consuming app resolves + * through the names declared here. * - * `./tailwind.css` carries the variant and utility layer the components paint - * through: the `data-*` variants that key off the `data-state` attributes the - * behaviour libraries emit, plus the scroll-mask and shimmer utilities. It is - * imported ahead of the token blocks because a variant has to exist before a - * utility can qualify on it. + * The colour vocabulary is closed. `@theme { --color-*: initial }` removes + * Tailwind's default palette, so `bg-red-500` or `text-emerald-700` is not a + * word in this system — it produces no CSS. What exists is the set of roles + * below: surfaces (`background`, `card`, `popover`, `muted`), emphasis + * (`primary`, `secondary`, `accent`), status (`success`, `warning`, `info`, + * `destructive`), structure (`border`, `input`, `ring`, `overlay`) and the + * `sidebar` family two product shells share. A component or a screen names a + * role, never a pigment; the value behind a role is this file's business. + * + * `brand` is Robomous's identity colour and is deliberately *not* a utility: + * it is a CSS variable (`var(--brand)`) for a wordmark or a styleguide swatch, + * so it cannot drift onto a control by way of `bg-brand`. * * A consuming app imports exactly this: * * import "@robomous/ui-core/styles.css"; * - * and adds an `@source` for its own sources. Tailwind v4 is CSS-first: there - * is no `tailwind.config.js` anywhere in this repository and there must not - * be one, or the tokens acquire a second home. + * and adds an `@source` for its own sources. There is no `tailwind.config.js` + * anywhere in this repository and there must not be one, or the tokens acquire + * a second home. `tokens.ts` mirrors the values for a runtime caller that cannot + * read CSS; this file is authoritative. */ @import "tailwindcss"; @@ -34,22 +35,12 @@ /* * The package's own components, so a class string that only ever appears - * inside `ui-core` still produces CSS in the consumer's build. - * - * `@source` resolves **relative to this file**, and this file is in - * `src/theme/`. So the target is `..` — `src/`, which holds `components/` — - * and not `.`, which would be `src/theme/` and contains no component at all. - * A consumer's Tailwind auto-detects its own sources and never looks inside - * `node_modules`, so this directive is the only thing that puts these - * components' class strings in front of the compiler: point it one directory - * too shallow and every consumer builds a stylesheet with no `h-8`, no - * `line-clamp-1` and no variant utility in it, while every check in this - * repository stays green. - * - * `src/gates/design.test.ts` resolves this path and fails if it stops - * reaching the components. + * inside `ui-core` still produces CSS in the consumer's build. `@source` + * resolves relative to this file, and a consumer's Tailwind never looks inside + * `node_modules` on its own. Only the components are scanned: nothing else in + * the package carries a class string a consumer needs. */ -@source ".."; +@source "../components"; @custom-variant dark (&:is(.dark *)); @@ -68,16 +59,20 @@ --muted-foreground: oklch(0.556 0 0); --accent: oklch(0.97 0 0); --accent-foreground: oklch(0.205 0 0); + + /* Status roles. The name is the contract; the value is a detail. */ + --success: oklch(0.508 0.118 165.612); + --warning: oklch(0.555 0.163 48.998); + --info: oklch(0.5 0.134 242.749); --destructive: oklch(0.577 0.245 27.325); + --border: oklch(0.922 0 0); --input: oklch(0.922 0 0); --ring: oklch(0.708 0 0); - --chart-1: oklch(0.87 0 0); - --chart-2: oklch(0.556 0 0); - --chart-3: oklch(0.439 0 0); - --chart-4: oklch(0.371 0 0); - --chart-5: oklch(0.269 0 0); + /* The scrim behind a Dialog or a Sheet: a purpose, not a pigment. */ + --overlay: oklch(0 0 0 / 10%); --radius: 0.625rem; + --sidebar: oklch(0.985 0 0); --sidebar-foreground: oklch(0.145 0 0); --sidebar-primary: oklch(0.205 0 0); @@ -88,13 +83,10 @@ --sidebar-ring: oklch(0.708 0 0); /* - * Robomous orange (#F5580B). Identity only — the wordmark and its styleguide - * swatch, named in `DESIGN.md` — never a functional-UI colour. A third site - * is a design decision, not a styling one. - * - * At 3.34:1 on `background` it clears 3:1 for large text and non-text marks - * and misses 4.5:1 for body copy, which is one more reason the wordmark is - * where it belongs. + * Robomous orange (#F5580B). Identity only — a wordmark, a styleguide swatch — + * reached as `var(--brand)`, never as a utility. At 3.34:1 on `background` it + * clears 3:1 for large text and non-text marks and misses 4.5:1 for body copy, + * which is one more reason it paints no control. */ --brand: oklch(0.663 0.205 39.9); } @@ -114,15 +106,17 @@ --muted-foreground: oklch(0.708 0 0); --accent: oklch(0.269 0 0); --accent-foreground: oklch(0.985 0 0); + + --success: oklch(0.765 0.177 163.223); + --warning: oklch(0.828 0.189 84.429); + --info: oklch(0.746 0.16 232.661); --destructive: oklch(0.704 0.191 22.216); + --border: oklch(1 0 0 / 10%); --input: oklch(1 0 0 / 15%); --ring: oklch(0.556 0 0); - --chart-1: oklch(0.87 0 0); - --chart-2: oklch(0.556 0 0); - --chart-3: oklch(0.439 0 0); - --chart-4: oklch(0.371 0 0); - --chart-5: oklch(0.269 0 0); + --overlay: oklch(0 0 0 / 10%); + --sidebar: oklch(0.205 0 0); --sidebar-foreground: oklch(0.985 0 0); --sidebar-primary: oklch(0.488 0.243 264.376); @@ -135,40 +129,46 @@ --brand: oklch(0.663 0.205 39.9); } +@theme { + /* Close Tailwind's default palette. The roles below are the whole vocabulary. */ + --color-*: initial; +} + @theme inline { --font-sans: "Geist Variable", sans-serif; --font-heading: var(--font-sans); - --color-sidebar-ring: var(--sidebar-ring); - --color-sidebar-border: var(--sidebar-border); - --color-sidebar-accent-foreground: var(--sidebar-accent-foreground); - --color-sidebar-accent: var(--sidebar-accent); - --color-sidebar-primary-foreground: var(--sidebar-primary-foreground); - --color-sidebar-primary: var(--sidebar-primary); - --color-sidebar-foreground: var(--sidebar-foreground); - --color-sidebar: var(--sidebar); - --color-chart-5: var(--chart-5); - --color-chart-4: var(--chart-4); - --color-chart-3: var(--chart-3); - --color-chart-2: var(--chart-2); - --color-chart-1: var(--chart-1); - --color-ring: var(--ring); - --color-input: var(--input); - --color-border: var(--border); - --color-destructive: var(--destructive); - --color-accent-foreground: var(--accent-foreground); - --color-accent: var(--accent); - --color-muted-foreground: var(--muted-foreground); - --color-muted: var(--muted); - --color-secondary-foreground: var(--secondary-foreground); - --color-secondary: var(--secondary); - --color-primary-foreground: var(--primary-foreground); - --color-primary: var(--primary); - --color-popover-foreground: var(--popover-foreground); - --color-popover: var(--popover); - --color-card-foreground: var(--card-foreground); - --color-card: var(--card); - --color-foreground: var(--foreground); + --color-background: var(--background); + --color-foreground: var(--foreground); + --color-card: var(--card); + --color-card-foreground: var(--card-foreground); + --color-popover: var(--popover); + --color-popover-foreground: var(--popover-foreground); + --color-primary: var(--primary); + --color-primary-foreground: var(--primary-foreground); + --color-secondary: var(--secondary); + --color-secondary-foreground: var(--secondary-foreground); + --color-muted: var(--muted); + --color-muted-foreground: var(--muted-foreground); + --color-accent: var(--accent); + --color-accent-foreground: var(--accent-foreground); + --color-success: var(--success); + --color-warning: var(--warning); + --color-info: var(--info); + --color-destructive: var(--destructive); + --color-border: var(--border); + --color-input: var(--input); + --color-ring: var(--ring); + --color-overlay: var(--overlay); + --color-sidebar: var(--sidebar); + --color-sidebar-foreground: var(--sidebar-foreground); + --color-sidebar-primary: var(--sidebar-primary); + --color-sidebar-primary-foreground: var(--sidebar-primary-foreground); + --color-sidebar-accent: var(--sidebar-accent); + --color-sidebar-accent-foreground: var(--sidebar-accent-foreground); + --color-sidebar-border: var(--sidebar-border); + --color-sidebar-ring: var(--sidebar-ring); + --radius-sm: calc(var(--radius) * 0.6); --radius-md: calc(var(--radius) * 0.8); --radius-lg: var(--radius); @@ -176,25 +176,21 @@ --radius-2xl: calc(var(--radius) * 1.8); --radius-3xl: calc(var(--radius) * 2.2); --radius-4xl: calc(var(--radius) * 2.6); - - /* Robomous's one identity token, exposed the same way as every other colour. */ - --color-brand: var(--brand); } @layer base { /* The hairline and the outline *colour* — the two things every element - inherits. The focus treatment itself belongs to the components: each - focusable one carries its own `ring-3` ring in `ring/50` (DESIGN.md, - Borders and focus), so the base layer names the colour and stays out of - the geometry. - Nothing here may declare focus geometry again: one blanket declaration in - this layer is how thirteen components' rings got overridden at once. */ + inherits. Focus geometry belongs to the components: each focusable one + carries its own `focus-visible:ring-3 focus-visible:ring-ring/50`, so this + layer names the colour and stays out of the geometry. One blanket + `:focus-visible` declaration here is how thirteen components' rings once + got overridden at once. */ * { @apply border-border outline-ring/50; } /* Motion is opt-out for vestibular safety: under prefers-reduced-motion every animation and transition collapses to a frame. Information is never - motion-only (DESIGN.md §Accessibility), so nothing is lost. */ + motion-only, so nothing is lost. */ @media (prefers-reduced-motion: reduce) { *, ::before, @@ -208,9 +204,8 @@ body { @apply bg-background text-foreground; } - /* Scoped to what is actually pressable and away from what is not: a disabled - control keeps the arrow, because a hand over something that will not - respond is the cursor telling a lie. */ + /* Scoped to what is actually pressable: a disabled control keeps the arrow, + because a hand over something that will not respond is a lie. */ button:not(:disabled), [role="button"]:not(:disabled) { cursor: pointer; @@ -218,9 +213,8 @@ html { @apply font-sans; } - /* One family now: `--font-heading` resolves to `--font-sans`, so this rule - keeps the semantic hook without changing the face. A later decision that - splits them again lands here without a diff at every heading. */ + /* One family: `--font-heading` resolves to `--font-sans`, so this rule keeps + the semantic hook without changing the face. */ h1, h2, h3, diff --git a/src/theme/tokens.ts b/src/theme/tokens.ts index a45828e..0f53fe5 100644 --- a/src/theme/tokens.ts +++ b/src/theme/tokens.ts @@ -1,18 +1,16 @@ /** - * The design tokens, as TypeScript. + * The design tokens, as TypeScript — a compatibility mirror, not a source. * - * `styles.css` is the one that *runs* — Tailwind reads its `:root`, `.dark` - * and `@theme inline` blocks and every utility in the package comes out of - * them. This module exists for the two kinds of caller that cannot read CSS: - * a ``/`` that needs a colour as a string, and `tokens.test.ts`, - * which parses the stylesheet and asserts the two agree, declaration for - * declaration. + * `styles.css` is authoritative: Tailwind reads its `:root`, `.dark` and + * `@theme` blocks and every utility comes out of them. This module exists for + * the runtime caller that cannot read CSS — a `` or an `` that + * needs a colour as a string, or a styleguide printing a value beside a swatch. + * Prefer `var(--foreground)` or + * `getComputedStyle(element).getPropertyValue("--foreground")` where the DOM + * is available; reach for these maps only when it is not. * - * `LIGHT_THEME`/`DARK_THEME` carry the semantic vocabulary on a neutral base - * colour and chart palette, plus one name of this package's own: `brand`, - * Robomous orange, identity only. A consumer's own vocabulary lives in the - * consumer's stylesheet and token module, never here — DESIGN.md, - * *Per-consumer extensions*. + * `tests/theme/tokens.test.ts` parses the stylesheet and asserts the two agree + * declaration for declaration, so the mirror cannot drift silently. */ export const LIGHT_THEME: Readonly> = Object.freeze({ @@ -30,15 +28,14 @@ export const LIGHT_THEME: Readonly> = Object.freeze({ "muted-foreground": "oklch(0.556 0 0)", accent: "oklch(0.97 0 0)", "accent-foreground": "oklch(0.205 0 0)", + success: "oklch(0.508 0.118 165.612)", + warning: "oklch(0.555 0.163 48.998)", + info: "oklch(0.5 0.134 242.749)", destructive: "oklch(0.577 0.245 27.325)", border: "oklch(0.922 0 0)", input: "oklch(0.922 0 0)", ring: "oklch(0.708 0 0)", - "chart-1": "oklch(0.87 0 0)", - "chart-2": "oklch(0.556 0 0)", - "chart-3": "oklch(0.439 0 0)", - "chart-4": "oklch(0.371 0 0)", - "chart-5": "oklch(0.269 0 0)", + overlay: "oklch(0 0 0 / 10%)", sidebar: "oklch(0.985 0 0)", "sidebar-foreground": "oklch(0.145 0 0)", "sidebar-primary": "oklch(0.205 0 0)", @@ -48,8 +45,7 @@ export const LIGHT_THEME: Readonly> = Object.freeze({ "sidebar-border": "oklch(0.922 0 0)", "sidebar-ring": "oklch(0.708 0 0)", - // Robomous orange (#F5580B). Identity only — the wordmark and its styleguide - // swatch — never a functional-UI colour. + // Robomous orange (#F5580B). Identity only; a CSS variable, never a utility. brand: "oklch(0.663 0.205 39.9)", }); @@ -68,15 +64,14 @@ export const DARK_THEME: Readonly> = Object.freeze({ "muted-foreground": "oklch(0.708 0 0)", accent: "oklch(0.269 0 0)", "accent-foreground": "oklch(0.985 0 0)", + success: "oklch(0.765 0.177 163.223)", + warning: "oklch(0.828 0.189 84.429)", + info: "oklch(0.746 0.16 232.661)", destructive: "oklch(0.704 0.191 22.216)", border: "oklch(1 0 0 / 10%)", input: "oklch(1 0 0 / 15%)", ring: "oklch(0.556 0 0)", - "chart-1": "oklch(0.87 0 0)", - "chart-2": "oklch(0.556 0 0)", - "chart-3": "oklch(0.439 0 0)", - "chart-4": "oklch(0.371 0 0)", - "chart-5": "oklch(0.269 0 0)", + overlay: "oklch(0 0 0 / 10%)", sidebar: "oklch(0.205 0 0)", "sidebar-foreground": "oklch(0.985 0 0)", "sidebar-primary": "oklch(0.488 0.243 264.376)", diff --git a/tests/components/components.test.tsx b/tests/components/components.test.tsx index 14df440..1a5135d 100644 --- a/tests/components/components.test.tsx +++ b/tests/components/components.test.tsx @@ -146,8 +146,11 @@ describe("Alert and Badge", () => { render(x); const el = screen.getByText("x"); expect(el.getAttribute("data-variant")).toBe(variant); + // A status chip is a soft surface and readable ink on a semantic role — + // never a coloured stroke, never a physical palette family. expect(el.className).toContain("border-transparent"); - expect(el.className).not.toMatch(/border-(emerald|amber|sky)/); + expect(el.className).toMatch(/\bbg-(success|warning|info)\/10\b|\bbg-muted\b/); + expect(el.className).not.toMatch(/emerald|amber|sky|border-(success|warning|info)/); }, ); }); diff --git a/tests/gates/design.test.ts b/tests/gates/design.test.ts deleted file mode 100644 index 2639e17..0000000 --- a/tests/gates/design.test.ts +++ /dev/null @@ -1,229 +0,0 @@ -// @vitest-environment node -import { readFileSync } from "node:fs"; -import path from "node:path"; -import { spawnSync } from "node:child_process"; -import { fileURLToPath } from "node:url"; -import { expect, test } from "vitest"; - -import { competingStatusPaletteIn, statusPaletteIn } from "../../src/gates/index.js"; - -const REPO = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "..", ".."); -const read = (rel: string) => readFileSync(path.join(REPO, rel), "utf8"); - -const SOURCE = /\.(?:ts|tsx|css)$/; - -/** - * Assembles a Tailwind utility fixture (e.g. "bg-emerald-500") from its - * prefix and family/shade at runtime. This file scans its own package (it - * lives under `src/`), so a fixture spelled out contiguously would trip the - * very gates below on itself — the same reason `HEX` and - * `RETIRED_ICON_PACKAGE` are built from fragments in the sibling gate files. - */ -const cls = (prefix: string, familyAndShade: string) => `${prefix}-${familyAndShade}`; - -/** Every tracked package file matching `SOURCE`. */ -function packageSources(): string[] { - const listed = spawnSync("git", ["ls-files", "-z", "src"], { cwd: REPO, encoding: "utf8" }); - expect(listed.status, `git ls-files failed: ${listed.stderr}`).toBe(0); - return listed.stdout.split("\0").filter((name) => SOURCE.test(name)); -} - -const FOUNDATION_BADGE = ["success", "warning", "info", "quiet"]; - -/** The class string of one variant line in a cva source. */ -function variantClasses(source: string, key: string): string { - const m = source.match(new RegExp(String.raw`^\s*"?${key}"?:\s*\n?\s*"([^"]*)"`, "m")); - expect(m, `no variant ${key}`).toBeTruthy(); - return m![1]; -} - -const BADGE = "src/components/badge.tsx"; - -test("Badge's status vocabulary is exactly the four owned names", () => { - const source = read("src/components/badge.tsx"); - for (const name of FOUNDATION_BADGE) { - expect(source.includes(`${name}:`), `badge.tsx is missing the ${name} variant`).toBeTruthy(); - } -}); - -test("a status Badge paints a soft surface and readable ink, never a coloured stroke", () => { - const src = read(BADGE); - const stroke = /\bborder-(?:emerald|amber|sky|success|warning|destructive|primary)\b/; - for (const k of [...FOUNDATION_BADGE, "destructive"]) { - expect(variantClasses(src, k), `${k} adds a coloured border`).not.toMatch(stroke); - } - expect(variantClasses(src, "success")).toMatch(/\bbg-emerald-500\/10\b/); - expect(variantClasses(src, "success")).toMatch(/\btext-emerald-700\b/); - expect(variantClasses(src, "warning")).toMatch(/\bbg-amber-500\/10\b/); - expect(variantClasses(src, "warning")).toMatch(/\btext-amber-700\b/); - expect(variantClasses(src, "info")).toMatch(/\bbg-sky-500\/10\b/); - expect(variantClasses(src, "info")).toMatch(/\btext-sky-700\b/); - expect(variantClasses(src, "quiet")).toMatch(/\bbg-muted\b/); - expect(variantClasses(src, "quiet")).toMatch(/\btext-muted-foreground\b/); - // Official destructive stays on the semantic token, never red-*. - expect(variantClasses(src, "destructive")).toMatch(/\bbg-destructive\/10\b/); - expect(src).not.toMatch(/\b(?:bg|text)-red-\d/); -}); - -test("statusPaletteIn finds the emerald/amber/sky family, and not a token or a comment", () => { - expect(statusPaletteIn("a.tsx", ` className="${cls("bg", "emerald-500/10")}"`)).toEqual([ - `a.tsx:1: className="${cls("bg", "emerald-500/10")}"`, - ]); - expect( - statusPaletteIn( - "b.tsx", - ` className="${cls("border", "amber-400")} dark:${cls("border", "amber-300")}"`, - ), - ).toEqual([ - `b.tsx:1: className="${cls("border", "amber-400")} dark:${cls("border", "amber-300")}"`, - ]); - expect(statusPaletteIn("c.tsx", ` className="${cls("ring", "sky-500")}"`)).toEqual([ - `c.tsx:1: className="${cls("ring", "sky-500")}"`, - ]); - expect(statusPaletteIn("d.tsx", ` className="${cls("shadow", "emerald-500/20")}"`)).toEqual([ - `d.tsx:1: className="${cls("shadow", "emerald-500/20")}"`, - ]); - // A token, not the palette. - expect(statusPaletteIn("e.tsx", ` className="bg-primary text-primary-foreground"`)).toEqual([]); - // The family name without a shade is not yet a colour. - expect(statusPaletteIn("f.tsx", ` className="bg-emerald"`)).toEqual([]); - // A comment recalling the palette states history, not a usage. - expect( - statusPaletteIn("g.tsx", ` // never ${cls("bg", "emerald-500")} outside statusTone`), - ).toEqual([]); -}); - -test("competingStatusPaletteIn finds a rival colour family, and not the status palette itself", () => { - expect(competingStatusPaletteIn("a.tsx", ` className="${cls("bg", "green-500")}"`)).toEqual([ - `a.tsx:1: className="${cls("bg", "green-500")}"`, - ]); - expect(competingStatusPaletteIn("b.tsx", ` className="${cls("text", "yellow-700")}"`)).toEqual([ - `b.tsx:1: className="${cls("text", "yellow-700")}"`, - ]); - expect(competingStatusPaletteIn("c.tsx", ` className="${cls("border", "blue-400")}"`)).toEqual([ - `c.tsx:1: className="${cls("border", "blue-400")}"`, - ]); - expect(competingStatusPaletteIn("d.tsx", ` className="${cls("bg", "emerald-500")}"`)).toEqual( - [], - ); - expect( - competingStatusPaletteIn("e.tsx", ` // never ${cls("bg", "red-500")} for a destructive state`), - ).toEqual([]); -}); - -test("the status palette lives in exactly Badge and statusTone, nowhere else", () => { - const ALLOWED_PALETTE_FILES = [ - "src/components/badge.tsx", - "src/theme/statusTone.ts", - "src/theme/statusTone.test.ts", - ]; - const tracked = packageSources(); - expect( - tracked.length > 0, - "the scan found no package sources, so it proves nothing", - ).toBeTruthy(); - - const offenders = tracked - .filter((file) => !ALLOWED_PALETTE_FILES.includes(file)) - .flatMap((file) => statusPaletteIn(file, readFileSync(path.join(REPO, file), "utf8"))); - expect( - offenders, - "the status palette has exactly one home outside Badge and statusTone — read the tone from " + - `src/theme/statusTone.ts instead:\n${offenders.join("\n")}`, - ).toEqual([]); -}); - -test("no competing colour family stands in for the status palette anywhere in the package", () => { - const tracked = packageSources(); - expect( - tracked.length > 0, - "the scan found no package sources, so it proves nothing", - ).toBeTruthy(); - - const offenders = tracked.flatMap((file) => - competingStatusPaletteIn(file, readFileSync(path.join(REPO, file), "utf8")), - ); - expect( - offenders, - `a competing colour family stands in for the status palette:\n${offenders.join("\n")}`, - ).toEqual([]); -}); - -/** - * Every component module is part of the public surface. - * - * `src/index.ts` lists its exports one by one rather than re-exporting a - * directory, which is what keeps the promise auditable — and is also what lets - * a finished component sit in `src/components/` for months without ever - * reaching a consumer. `separator.tsx` did exactly that: written, imported by - * `field.tsx`, never exported, and nothing noticed because every other check - * here reads the files rather than the surface. - * - * A module that is deliberately internal has no home in `src/components/`; move - * it beside its one caller instead, and this gate stops asking about it. - */ -test("src/index.ts exports every component module in src/components", () => { - const listed = spawnSync("git", ["ls-files", "-z", "src/components"], { - cwd: REPO, - encoding: "utf8", - }); - expect(listed.status, `git ls-files failed: ${listed.stderr}`).toBe(0); - - const modules = listed.stdout - .split("\0") - .filter((name) => name.endsWith(".tsx") && !name.includes(".test.")) - .map((name) => path.basename(name, ".tsx")); - expect( - modules.length > 0, - "no component modules were found, so this proves nothing", - ).toBeTruthy(); - - const surface = read("src/index.ts"); - const missing = modules.filter((name) => !surface.includes(`from "./components/${name}.js"`)); - expect( - missing, - "these components exist but no consumer can import them — add an export to src/index.ts, " + - `or move the module next to its only caller:\n${missing.join("\n")}`, - ).toEqual([]); -}); - -/** - * The stylesheet's `@source` still reaches the components. - * - * `styles.css` ships as source and a consumer's Tailwind compiles it. That - * compiler auto-detects the *consumer's* files and never walks - * `node_modules`, so `@source` is the only thing that puts this package's - * class strings in front of it. The directive resolves relative to the - * stylesheet, so moving the stylesheet moves the target — and pointing it one - * directory too shallow costs a consumer every utility that only this package - * writes, with no error anywhere: the CSS compiles, it is simply missing - * `h-8`, `line-clamp-1` and every variant utility the components rely on. - * - * Nothing else here can catch that, because nothing here compiles CSS. This - * resolves the path and asks whether the components are under it. - */ -test("the stylesheet's @source resolves to a directory that holds the components", () => { - const stylesheet = read("src/theme/styles.css"); - const directives = [...stylesheet.matchAll(/^\s*@source\s+"([^"]+)"\s*;/gm)].map((m) => m[1]); - expect(directives.length, "styles.css declares no @source, so it scans nothing").toBeGreaterThan( - 0, - ); - - const stylesheetDir = path.join(REPO, "src", "theme"); - const componentDir = path.join(REPO, "src", "components"); - - const reaching = directives.filter((spec) => { - const target = path.resolve(stylesheetDir, spec); - // `@source` scans a directory recursively, so it reaches the components - // when its target is the component directory or an ancestor of it. - const rel = path.relative(target, componentDir); - return rel === "" || (!rel.startsWith("..") && !path.isAbsolute(rel)); - }); - - expect( - reaching, - "no @source in src/theme/styles.css reaches src/components, so a consumer's build would " + - "emit none of this package's own utilities. @source resolves relative to the stylesheet: " + - `from src/theme/ the components are at "..". Declared: ${directives.join(", ")}`, - ).not.toEqual([]); -}); diff --git a/tests/gates/index.test.ts b/tests/gates/index.test.ts deleted file mode 100644 index 456dcbc..0000000 --- a/tests/gates/index.test.ts +++ /dev/null @@ -1,11 +0,0 @@ -// @vitest-environment node -import { execFileSync } from "node:child_process"; -import { expect, test } from "vitest"; - -test("foundationTokenNames resolves the stylesheet from the built package", () => { - const script = `import("./dist/gates/index.js").then((m) => { const n = m.foundationTokenNames(); if (!n.includes("background")) { throw new Error("missing token: " + n.join(",")); } console.log("ok"); });`; - const out = execFileSync(process.execPath, ["--input-type=module", "-e", script], { - encoding: "utf8", - }); - expect(out.trim()).toBe("ok"); -}); diff --git a/tests/gates/tokens.test.ts b/tests/gates/tokens.test.ts deleted file mode 100644 index 313e08b..0000000 --- a/tests/gates/tokens.test.ts +++ /dev/null @@ -1,215 +0,0 @@ -// @vitest-environment node -/** - * `DESIGN.md`'s first principle, machine-enforced: **never a colour in a class - * string.** The helpers live in `./index.ts` so consumers run the same scans; - * this file keeps their fabricated-input self-tests and runs the repo gates - * over this package's own sources. - */ - -import { readFileSync } from "node:fs"; -import path from "node:path"; -import { spawnSync } from "node:child_process"; -import { fileURLToPath } from "node:url"; -import { expect, test } from "vitest"; - -import { brandUsagesIn, colouredClassesIn } from "../../src/gates/index.js"; - -const REPO = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "..", ".."); - -const SOURCE = /\.(?:ts|tsx|css)$/; - -// Fragment for fabricating violating input without this file matching itself. -const HEX = ["#", "[0-9a-fA-F]{3,8}"].join(""); - -/** - * Assembles a bracketed Tailwind arbitrary-value utility (e.g. - * `text-[var(--accent)]`) from its prefix and bracket contents at runtime, for - * the same reason `HEX` above is built from fragments: this file scans its - * own package, so a fixture spelled out contiguously would trip the very gate - * it exists to exercise. - */ -const bracket = (prefix: string, inner: string) => `${prefix}-[${inner}]`; - -/** - * Assembles a `-brand` utility fixture at runtime, for the - * same self-matching reason as `bracket` above. - */ -const brand = (prefix: string, suffix = "") => `${prefix}-brand${suffix}`; - -test("the scan finds a colour smuggled into a class, and nothing that merely looks like one", () => { - expect(colouredClassesIn("a.tsx", `
`)).toEqual( - [`a.tsx:1:
`], - ); - expect(colouredClassesIn("b.tsx", ` className="${bracket("text", "var(--accent)")}"`)).toEqual([ - `b.tsx:1: className="${bracket("text", "var(--accent)")}"`, - ]); - expect(colouredClassesIn("c.tsx", ` className="${bracket("ring", "rgb(0 0 0)")}"`)).toEqual([ - `c.tsx:1: className="${bracket("ring", "rgb(0 0 0)")}"`, - ]); - - // A token utility is the whole point of the rule and must pass. - expect(colouredClassesIn("d.tsx", ` className="bg-primary text-primary-foreground"`)).toEqual( - [], - ); - // The accent at 10% is a token with an opacity modifier, not a colour. - expect(colouredClassesIn("e.tsx", ` className="bg-primary/10 border-primary"`)).toEqual([]); - // An arbitrary value that is *not* a colour stays legal — the rule is about - // colour, and a one-off `top-[50%]` is not what v1 got wrong. - expect(colouredClassesIn("f.tsx", ` className="translate-y-[3px]"`)).toEqual([]); - // An inline style carrying a schema-supplied colour is the sanctioned road: - // `classColor` answers with whatever the kernel stored, and Tailwind has never - // seen it, so no utility could name it. - expect( - colouredClassesIn("g.tsx", ` style={{ background: classColor(declared, name) }}`), - ).toEqual([]); - // A docstring explaining the rule must pass, or the gate forbids its own - // explanation — the mistake a boundary scan makes when it matches its own prose. - expect(colouredClassesIn("h.tsx", ` * Never write \`bg-[${"#"}eb5a47]\`.`)).toEqual([]); - // And a CSS custom property *declaration* is where colours are supposed to live. - expect(colouredClassesIn("i.css", ` --color-primary: #eb5a47;`)).toEqual([]); - // A colour-mix of two tokens names no colour of its own — the preset's own - // Button hover step. - expect( - colouredClassesIn( - "x.tsx", - 'className="hover:bg-[color-mix(in_oklch,var(--secondary),var(--foreground)_5%)]"', - ), - ).toEqual([]); - // A colour-mix that mixes in a literal is still a colour smuggled into a class. - expect( - colouredClassesIn("x.tsx", `className="${bracket("bg", "color-mix(in_srgb,#fff,var(--x))")}"`) - .length, - ).toBe(1); - // A named CSS colour inside color-mix is still a colour. - expect( - colouredClassesIn("x.tsx", `className="${bracket("bg", "color-mix(in_oklch,red,var(--x))")}"`) - .length, - ).toBe(1); - // Whatever order the tokens come in, and whichever colour space, two tokens - // stay two tokens. - expect( - colouredClassesIn("x.tsx", 'className="bg-[color-mix(in_srgb,var(--a)_40%,var(--b))]"').length, - ).toBe(0); -}); - -test("the brand scan counts a usage, and not the comment that states the rule", () => { - // A utility usage on any of the six colour-bearing prefixes is counted. - expect(brandUsagesIn("a.tsx", ` Robomous`)).toEqual([ - { file: "a.tsx", at: 1, text: `Robomous` }, - ]); - expect( - brandUsagesIn("b.tsx", ` className="h-full ${brand("bg")} transition-transform"`), - ).toEqual([ - { file: "b.tsx", at: 1, text: `className="h-full ${brand("bg")} transition-transform"` }, - ]); - // An opacity modifier is still a usage of the brand colour. - expect(brandUsagesIn("c.tsx", ` className="${brand("bg", "/10")}"`).map((u) => u.at)).toEqual([ - 1, - ]); - // A comment line states the rule rather than applying it. - expect(brandUsagesIn("d.css", ` * a third \`${brand("bg")}\` is a design decision`)).toEqual( - [], - ); - expect(brandUsagesIn("e.tsx", ` // never add ${brand("bg")} here`)).toEqual([]); - // Another token on the same prefixes is not the brand. - expect(brandUsagesIn("f.tsx", ` className="bg-primary text-primary-foreground"`)).toEqual([]); - // The token *name* without a utility prefix is not a usage — tokens.test.ts - // asserts LIGHT_THEME.brand's value and must not trip the gate. - expect(brandUsagesIn("g.ts", ` expect(COLOR.brand).toBe("#e85d44");`)).toEqual([]); -}); - -/** Every tracked source in this repo. */ -function repoSources(): string[] { - const listed = spawnSync("git", ["ls-files", "-z"], { cwd: REPO, encoding: "utf8" }); - expect(listed.status, `git ls-files failed: ${listed.stderr}`).toBe(0); - return listed.stdout.split("\0").filter((name) => SOURCE.test(name)); -} - -test("no source puts a colour inside a class name", () => { - const tracked = repoSources(); - expect(tracked.length > 0, "the scan found no sources, so it proves nothing").toBeTruthy(); - - const offenders = tracked.flatMap((file) => - colouredClassesIn(file, readFileSync(path.join(REPO, file), "utf8")), - ); - expect( - offenders, - "colour belongs to the token contract — add a token to " + - `src/theme/styles.css and name the intent:\n${offenders.join("\n")}`, - ).toEqual([]); -}); - -/** - * This package has zero brand sites: it declares the `--brand` token and never - * paints with it. `DESIGN.md` "Where the brand is": brand sites are a consumer - * decision — two enumerated sites per consuming app (the wordmark and the - * styleguide swatch that displays the value), gated in the consumer's repo. - */ -const BRAND_SITES: string[] = []; - -test("the brand colour paints nothing here — brand sites are a consumer decision", () => { - const tracked = repoSources(); - expect(tracked.length > 0, "the scan found no sources, so it proves nothing").toBeTruthy(); - - const usages = tracked.flatMap((file) => - brandUsagesIn(file, readFileSync(path.join(REPO, file), "utf8")), - ); - expect( - usages.map((u) => u.file).sort(), - "DESIGN.md 'Where the brand is': this package declares the brand token and never uses it — " + - "a brand-coloured site belongs to a consuming app (two enumerated sites per app):\n" + - usages.map((u) => `${u.file}:${u.at}: ${u.text}`).join("\n"), - ).toEqual(BRAND_SITES); -}); - -/** - * Lucide is the icon set, and the only one. - * - * The rule is "one icon library", not "this particular library". So this - * guards whichever set is currently *not* in use, and the value below is the - * whole of what changes when that decision changes. - * - * Assembled from fragments so this file never holds the package's name as a - * contiguous string, and a repository-wide sweep for it never mistakes its own - * guard for a lingering usage. - */ -const RETIRED_ICON_PACKAGE = ["@tabler", "icons-react"].join("/"); - -test("no package declares a second icon set, and no source imports one", () => { - const listed = spawnSync("git", ["ls-files", "-z"], { cwd: REPO, encoding: "utf8" }); - expect(listed.status, `git ls-files failed: ${listed.stderr}`).toBe(0); - const tracked = listed.stdout.split("\0").filter(Boolean); - - const manifests = tracked.filter((name) => /(?:^|\/)package\.json$/.test(name)); - expect(manifests.length > 0, "no manifests were read, so this proves nothing").toBeTruthy(); - const declaring = manifests.filter((name) => - readFileSync(path.join(REPO, name), "utf8").includes(`"${RETIRED_ICON_PACKAGE}"`), - ); - expect( - declaring, - `this package draws one icon set, and ${RETIRED_ICON_PACKAGE} is not it. ` + - `A second one is a decision for DESIGN.md, not a dependency:\n${declaring.join("\n")}`, - ).toEqual([]); - - const sources = tracked.filter((name) => SOURCE.test(name)); - expect(sources.length > 0, "no sources were read, so this proves nothing").toBeTruthy(); - const importing = sources.filter((name) => - new RegExp(String.raw`(?:from|require\()\s*["']${RETIRED_ICON_PACKAGE}["']`).test( - readFileSync(path.join(REPO, name), "utf8"), - ), - ); - expect(importing, `these draw from the retired icon set:\n${importing.join("\n")}`).toEqual([]); -}); - -test("the tokens have exactly one home, and it is the stylesheet", () => { - const listed = spawnSync("git", ["ls-files", "-z"], { cwd: REPO, encoding: "utf8" }); - expect(listed.status, `git ls-files failed: ${listed.stderr}`).toBe(0); - const configs = listed.stdout - .split("\0") - .filter((name) => /(?:^|\/)tailwind\.config\.[cm]?[jt]s$/.test(name)); - expect( - configs, - "Tailwind v4 is CSS-first: the tokens live in src/theme/styles.css. " + - `A config file gives them a second definition that wins for some utilities and not others:\n${configs.join("\n")}`, - ).toEqual([]); -}); diff --git a/tests/theme/statusTone.test.ts b/tests/theme/statusTone.test.ts deleted file mode 100644 index d941dc3..0000000 --- a/tests/theme/statusTone.test.ts +++ /dev/null @@ -1,24 +0,0 @@ -import { describe, expect, it } from "vitest"; - -import { STATUS_INK, TONE_BORDER, TONE_FILL } from "../../src/theme/statusTone"; - -describe("status tones", () => { - it("names one Tailwind family per status and never a retired token", () => { - expect(TONE_FILL.success).toBe("bg-emerald-500 dark:bg-emerald-400"); - expect(TONE_FILL.warning).toBe("bg-amber-500 dark:bg-amber-400"); - expect(TONE_BORDER.success).toBe("border-emerald-500 dark:border-emerald-400"); - expect(TONE_BORDER.warning).toBe("border-amber-500 dark:border-amber-400"); - expect(TONE_FILL.destructive).toBe("bg-destructive"); - expect(TONE_BORDER.neutral).toBe("border-border"); - expect(STATUS_INK.warning).toBe("text-amber-700 dark:text-amber-400"); - expect(STATUS_INK.success).toBe("text-emerald-700 dark:text-emerald-400"); - expect(STATUS_INK.info).toBe("text-sky-700 dark:text-sky-400"); - for (const v of [ - ...Object.values(TONE_FILL), - ...Object.values(TONE_BORDER), - ...Object.values(STATUS_INK), - ]) { - expect(v).not.toMatch(/\b(?:bg|border|text)-(?:success|warning)\b/); - } - }); -}); diff --git a/tests/theme/tokens.test.ts b/tests/theme/tokens.test.ts index 08bb437..3dd6594 100644 --- a/tests/theme/tokens.test.ts +++ b/tests/theme/tokens.test.ts @@ -1,17 +1,14 @@ /** * @vitest-environment node * - * The token contract: `styles.css` is the one home for a colour (`:root`, - * `.dark`, `@theme inline`, the base layer) — the semantic vocabulary plus - * `brand`, the one Robomous extension. `tokens.ts` is the mirror a - * ``/`` or a test reads a colour off of; this suite parses the - * stylesheet structurally and asserts the two agree, declaration for - * declaration, and that none of the tokens this rewrite retired have crept - * back in. + * The token contract. `styles.css` is authoritative and `tokens.ts` is its + * runtime mirror; this suite parses the stylesheet structurally and asserts + * the two agree declaration for declaration, that the colour namespace is + * closed, and that the few stylesheet-level rules the components depend on are + * still there. * - * The CSS is parsed rather than imported — same reason as before: vitest's - * jsdom does not evaluate `@theme`, and `import.meta.url` under jsdom is an - * `http://localhost/` URL that `fileURLToPath` rejects. Hence `node` above. + * Parsed rather than imported: nothing here evaluates `@theme`, and the + * packed-consumer test is where a real Tailwind compiles this file. */ import { readFileSync } from "node:fs"; @@ -26,21 +23,12 @@ const STYLESHEET = readFileSync( "utf8", ); -/** - * Whitespace and quote style are presentation: a value that wraps, and a font - * family spelled in double quotes rather than single, are the same value. Both - * change under an ordinary formatter run and neither changes what a browser - * resolves, so neither may fail this suite. - */ +/** Whitespace and quote style are presentation, not value. */ function normalize(value: string): string { return value.replace(/\s+/g, " ").replace(/"/g, "'").trim(); } -/** - * The `{ … }` body belonging to the block whose header (e.g. `:root {` or - * `.dark {`) appears first in the file, matched by brace depth rather than by - * the next `\n}` so a nested `calc(...)` or `color-mix(...)` cannot fool it. - */ +/** The `{ … }` body of the block whose header appears first, matched by brace depth. */ function blockBody(css: string, header: string): string { const headerAt = css.indexOf(header); expect(headerAt, `styles.css has no ${JSON.stringify(header)} block`).toBeGreaterThan(-1); @@ -73,21 +61,9 @@ function declarations(block: string): Map { return map; } -// The semantic vocabulary, written out here rather than derived from -// `tokens.ts`, so a mistake in the mirror cannot also erase the thing it was -// supposed to mirror. -const CHART_NAMES = ["chart-1", "chart-2", "chart-3", "chart-4", "chart-5"]; -const SIDEBAR_NAMES = [ - "sidebar", - "sidebar-foreground", - "sidebar-primary", - "sidebar-primary-foreground", - "sidebar-accent", - "sidebar-accent-foreground", - "sidebar-border", - "sidebar-ring", -]; -const BASE_SEMANTIC_NAMES = [ +// The vocabulary, written out rather than derived from `tokens.ts`, so a +// mistake in the mirror cannot also erase the thing it was supposed to mirror. +const ROLE_NAMES = [ "background", "foreground", "card", @@ -102,91 +78,76 @@ const BASE_SEMANTIC_NAMES = [ "muted-foreground", "accent", "accent-foreground", + "success", + "warning", + "info", "destructive", "border", "input", "ring", + "overlay", + "sidebar", + "sidebar-foreground", + "sidebar-primary", + "sidebar-primary-foreground", + "sidebar-accent", + "sidebar-accent-foreground", + "sidebar-border", + "sidebar-ring", ]; -const SEMANTIC_NAMES = [...BASE_SEMANTIC_NAMES, ...CHART_NAMES, ...SIDEBAR_NAMES]; -// The one name this package keeps beyond the semantic vocabulary. -const EXTENSION_NAMES = ["brand"]; - -const NEUTRAL_CHART = { - "chart-1": "oklch(0.87 0 0)", - "chart-2": "oklch(0.556 0 0)", - "chart-3": "oklch(0.439 0 0)", - "chart-4": "oklch(0.371 0 0)", - "chart-5": "oklch(0.269 0 0)", -} as const; +// Declared as a variable in both themes, exposed as no utility. +const VARIABLE_ONLY = ["brand"]; describe(":root", () => { const root = declarations(blockBody(STYLESHEET, ":root {")); - it("declares every semantic variable, the one extension, and --radius, and nothing else", () => { - expect([...root.keys()].sort()).toEqual( - [...SEMANTIC_NAMES, ...EXTENSION_NAMES, "radius"].sort(), - ); + it("declares every role, the brand variable and --radius, and nothing else", () => { + expect([...root.keys()].sort()).toEqual([...ROLE_NAMES, ...VARIABLE_ONLY, "radius"].sort()); }); - it("matches LIGHT_THEME's value for every name it declares", () => { - for (const [name, value] of root) { - if (name === "radius") continue; - expect(LIGHT_THEME[name], `LIGHT_THEME is missing ${name}`).toBe(value); - } + it("agrees with LIGHT_THEME declaration for declaration", () => { + const light = new Map(Object.entries(LIGHT_THEME)); + root.delete("radius"); + expect(Object.fromEntries(root)).toEqual(Object.fromEntries(light)); }); - it("names no variable LIGHT_THEME does not also carry", () => { - const lightKeys = Object.keys(LIGHT_THEME).sort(); - const rootKeys = [...root.keys()].filter((name) => name !== "radius").sort(); - expect(lightKeys).toEqual(rootKeys); - }); - - it("pins --radius to the medium step, in both the CSS and THEME", () => { - expect(root.get("radius")).toBe("0.625rem"); + it("pins --radius in both the CSS and THEME", () => { + expect(declarations(blockBody(STYLESHEET, ":root {")).get("radius")).toBe("0.625rem"); expect(THEME.radius).toBe("0.625rem"); }); - - it("pins the neutral chart palette exactly", () => { - for (const [name, value] of Object.entries(NEUTRAL_CHART)) { - expect(root.get(name)).toBe(value); - } - }); }); describe(".dark", () => { const dark = declarations(blockBody(STYLESHEET, ".dark {")); - it("declares every semantic variable and the one extension, and nothing else (no --radius)", () => { - expect([...dark.keys()].sort()).toEqual([...SEMANTIC_NAMES, ...EXTENSION_NAMES].sort()); + it("declares every role and the brand variable, and nothing else", () => { + expect([...dark.keys()].sort()).toEqual([...ROLE_NAMES, ...VARIABLE_ONLY].sort()); }); - it("matches DARK_THEME's value for every name it declares", () => { - for (const [name, value] of dark) { - expect(DARK_THEME[name], `DARK_THEME is missing ${name}`).toBe(value); - } + it("agrees with DARK_THEME declaration for declaration", () => { + expect(Object.fromEntries(dark)).toEqual({ ...DARK_THEME }); }); +}); + +describe("@theme", () => { + const inline = rawDeclarations(blockBody(STYLESHEET, "@theme inline {")); - it("names no variable DARK_THEME does not also carry", () => { - expect(Object.keys(DARK_THEME).sort()).toEqual([...dark.keys()].sort()); + it("closes Tailwind's default palette before declaring its own", () => { + const reset = STYLESHEET.indexOf("--color-*: initial;"); + expect(reset, "styles.css does not reset --color-*").toBeGreaterThan(-1); + expect(reset).toBeLessThan(STYLESHEET.indexOf("@theme inline {")); }); - it("pins the neutral chart palette exactly, unchanged from light", () => { - for (const [name, value] of Object.entries(NEUTRAL_CHART)) { - expect(dark.get(name)).toBe(value); + it("exposes --color-: var(--) for every role", () => { + for (const name of ROLE_NAMES) { + expect(inline.get(`--color-${name}`), `missing --color-${name}`).toBe(`var(--${name})`); } }); -}); - -describe("@theme inline", () => { - const inline = rawDeclarations(blockBody(STYLESHEET, "@theme inline {")); - it("exposes --color-: var(--) for every semantic and extension name", () => { - for (const name of [...SEMANTIC_NAMES, ...EXTENSION_NAMES]) { - expect(inline.get(`--color-${name}`), `@theme inline is missing --color-${name}`).toBe( - `var(--${name})`, - ); - } + it("exposes no utility for the brand or for anything outside the roles", () => { + const colours = [...inline.keys()].filter((key) => key.startsWith("--color-")); + expect(colours.sort()).toEqual(ROLE_NAMES.map((name) => `--color-${name}`).sort()); }); it("declares the two font variables from THEME", () => { @@ -194,7 +155,7 @@ describe("@theme inline", () => { expect(inline.get("--font-heading")).toBe(normalize(THEME.fontHeading)); }); - it("derives all seven radius steps from --radius, verbatim", () => { + it("derives every radius step from --radius", () => { expect(inline.get("--radius-sm")).toBe("calc(var(--radius) * 0.6)"); expect(inline.get("--radius-md")).toBe("calc(var(--radius) * 0.8)"); expect(inline.get("--radius-lg")).toBe("var(--radius)"); @@ -205,81 +166,23 @@ describe("@theme inline", () => { }); }); -/** - * The four `--text-*` custom properties this rewrite retired (Task 2 moved - * their consumers onto Tailwind's standard scale: xs/sm/base/2xl). Assembled - * from fragments — the same trick `src/gates/tokens.test.ts`'s `HEX` uses — so - * this guard's own source never spells any retired name as a - * contiguous string and cannot trip the repo-wide sweep that proves the - * migration complete everywhere else. - */ -const RETIRED_TEXT_SCALE_SUFFIXES = ["meta", "body", "section", "page"]; -const RETIRED_TEXT_SCALE = RETIRED_TEXT_SCALE_SUFFIXES.map((name) => `--text-${name}`); - -describe("legacy tokens", () => { - it("removes every v1 name this rewrite retired from the whole stylesheet", () => { - const retired = [ - "--color-primary-hover", - "--color-disabled", - "--color-disabled-foreground", - "--color-success-hover", - "--color-destructive-foreground", - "--color-sidebar-strong", - "--color-sidebar-muted", - ...RETIRED_TEXT_SCALE, - "--spacing-sidebar-mobile", - ]; - const present = retired.filter((name) => STYLESHEET.includes(name)); - expect(present, `styles.css still names: ${present.join(", ")}`).toEqual([]); - }); -}); - -describe("structure", () => { - it("declares the dark variant and the four imports, in order", () => { - const anchors = [ - '@import "tailwindcss";', - '@import "tw-animate-css";', - // The variant and utility layer, ahead of the token blocks: a variant - // has to exist before a utility can qualify on it. - '@import "./tailwind.css";', - // One family, so one font import: the heading face resolves to the body's - // rather than naming a second one. - '@import "@fontsource-variable/geist";', - "@custom-variant dark (&:is(.dark *));", - ]; - let cursor = -1; - for (const anchor of anchors) { - const at = STYLESHEET.indexOf(anchor); - expect(at, `styles.css is missing ${JSON.stringify(anchor)}`).toBeGreaterThan(-1); - expect(at, `${JSON.stringify(anchor)} is out of order`).toBeGreaterThan(cursor); - cursor = at; - } - }); - - it("reaches its own package's classes: @source after the imports", () => { - const sourceAt = STYLESHEET.indexOf('@source "..";'); - const lastImportAt = STYLESHEET.lastIndexOf("@import"); - expect(sourceAt, 'styles.css has no @source "..";').toBeGreaterThan(-1); - expect(sourceAt).toBeGreaterThan(lastImportAt); +describe("base layer", () => { + it("keys dark mode off the .dark class", () => { + expect(STYLESHEET).toContain("@custom-variant dark (&:is(.dark *));"); }); - it("applies the base-layer ring/border rule to every element", () => { + it("names the border and outline colour on every element", () => { expect(/\*\s*\{\s*@apply border-border outline-ring\/50;\s*\}/.test(STYLESHEET)).toBe(true); }); /** - * The `*` rule above names the outline *colour* and nothing else about focus. - * The geometry belongs to the components: every focusable one carries its own - * `focus-visible:ring-3 focus-visible:ring-ring/50` (the tab bar adds a 1px - * `outline-ring` on top of it), so a stylesheet-level `:focus-visible` - * override would now fight the components instead of backing them up. - * - * Asserted as an absence, because the absence is what the migration bought: - * re-adding a blanket rule here is how a single global declaration would - * quietly start overriding thirteen components again. + * Focus geometry belongs to the components. A stylesheet-level + * `:focus-visible` rule would override every component's ring at once, which + * is how thirteen rings were once lost together. */ - it("leaves focus geometry to the components: no stylesheet-level :focus-visible rule", () => { - expect(STYLESHEET).not.toContain(":focus-visible"); + it("declares no focus geometry", () => { + const rules = STYLESHEET.replace(/\/\*[\s\S]*?\*\//g, ""); + expect(rules).not.toContain(":focus-visible"); }); it("applies the heading font at the semantic-HTML level", () => { From 6f3862a61ae8ce6ec12479fec00d1f94356383da Mon Sep 17 00:00:00 2001 From: YaelAnaya Date: Tue, 8 Sep 2026 19:05:49 -0700 Subject: [PATCH 03/19] refactor(theme): use each library's own state attributes; delete tailwind.css Radix emits data-state and data-orientation, Base UI emits bare data-open, data-closed and data-highlighted, and Tailwind's built-in data-* variant already matches the bare form. So the Radix components spell data-[state=open] and data-[orientation=horizontal], the combobox keeps data-open, and the custom variant layer that papered over the difference is gone with the scroll-fade, shimmer and accordion utilities nothing here used. The one custom utility a component did use, no-scrollbar, is inlined at its single call site. --- src/components/combobox.tsx | 2 +- src/components/dialog.tsx | 4 +- src/components/dropdown-menu.tsx | 6 +- src/components/field.tsx | 2 +- src/components/select.tsx | 2 +- src/components/separator.tsx | 2 +- src/components/sheet.tsx | 4 +- src/components/tabs.tsx | 12 +- src/components/tooltip.tsx | 2 +- src/theme/styles.css | 1 - src/theme/tailwind.css | 646 ------------------------------- tests/theme/imports.test.ts | 33 -- 12 files changed, 18 insertions(+), 698 deletions(-) delete mode 100644 src/theme/tailwind.css diff --git a/src/components/combobox.tsx b/src/components/combobox.tsx index fe2790a..88669bf 100644 --- a/src/components/combobox.tsx +++ b/src/components/combobox.tsx @@ -116,7 +116,7 @@ function ComboboxList({ className, ...props }: ComboboxPrimitive.List.Props) { ) {

a]:underline [&>a]:underline-offset-4 [&>a:hover]:text-primary", className, diff --git a/src/components/select.tsx b/src/components/select.tsx index c188fae..9a7c4c9 100644 --- a/src/components/select.tsx +++ b/src/components/select.tsx @@ -68,7 +68,7 @@ function SelectContent({ data-slot="select-content" data-align-trigger={position === "item-aligned"} className={cn( - "relative z-50 max-h-(--radix-select-content-available-height) min-w-36 origin-(--radix-select-content-transform-origin) overflow-x-hidden overflow-y-auto rounded-lg bg-popover text-popover-foreground shadow-md ring-1 ring-foreground/10 duration-100 data-[align-trigger=true]:animate-none data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2 data-open:animate-in data-open:fade-in-0 data-open:zoom-in-95 data-closed:animate-out data-closed:fade-out-0 data-closed:zoom-out-95", + "relative z-50 max-h-(--radix-select-content-available-height) min-w-36 origin-(--radix-select-content-transform-origin) overflow-x-hidden overflow-y-auto rounded-lg bg-popover text-popover-foreground shadow-md ring-1 ring-foreground/10 duration-100 data-[align-trigger=true]:animate-none data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2 data-[state=open]:animate-in data-[state=open]:fade-in-0 data-[state=open]:zoom-in-95 data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=closed]:zoom-out-95", position === "popper" && "data-[side=bottom]:translate-y-1 data-[side=left]:-translate-x-1 data-[side=right]:translate-x-1 data-[side=top]:-translate-y-1", className, diff --git a/src/components/separator.tsx b/src/components/separator.tsx index 5d1dce2..b9229d6 100644 --- a/src/components/separator.tsx +++ b/src/components/separator.tsx @@ -17,7 +17,7 @@ function Separator({ decorative={decorative} orientation={orientation} className={cn( - "shrink-0 bg-border data-horizontal:h-px data-horizontal:w-full data-vertical:w-px data-vertical:self-stretch", + "shrink-0 bg-border data-[orientation=horizontal]:h-px data-[orientation=horizontal]:w-full data-[orientation=vertical]:w-px data-[orientation=vertical]:self-stretch", className, )} {...props} diff --git a/src/components/sheet.tsx b/src/components/sheet.tsx index 97624f1..2171456 100644 --- a/src/components/sheet.tsx +++ b/src/components/sheet.tsx @@ -29,7 +29,7 @@ function SheetOverlay({ ); } const tabsListVariants = cva( - "group/tabs-list inline-flex w-fit items-center justify-center rounded-lg p-[3px] text-muted-foreground group-data-horizontal/tabs:h-8 group-data-vertical/tabs:h-fit group-data-vertical/tabs:flex-col data-[variant=line]:rounded-none", + "group/tabs-list inline-flex w-fit items-center justify-center rounded-lg p-[3px] text-muted-foreground group-data-[orientation=horizontal]/tabs:h-8 group-data-[orientation=vertical]/tabs:h-fit group-data-[orientation=vertical]/tabs:flex-col data-[variant=line]:rounded-none", { variants: { variant: { @@ -54,10 +54,10 @@ function TabsTrigger({ className, ...props }: React.ComponentProps"; - inherits: false; - initial-value: 0px; -} -@property --scroll-fade-b { - syntax: ""; - inherits: false; - initial-value: 0px; -} -@property --scroll-fade-s { - syntax: ""; - inherits: false; - initial-value: 0px; -} -@property --scroll-fade-e { - syntax: ""; - inherits: false; - initial-value: 0px; -} -@property --scroll-fade-mask { - syntax: "*"; - inherits: false; -} - -@theme inline { - @keyframes scroll-fade-reveal-t { - from { - --scroll-fade-t: 0px; - } - to { - --scroll-fade-t: var( - --_scroll-fade-size-t, - var(--scroll-fade-size, min(12%, calc(var(--spacing) * 10))) - ); - } - } - @keyframes scroll-fade-reveal-b { - from { - --scroll-fade-b: var( - --_scroll-fade-size-b, - var(--scroll-fade-size, min(12%, calc(var(--spacing) * 10))) - ); - } - to { - --scroll-fade-b: 0px; - } - } - @keyframes scroll-fade-reveal-s { - from { - --scroll-fade-s: 0px; - } - to { - --scroll-fade-s: var( - --_scroll-fade-size-s, - var(--scroll-fade-size, min(12%, calc(var(--spacing) * 10))) - ); - } - } - @keyframes scroll-fade-reveal-e { - from { - --scroll-fade-e: var( - --_scroll-fade-size-e, - var(--scroll-fade-size, min(12%, calc(var(--spacing) * 10))) - ); - } - to { - --scroll-fade-e: 0px; - } - } -} - -@utility scroll-fade { - --_scroll-fade-size-t: var( - --scroll-fade-t-size, - var(--scroll-fade-size, min(12%, calc(var(--spacing) * 10))) - ); - --_scroll-fade-size-b: var( - --scroll-fade-b-size, - var(--scroll-fade-size, min(12%, calc(var(--spacing) * 10))) - ); - --scroll-fade-block: linear-gradient( - to bottom, - transparent 0, - #000 var(--scroll-fade-t, 0px), - #000 calc(100% - var(--scroll-fade-b, 0px)), - transparent 100% - ); - -webkit-mask-image: var(--scroll-fade-mask, var(--scroll-fade-block)); - mask-image: var(--scroll-fade-mask, var(--scroll-fade-block)); - -webkit-mask-composite: source-in; - mask-composite: intersect; - -webkit-mask-repeat: no-repeat; - mask-repeat: no-repeat; - - @supports (animation-timeline: scroll()) { - animation: - scroll-fade-reveal-t 1ms ease-in-out, - scroll-fade-reveal-b 1ms ease-in-out; - animation-timeline: scroll(self y), scroll(self y); - animation-range: - 0 var(--scroll-fade-reveal, calc(var(--spacing) * 24)), - calc(100% - var(--scroll-fade-reveal, calc(var(--spacing) * 24))) 100%; - animation-fill-mode: both; - } - - @supports not (animation-timeline: scroll()) { - --scroll-fade-t: var(--_scroll-fade-size-t); - --scroll-fade-b: var(--_scroll-fade-size-b); - } -} - -@utility scroll-fade-y { - --_scroll-fade-size-t: var( - --scroll-fade-t-size, - var(--scroll-fade-size, min(12%, calc(var(--spacing) * 10))) - ); - --_scroll-fade-size-b: var( - --scroll-fade-b-size, - var(--scroll-fade-size, min(12%, calc(var(--spacing) * 10))) - ); - --scroll-fade-block: linear-gradient( - to bottom, - transparent 0, - #000 var(--scroll-fade-t, 0px), - #000 calc(100% - var(--scroll-fade-b, 0px)), - transparent 100% - ); - -webkit-mask-image: var(--scroll-fade-mask, var(--scroll-fade-block)); - mask-image: var(--scroll-fade-mask, var(--scroll-fade-block)); - -webkit-mask-composite: source-in; - mask-composite: intersect; - -webkit-mask-repeat: no-repeat; - mask-repeat: no-repeat; - - @supports (animation-timeline: scroll()) { - animation: - scroll-fade-reveal-t 1ms ease-in-out, - scroll-fade-reveal-b 1ms ease-in-out; - animation-timeline: scroll(self y), scroll(self y); - animation-range: - 0 var(--scroll-fade-reveal, calc(var(--spacing) * 24)), - calc(100% - var(--scroll-fade-reveal, calc(var(--spacing) * 24))) 100%; - animation-fill-mode: both; - } - - @supports not (animation-timeline: scroll()) { - --scroll-fade-t: var(--_scroll-fade-size-t); - --scroll-fade-b: var(--_scroll-fade-size-b); - } -} - -@utility scroll-fade-x { - --_scroll-fade-size-s: var( - --scroll-fade-s-size, - var(--scroll-fade-size, min(12%, calc(var(--spacing) * 10))) - ); - --_scroll-fade-size-e: var( - --scroll-fade-e-size, - var(--scroll-fade-size, min(12%, calc(var(--spacing) * 10))) - ); - --scroll-fade-inline: linear-gradient( - to right, - transparent 0, - #000 var(--scroll-fade-s, 0px), - #000 calc(100% - var(--scroll-fade-e, 0px)), - transparent 100% - ); - &:where([dir="rtl"], [dir="rtl"] *) { - --scroll-fade-inline: linear-gradient( - to left, - transparent 0, - #000 var(--scroll-fade-s, 0px), - #000 calc(100% - var(--scroll-fade-e, 0px)), - transparent 100% - ); - } - -webkit-mask-image: var(--scroll-fade-mask, var(--scroll-fade-inline)); - mask-image: var(--scroll-fade-mask, var(--scroll-fade-inline)); - -webkit-mask-composite: source-in; - mask-composite: intersect; - -webkit-mask-repeat: no-repeat; - mask-repeat: no-repeat; - - @supports (animation-timeline: scroll()) { - animation: - scroll-fade-reveal-s 1ms ease-in-out, - scroll-fade-reveal-e 1ms ease-in-out; - animation-timeline: scroll(self inline), scroll(self inline); - animation-range: - 0 var(--scroll-fade-reveal, calc(var(--spacing) * 24)), - calc(100% - var(--scroll-fade-reveal, calc(var(--spacing) * 24))) 100%; - animation-fill-mode: both; - } - - @supports not (animation-timeline: scroll()) { - --scroll-fade-s: var(--_scroll-fade-size-s); - --scroll-fade-e: var(--_scroll-fade-size-e); - } -} - -@utility scroll-fade-t { - --_scroll-fade-size-t: var( - --scroll-fade-t-size, - var(--scroll-fade-size, min(12%, calc(var(--spacing) * 10))) - ); - --scroll-fade-mask: linear-gradient( - to bottom, - transparent 0, - #000 var(--scroll-fade-t, 0px), - #000 100% - ); - -webkit-mask-image: var(--scroll-fade-mask); - mask-image: var(--scroll-fade-mask); - -webkit-mask-composite: source-in; - mask-composite: intersect; - -webkit-mask-repeat: no-repeat; - mask-repeat: no-repeat; - - @supports (animation-timeline: scroll()) { - animation: scroll-fade-reveal-t 1ms ease-in-out; - animation-timeline: scroll(self y); - animation-range: 0 var(--scroll-fade-reveal, calc(var(--spacing) * 24)); - animation-fill-mode: both; - } - - @supports not (animation-timeline: scroll()) { - --scroll-fade-t: var(--_scroll-fade-size-t); - } -} - -@utility scroll-fade-b { - --_scroll-fade-size-b: var( - --scroll-fade-b-size, - var(--scroll-fade-size, min(12%, calc(var(--spacing) * 10))) - ); - --scroll-fade-mask: linear-gradient( - to bottom, - #000 0, - #000 calc(100% - var(--scroll-fade-b, 0px)), - transparent 100% - ); - -webkit-mask-image: var(--scroll-fade-mask); - mask-image: var(--scroll-fade-mask); - -webkit-mask-composite: source-in; - mask-composite: intersect; - -webkit-mask-repeat: no-repeat; - mask-repeat: no-repeat; - - @supports (animation-timeline: scroll()) { - animation: scroll-fade-reveal-b 1ms ease-in-out; - animation-timeline: scroll(self y); - animation-range: calc(100% - var(--scroll-fade-reveal, calc(var(--spacing) * 24))) 100%; - animation-fill-mode: both; - } - - @supports not (animation-timeline: scroll()) { - --scroll-fade-b: var(--_scroll-fade-size-b); - } -} - -@utility scroll-fade-l { - --_scroll-fade-size-s: var( - --scroll-fade-s-size, - var(--scroll-fade-size, min(12%, calc(var(--spacing) * 10))) - ); - --scroll-fade-mask: linear-gradient( - to right, - transparent 0, - #000 var(--scroll-fade-s, 0px), - #000 100% - ); - -webkit-mask-image: var(--scroll-fade-mask); - mask-image: var(--scroll-fade-mask); - -webkit-mask-composite: source-in; - mask-composite: intersect; - -webkit-mask-repeat: no-repeat; - mask-repeat: no-repeat; - - @supports (animation-timeline: scroll()) { - animation: scroll-fade-reveal-s 1ms ease-in-out; - animation-timeline: scroll(self x); - animation-range: 0 var(--scroll-fade-reveal, calc(var(--spacing) * 24)); - animation-fill-mode: both; - } - - @supports not (animation-timeline: scroll()) { - --scroll-fade-s: var(--_scroll-fade-size-s); - } -} - -@utility scroll-fade-r { - --_scroll-fade-size-e: var( - --scroll-fade-e-size, - var(--scroll-fade-size, min(12%, calc(var(--spacing) * 10))) - ); - --scroll-fade-mask: linear-gradient( - to right, - #000 0, - #000 calc(100% - var(--scroll-fade-e, 0px)), - transparent 100% - ); - -webkit-mask-image: var(--scroll-fade-mask); - mask-image: var(--scroll-fade-mask); - -webkit-mask-composite: source-in; - mask-composite: intersect; - -webkit-mask-repeat: no-repeat; - mask-repeat: no-repeat; - - @supports (animation-timeline: scroll()) { - animation: scroll-fade-reveal-e 1ms ease-in-out; - animation-timeline: scroll(self x); - animation-range: calc(100% - var(--scroll-fade-reveal, calc(var(--spacing) * 24))) 100%; - animation-fill-mode: both; - } - - @supports not (animation-timeline: scroll()) { - --scroll-fade-e: var(--_scroll-fade-size-e); - } -} - -@utility scroll-fade-s { - --_scroll-fade-size-s: var( - --scroll-fade-s-size, - var(--scroll-fade-size, min(12%, calc(var(--spacing) * 10))) - ); - --scroll-fade-mask: linear-gradient( - to right, - transparent 0, - #000 var(--scroll-fade-s, 0px), - #000 100% - ); - &:where([dir="rtl"], [dir="rtl"] *) { - --scroll-fade-mask: linear-gradient( - to left, - transparent 0, - #000 var(--scroll-fade-s, 0px), - #000 100% - ); - } - -webkit-mask-image: var(--scroll-fade-mask); - mask-image: var(--scroll-fade-mask); - -webkit-mask-composite: source-in; - mask-composite: intersect; - -webkit-mask-repeat: no-repeat; - mask-repeat: no-repeat; - - @supports (animation-timeline: scroll()) { - animation: scroll-fade-reveal-s 1ms ease-in-out; - animation-timeline: scroll(self inline); - animation-range: 0 var(--scroll-fade-reveal, calc(var(--spacing) * 24)); - animation-fill-mode: both; - } - - @supports not (animation-timeline: scroll()) { - --scroll-fade-s: var(--_scroll-fade-size-s); - } -} - -@utility scroll-fade-e { - --_scroll-fade-size-e: var( - --scroll-fade-e-size, - var(--scroll-fade-size, min(12%, calc(var(--spacing) * 10))) - ); - --scroll-fade-mask: linear-gradient( - to right, - #000 0, - #000 calc(100% - var(--scroll-fade-e, 0px)), - transparent 100% - ); - &:where([dir="rtl"], [dir="rtl"] *) { - --scroll-fade-mask: linear-gradient( - to left, - #000 0, - #000 calc(100% - var(--scroll-fade-e, 0px)), - transparent 100% - ); - } - -webkit-mask-image: var(--scroll-fade-mask); - mask-image: var(--scroll-fade-mask); - -webkit-mask-composite: source-in; - mask-composite: intersect; - -webkit-mask-repeat: no-repeat; - mask-repeat: no-repeat; - - @supports (animation-timeline: scroll()) { - animation: scroll-fade-reveal-e 1ms ease-in-out; - animation-timeline: scroll(self inline); - animation-range: calc(100% - var(--scroll-fade-reveal, calc(var(--spacing) * 24))) 100%; - animation-fill-mode: both; - } - - @supports not (animation-timeline: scroll()) { - --scroll-fade-e: var(--_scroll-fade-size-e); - } -} - -@utility scroll-fade-* { - --scroll-fade-size: calc(var(--spacing) * --value(integer)); - --scroll-fade-size: --value([length], [percentage]); -} - -@utility scroll-fade-t-* { - --scroll-fade-t-size: calc(var(--spacing) * --value(integer)); - --scroll-fade-t-size: --value([length], [percentage]); -} - -@utility scroll-fade-b-* { - --scroll-fade-b-size: calc(var(--spacing) * --value(integer)); - --scroll-fade-b-size: --value([length], [percentage]); -} - -@utility scroll-fade-s-* { - --scroll-fade-s-size: calc(var(--spacing) * --value(integer)); - --scroll-fade-s-size: --value([length], [percentage]); -} - -@utility scroll-fade-e-* { - --scroll-fade-e-size: calc(var(--spacing) * --value(integer)); - --scroll-fade-e-size: --value([length], [percentage]); -} - -@utility scroll-fade-none { - --scroll-fade-mask: none; -} - -/* shimmer */ -@property --shimmer-angle { - syntax: ""; - inherits: true; - initial-value: 20deg; -} -@property --shimmer-image { - syntax: "*"; - inherits: false; -} -@property --shimmer-text-fill { - syntax: "*"; - inherits: false; -} - -@theme inline { - @keyframes tw-shimmer { - from { - background-position: 100% 0; - } - to { - background-position: 0 0; - } - } -} - -@utility shimmer { - --_spread: var(--shimmer-spread, calc(3ch + 40px)); - --_base: currentColor; - --_highlight: var(--shimmer-color, oklch(from currentColor l c h / calc(alpha* 0.2))); - - background-image: var( - --shimmer-image, - linear-gradient( - calc(90deg + var(--shimmer-angle)), - var(--_base) calc(50% - var(--_spread)), - color-mix(in oklch, var(--_highlight), var(--_base) 50%) calc(50% - var(--_spread) * 0.5), - var(--_highlight) 50%, - color-mix(in oklch, var(--_highlight), var(--_base) 50%) calc(50% + var(--_spread) * 0.5), - var(--_base) calc(50% + var(--_spread)) - ) - ); - background-repeat: no-repeat; - background-size: calc(200% + var(--_spread) * 2) 100%; - background-position: 0 0; - background-clip: text; - -webkit-background-clip: text; - -webkit-text-fill-color: var(--shimmer-text-fill, transparent); - animation: tw-shimmer var(--shimmer-duration, 2s) linear infinite; - - @variant dark { - --_highlight: var( - --shimmer-color, - oklch(from currentColor max(0.8, calc(l + 0.4)) c h / calc(alpha + 0.4)) - ); - } - - &:where([dir="rtl"], [dir="rtl"] *) { - animation-direction: reverse; - } -} - -@utility shimmer-once { - animation-iteration-count: 1; -} - -@utility shimmer-reverse { - animation-direction: reverse; -} - -@utility shimmer-none { - --shimmer-image: none; - --shimmer-text-fill: currentColor; -} - -@utility shimmer-color-* { - --shimmer-color: --value(--color, [color]); - --shimmer-color: color-mix( - in oklch, - --value(--color, [color]) calc(--modifier(integer) * 1%), - transparent - ); -} - -@utility shimmer-duration-* { - --shimmer-duration: calc(--value(integer) * 1ms); -} - -@utility shimmer-spread-* { - --shimmer-spread: calc(var(--spacing) * --value(integer)); - --shimmer-spread: --value([length], [percentage]); -} - -@utility shimmer-angle-* { - --shimmer-angle: calc(--value(integer) * 1deg); -} - -@media (prefers-reduced-motion: reduce) { - .shimmer { - animation: none; - background-image: none; - -webkit-text-fill-color: currentColor; - } -} diff --git a/tests/theme/imports.test.ts b/tests/theme/imports.test.ts index 7c2151f..aa9dc51 100644 --- a/tests/theme/imports.test.ts +++ b/tests/theme/imports.test.ts @@ -24,7 +24,6 @@ * correctly installed package as missing. */ -import { spawnSync } from "node:child_process"; import { existsSync, readFileSync } from "node:fs"; import path from "node:path"; import { fileURLToPath } from "node:url"; @@ -126,35 +125,3 @@ test("every package the stylesheet imports is installed and exports the file it `CSS to catch it:\n${broken.join("\n")}`, ).toEqual([]); }); - -test("every sibling the stylesheet imports exists and is committed", () => { - const specs = importsIn(STYLESHEET).filter(isRelative); - expect(specs.length > 0, "the stylesheet named no siblings, so this proves nothing").toBe(true); - - const broken = specs.flatMap((spec) => { - const file = path.resolve(STYLESHEET_DIR, spec); - if (!existsSync(file)) { - return [`@import "${spec}": ${path.relative(REPO, file)} does not exist`]; - } - // On disk is not enough. `files: ["src"]` publishes what the working tree - // holds, so an uncommitted sibling packs fine here and is absent from the - // clone CI builds and publishes from — the failure lands on consumers only. - const tracked = spawnSync("git", ["ls-files", "--error-unmatch", file], { - cwd: REPO, - encoding: "utf8", - }); - if (tracked.status !== 0) { - return [ - `@import "${spec}": ${path.relative(REPO, file)} exists but is not tracked by git — ` + - "commit it, or the published package will not carry it", - ]; - } - return []; - }); - - expect( - broken, - "styles.css ships as source, so a sibling it imports has to reach the consumer with it:\n" + - broken.join("\n"), - ).toEqual([]); -}); From 3f745b8d1e956dd3a8d6d7ee4e1900123a57b871 Mon Sep 17 00:00:00 2001 From: YaelAnaya Date: Tue, 8 Sep 2026 19:12:19 -0700 Subject: [PATCH 04/19] fix: Button defaults to type=button; submenus leave on dismissal; behavioural tests A native + , + ); + const button = screen.getByRole("button", { name: "Cancel" }); + expect(button.getAttribute("type")).toBe("button"); + await userEvent.click(button); + expect(onSubmit).not.toHaveBeenCalled(); + }); + + it("submits when asked to", async () => { + const onSubmit = vi.fn((event: FormEvent) => event.preventDefault()); + render( +

+ +
, + ); + const button = screen.getByRole("button", { name: "Save" }); + expect(button.getAttribute("type")).toBe("submit"); + await userEvent.click(button); + expect(onSubmit).toHaveBeenCalledTimes(1); + }); + + it("renders the child element with asChild, and forces no type onto it", () => { + render( + , + ); + // A `role="link"` on a `, + ); + const button = screen.getByRole("button", { name: "Nope" }); + expect(button).toHaveProperty("disabled", true); + await userEvent.click(button); + expect(onClick).not.toHaveBeenCalled(); + }); + + it("lets a caller's className win a conflicting utility rather than emitting both", () => { + // Without a Tailwind-aware merge both `px-2.5` and `px-6` survive and which + // one wins is decided by stylesheet order — a rule nobody can see from the + // call site. This is what makes `className` an extension point. + render(); + const classes = screen.getByRole("button", { name: "Wide" }).className.split(" "); + expect(classes).toContain("px-6"); + expect(classes).not.toContain("px-2.5"); + }); + + it("lets a caller's className outrank a size's own geometry", () => { + render( + , + ); + const classes = screen.getByRole("button").className.split(" "); + expect(classes).toContain("h-8"); + expect(classes).not.toContain("h-auto"); + }); + + it("marks its variant and size as data, so a parent can style by decision rather than by colour", () => { + render( + , + ); + const button = screen.getByRole("button", { name: "Read the docs" }); + expect(button.getAttribute("data-variant")).toBe("link"); + expect(button.getAttribute("data-size")).toBe("inline"); + }); +}); diff --git a/tests/components/components.test.tsx b/tests/components/components.test.tsx index 1a5135d..0535a11 100644 --- a/tests/components/components.test.tsx +++ b/tests/components/components.test.tsx @@ -1,16 +1,11 @@ /** - * The component harness, proved on the components that carry a decision. + * The smaller components, on the behaviour a screen would silently lose. * * Deliberately not an echo of every class string: pinning the design system to - * whatever it looked like on the day is the mistake this repository just spent - * a restructure undoing, and a restyle is coming. What is asserted here is the - * handful of behaviours a screen would silently lose — the merge that makes - * `className` a real override, the `asChild` that keeps a link a link, the role - * an error is announced with, and the value a `Progress` reports without being - * asked. - * - * The button does not default `type`, so nothing here stops a "Cancel" - * submitting a form; that is a call-site property, gated in the consumer. + * whatever it looked like on the day is the mistake a restyle would pay for. + * What is asserted is roles, `aria-*`, the `data-*` a consumer styles against, + * and the handful of layout facts a caller depends on. Button, Dialog, Field + * and DropdownMenu have their own files. */ import { render, screen } from "@testing-library/react"; @@ -20,19 +15,8 @@ import { describe, expect, it } from "vitest"; import { Alert, AlertDescription, AlertTitle } from "../../src/components/alert"; import { Badge } from "../../src/components/badge"; -import { Button } from "../../src/components/button"; import { Card, CardTitle } from "../../src/components/card"; -import { Dialog, DialogContent, DialogDescription, DialogTitle } from "../../src/components/dialog"; -import { - DropdownMenu, - DropdownMenuContent, - DropdownMenuItem, - DropdownMenuTrigger, -} from "../../src/components/dropdown-menu"; import { Progress } from "../../src/components/progress"; -import { FieldError } from "../../src/components/field"; -import { Input } from "../../src/components/input"; -import { Label } from "../../src/components/label"; import { Select, SelectContent, @@ -40,87 +24,9 @@ import { SelectTrigger, SelectValue, } from "../../src/components/select"; -import { - Sheet, - SheetContent, - SheetDescription, - SheetHeader, - SheetTitle, -} from "../../src/components/sheet"; import { Table, TableBody, TableHead, TableHeader, TableRow } from "../../src/components/table"; import { Tabs, TabsContent, TabsList, TabsTrigger } from "../../src/components/tabs"; -describe("Button", () => { - it("keeps an explicit type", () => { - render(); - expect(screen.getByRole("button").getAttribute("type")).toBe("submit"); - }); - - it("lets a caller override a conflicting utility rather than emitting both", () => { - // Without `tailwind-merge` both `px-2.5` and `px-6` survive and which one wins is - // decided by the order Tailwind wrote them into the stylesheet — a rule nobody - // can see from the call site. This is what makes `className` an extension - // point rather than a suggestion. - render(); - const className = screen.getByRole("button", { name: "Wide" }).className; - expect(className).toContain("px-6"); - expect(className).not.toContain("px-2.5"); - }); - - it("renders the child element with asChild, so a link stays a link", () => { - render( - , - ); - // A `role="link"` on a `); - - const classes = screen.getByRole("button").className; - // The underline arrives on hover. A rule that underlined at rest would look - // like a link in a screenshot and read as one to `toContain("underline")`, - // which is why the resting state is asserted as an absence and the utility - // is matched whole — `hover:underline` and `underline-offset-4` both contain - // the substring. - expect(classes).toContain("hover:underline"); - expect(classes).not.toMatch(/(^|\s)underline(\s|$)/); - }); - - it("lets a caller's className outrank the inline size", () => { - render( - , - ); - - // `h-auto` and `h-8` are the same utility group, so tailwind-merge has to - // drop the one the variant supplied. Without the merge both survive and - // which wins is decided by stylesheet order — a rule the call site cannot see. - const classes = screen.getByRole("button").className.split(" "); - expect(classes).toContain("h-8"); - expect(classes).not.toContain("h-auto"); - }); - - it("has an inline size that keeps a link button inside a sentence", () => { - render( - , - ); - const button = screen.getByRole("button", { name: "Read the docs" }); - expect(button.getAttribute("data-size")).toBe("inline"); - expect(button.className).toContain("h-auto"); - expect(button.className).toContain("p-0"); - }); -}); - describe("Alert and Badge", () => { it("announces an alert, and composes its title and description", () => { render( @@ -141,13 +47,11 @@ describe("Alert and Badge", () => { }); it.each(["success", "warning", "info", "quiet"] as const)( - "%s is a Badge variant keyed on data", + "%s is a soft surface on a semantic role, never a coloured stroke", (variant) => { render(x); const el = screen.getByText("x"); expect(el.getAttribute("data-variant")).toBe(variant); - // A status chip is a soft surface and readable ink on a semantic role — - // never a coloured stroke, never a physical palette family. expect(el.className).toContain("border-transparent"); expect(el.className).toMatch(/\bbg-(success|warning|info)\/10\b|\bbg-muted\b/); expect(el.className).not.toMatch(/emerald|amber|sky|border-(success|warning|info)/); @@ -155,33 +59,11 @@ describe("Alert and Badge", () => { ); }); -describe("fields", () => { - it("associates a label with its control by id", () => { - render( - <> - - - , - ); - expect(screen.getByLabelText("Tag")).toHaveProperty("value", "v1"); - }); - - it("announces a field error", () => { - render(must not be blank); - expect(screen.getByRole("alert").textContent).toBe("must not be blank"); - }); -}); - /** - * The two-line option — a `multiline` prop on the trigger itself rather than a - * class string composed at the call site, which is why what is asserted here - * is the trigger's own behaviour and not a layout this file would otherwise - * have to keep in step with a call site. - * - * The claim worth a test is the one that is easy to lose: the trigger shows the - * *same* two lines the list does, because Radix renders the selected item's own - * children into it. A second copy of the layout at the call site would look - * identical the day it was written and drift the day either half moved. + * The two-line option — a `multiline` prop on the trigger rather than a class + * string composed at the call site. The claim worth a test is the one that is + * easy to lose: the trigger shows the *same* two lines the list does, because + * Radix renders the selected item's own children into it. */ describe("Select", () => { function pickOne(): JSX.Element { @@ -208,54 +90,28 @@ describe("Select", () => { const trigger = screen.getByTestId("model"); expect(trigger.textContent).toContain("org/model-tiny"); expect(trigger.textContent).toContain("311.9 MB · tiny"); - // Two elements, not one line that happens to wrap — the meta carries the - // muted role and the id does not. const meta = trigger.querySelector(".text-muted-foreground"); expect(meta?.textContent).toBe("311.9 MB · tiny"); - expect(meta?.textContent).not.toContain("org/model-tiny"); }); - it("grows rather than clipping, and leaves a one-line option where it was", () => { + it("grows rather than clipping when multiline, and clamps otherwise", () => { render(pickOne()); - // `h-8` would fix the height and squash the second line; `min-h-8` keeps a - // one-line control at the trigger's usual height and lets a two-line one grow. const trigger = screen.getByTestId("model"); + // `min-h-8` keeps a one-line control at the usual height and lets a + // two-line one grow; the merge has to *replace* the fixed height and the + // value clamp rather than stack beside them. expect(trigger.className).toContain("min-h-8"); - // Nothing truncates: half a model id is not a model id. - expect(trigger.className).not.toContain("truncate"); - // `multiline` is a prop, not a class string arriving from outside, so it - // does not need to outrank anything — but `cn`'s merge still has to replace - // the fixed height and the value clamp rather than stack beside them, which - // is what this checks at render, not an echo of the prop's own classes. - expect(trigger.className).toContain("data-[size=default]:h-auto"); expect(trigger.className).toContain("line-clamp-none"); expect(trigger.className).not.toContain("data-[size=default]:h-8"); expect(trigger.className).not.toContain("line-clamp-1"); }); - it("floors the open list at the closed control's width", async () => { + it("opens to a listbox floored at the closed control's width", async () => { render(pickOne()); await userEvent.click(screen.getByTestId("model")); - const viewport = document.querySelector("[data-radix-select-viewport]"); - expect(viewport).not.toBeNull(); - const className = (viewport as HTMLElement).className; - expect(className).toContain("w-full"); - expect(className).toContain("min-w-(--radix-select-trigger-width)"); - }); - - it("grows and stops clamping its value when multiline", () => { - render( - , - ); - const trigger = screen.getByRole("combobox", { name: "Export target" }); - expect(trigger.className).toContain("data-[size=default]:h-auto"); - expect(trigger.className).not.toContain("data-[size=default]:h-8"); - expect(trigger.className).toContain("line-clamp-none"); - expect(trigger.className).not.toContain("line-clamp-1"); + expect(await screen.findByRole("listbox")).toBeTruthy(); + const viewport = document.querySelector("[data-radix-select-viewport]") as HTMLElement; + expect(viewport.className).toContain("min-w-(--radix-select-trigger-width)"); }); }); @@ -287,17 +143,9 @@ describe("Card and Table", () => { }); /** - * The tab bar, asserted on what it *means* rather than on what it looks like. - * - * The distinction between the open section and the other two has to survive a - * restyling, so nothing here matches a class string — a test that pinned - * `bg-card` would have failed on the very change it was supposed to protect, and - * a test that pinned `border-primary` would fail on the next one. What is asserted - * is the part a screen reader and the keyboard both read: the roles, `aria-selected`, - * Radix's `data-state`, and that only the open panel is in the tree at all. - * - * There is nothing here about a variant cascade: `TabsList` has one shape and no - * context to hand down. + * The tab bar, asserted on what it means rather than what it looks like: the + * roles, `aria-selected`, Radix's `data-state`, and that only the open panel is + * in the tree at all. */ describe("Tabs", () => { function bar(): JSX.Element { @@ -315,7 +163,6 @@ describe("Tabs", () => { it("marks the open section as the selected tab and the others as not", () => { render(bar()); - const [schema, batches] = screen.getAllByRole("tab"); expect(schema.getAttribute("aria-selected")).toBe("true"); expect(schema.dataset.state).toBe("active"); @@ -323,30 +170,25 @@ describe("Tabs", () => { expect(batches.dataset.state).toBe("inactive"); }); - it("moves the selection when the tab is clicked, so the state is the source of the styling", async () => { + it("moves the selection when a tab is clicked", async () => { render(bar()); - await userEvent.click(screen.getByRole("tab", { name: "Batches" })); expect(screen.getByRole("tab", { name: "Batches" }).getAttribute("aria-selected")).toBe("true"); expect(screen.getByRole("tab", { name: "Schema" }).getAttribute("aria-selected")).toBe("false"); }); - it("keeps only the open panel in the tree, and labels it with its tab", () => { + it("keeps only the open panel in the tree, and labels the list", () => { render(bar()); - const panels = screen.getAllByRole("tabpanel"); expect(panels).toHaveLength(1); expect(panels[0]?.textContent).toBe("the classes"); expect(screen.getByRole("tablist").getAttribute("aria-label")).toBe("Sections"); }); - it("is operable from the keyboard, because every trigger is a real button", async () => { + it("is operable from the keyboard: one tab stop, arrows move within it", async () => { render(bar()); - - // Radix's roving tabindex: one stop for the whole bar, arrows move within it. await userEvent.tab(); expect(document.activeElement).toBe(screen.getByRole("tab", { name: "Schema" })); - await userEvent.keyboard("{ArrowRight}"); expect(document.activeElement).toBe(screen.getByRole("tab", { name: "Batches" })); expect(screen.getByRole("tab", { name: "Batches" }).getAttribute("aria-selected")).toBe("true"); @@ -358,18 +200,17 @@ describe("Progress", () => { render(); const bar = screen.getByRole("progressbar", { name: "Ingest" }); expect(bar.getAttribute("aria-valuenow")).toBe("42"); + expect(bar.getAttribute("aria-valuemax")).toBe("100"); expect(bar.getAttribute("data-state")).not.toBe("indeterminate"); }); - it("fills with the functional colour, and carries the data-slot a caller can restyle from", () => { - // `Progress` has no `variant` prop and no notion of status — a - // batch's completion is an amount, not a polarity, so the indicator is - // always `bg-primary`. A caller who needs to reach it targets the - // `data-slot` it renders with, from its own `className` on `Root`. + it("fills with the functional colour and carries a data-slot a caller can restyle from", () => { + // No `variant`, no notion of status: completion is an amount, not a polarity. render(); - const fill = screen.getByRole("progressbar").firstElementChild as Element; + const fill = screen.getByRole("progressbar").firstElementChild as HTMLElement; expect(fill.getAttribute("data-slot")).toBe("progress-indicator"); - expect(fill.className).toBe("size-full flex-1 bg-primary transition-all"); + expect(fill.className).toContain("bg-primary"); + expect(fill.style.transform).toBe("translateX(-58%)"); }); it("hands its ref to the track element", () => { @@ -386,107 +227,3 @@ describe("Progress", () => { expect(track).toBe(screen.getByRole("progressbar")); }); }); - -describe("Dialog", () => { - function describedBy(dialog: HTMLElement): readonly (string | null)[] { - return (dialog.getAttribute("aria-describedby") ?? "") - .split(" ") - .filter(Boolean) - .map((id) => document.getElementById(id)?.textContent ?? null); - } - - it("points aria-describedby at every description, each under its own id, when the caller names them", () => { - render( - - - Narrowing - one class narrows - nothing becomes invalid - - , - ); - const dialog = screen.getByRole("dialog"); - expect(dialog.getAttribute("aria-describedby")).toBe("d1 d2"); - expect(describedBy(dialog)).toEqual(["one class narrows", "nothing becomes invalid"]); - expect(document.getElementById("d1")?.textContent).toBe("one class narrows"); - expect(document.getElementById("d2")?.textContent).toBe("nothing becomes invalid"); - }); - - it("wires a single, unnamed description through Radix's own id by default", () => { - render( - - - Narrowing - one class narrows - - , - ); - const dialog = screen.getByRole("dialog"); - const describedById = dialog.getAttribute("aria-describedby"); - expect(describedById).toBeTruthy(); - expect(document.getElementById(describedById ?? "")?.textContent).toBe("one class narrows"); - }); -}); - -describe("sheet", () => { - it("renders a dialog with its title and description wired by Radix", async () => { - render( - - - - Filters - Narrow the list - - - , - ); - const dialog = await screen.findByRole("dialog"); - expect(dialog.getAttribute("data-slot")).toBe("sheet-content"); - expect(dialog.getAttribute("data-side")).toBe("right"); - expect(screen.getByRole("button", { name: "Close" })).toBeTruthy(); - }); -}); - -describe("DropdownMenu", () => { - it("sizes its surface to the items, not to the trigger", async () => { - const user = userEvent.setup(); - render( - - -