diff --git a/.agents/skills/frontend/nodejs-setup/SKILL.md b/.agents/skills/frontend/nodejs-setup/SKILL.md index b6c20430..8c757377 100644 --- a/.agents/skills/frontend/nodejs-setup/SKILL.md +++ b/.agents/skills/frontend/nodejs-setup/SKILL.md @@ -64,8 +64,8 @@ pnpm add -w -D # root tooling only - `@visionset/annotator` keeps `react` as an **optional peer dependency** — never promote it to a hard dependency; the core must stay usable without React. - `@visionset/ui-core` UI primitives are **Radix behaviour + shadcn Nova styling, iconed with - Tabler**. Do not add another component library, and do not reach for `lucide-react` in new - code — it survives only in `src/annotator/`, as migration debt. + Tabler**. Do not add another component library, and do not add a second icon set: + `@tabler/icons-react` is the only one the frontend declares, and a gate refuses another. - `@visionset/app` is `private: true` — never published to npm; its bundle ships inside the Python wheel. diff --git a/DESIGN.md b/DESIGN.md index 936e5322..5b7f8e32 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -409,12 +409,11 @@ First-class, and part of every rule above rather than a section to satisfy after - **Ad-hoc geometry that fights Nova.** A control's height, padding, or radius is not a per-screen decision; reaching past the geometry table above for a bespoke size is a design decision to make in this document, not in a component diff. -- **Mixing icon sets in new code.** Tabler is the set, and across the primitives *and* - the screens it is already the only one: every icon `frontend/ui-core/src/primitives/` - and `frontend/ui-core/src/screens/` draws is `@tabler/icons-react`, and neither - directory imports `lucide-react` at all. Lucide survives in exactly one place — - `frontend/ui-core/src/annotator/` — and that is the whole of the remaining debt: a file - there still importing it is a file to migrate, not a precedent to follow. +- **Mixing icon sets in new code.** Tabler is the set, and now the only one: every icon + the primitives, the screens and the annotation workspace draw is + `@tabler/icons-react`, no package declares a second icon library, and + `tests/scripts/design_tokens.test.mjs` refuses one that reappears in a manifest or an + import. A second set is a decision to make in this document, not a dependency to add. - **Brand in a functional control.** Robomous coral is identity — the wordmark and its styleguide swatch, nothing else. A functional control reaching for `brand` is a semantic-colour violation regardless of how many other sites already use it correctly. diff --git a/docs/content/architecture/frontend/ui-core.md b/docs/content/architecture/frontend/ui-core.md index 8205df2d..f74fc618 100644 --- a/docs/content/architecture/frontend/ui-core.md +++ b/docs/content/architecture/frontend/ui-core.md @@ -106,7 +106,7 @@ visual-design rule. The current choices: | Concern | Choice | | --- | --- | | UI primitives | Radix (+ shadcn-style composition with `cva` and `cn`) - the open-code shadcn maintenance model is the direction: a primitive is VisionSet-owned source in `frontend/ui-core/src/primitives/`, edited directly, not a package dependency upgraded blindly | -| Icons | `@tabler/icons-react` is the set: the primitives and the screens draw nothing else. `lucide-react` remains only in `src/annotator/`, as migration debt | +| Icons | `@tabler/icons-react`, and nothing else: the primitives, the screens and the annotation workspace all draw from it, and no package declares a second icon library | | Styling | Tailwind v4, CSS-first `@theme`, on the shadcn preset `b3bXyyPdWj` - no `tailwind.config.js`, ever | | Toasts | sonner | | Component tests | vitest + jsdom + @testing-library/react | diff --git a/frontend/app/package.json b/frontend/app/package.json index b5dabea8..cdfa9a2b 100644 --- a/frontend/app/package.json +++ b/frontend/app/package.json @@ -20,7 +20,6 @@ "@tabler/icons-react": "^3.46.0", "@visionset/annotator": "workspace:*", "@visionset/ui-core": "workspace:*", - "lucide-react": "^1.32.0", "react": "^19.0.0", "react-dom": "^19.0.0", "react-router": "^8.3.0" diff --git a/frontend/app/src/demo/ToolStrip.tsx b/frontend/app/src/demo/ToolStrip.tsx index b3b4ded1..0a71eb0e 100644 --- a/frontend/app/src/demo/ToolStrip.tsx +++ b/frontend/app/src/demo/ToolStrip.tsx @@ -32,14 +32,12 @@ * "what a product does about a missing tool" is a product decision it is not * the job of a demo to make. * - * ## The icons are hand-drawn, and that is a deferral rather than a preference + * ## The icons are hand-drawn * - * `DESIGN.md` pins lucide-react (MousePointer2 / Square / Spline) and the design - * system is the - * task that installs it. Adding a dependency to `@visionset/app` so a demo can - * draw three glyphs would put the choice in the wrong milestone, so these are - * three inline paths at lucide's 24-unit grid and 2px stroke. They are replaced, - * not extended, when the real icon set arrives. + * Three inline paths on a 24-unit grid at 2px stroke. This strip draws its own + * chrome from `./theme` and imports nothing but the engine, which is the point of + * it — the product's own geometry glyphs are `GeometryIcon`'s, and they are not + * what a demo of the headless engine should be exercising. */ import { drawableGeometries, hotkeyForClass } from "@visionset/annotator"; @@ -219,7 +217,7 @@ function IconButton({ ); } -/** lucide's grid and stroke, three paths of it. See the header note. */ +/** Three paths on the grid and stroke the header note describes. */ function ToolIcon({ tool }: { readonly tool: Tool }): JSX.Element { return ( /\.tsx?$/.test(name) && !/\.test\.tsx?$/.test(name)) - .sort(); -} - -describe("the annotation workspace's icon set", () => { - it("draws Tabler only, in every production module", () => { - const sources = productionSources(); - expect(sources.length).toBeGreaterThan(0); - - const offenders = sources.filter((name) => - readFileSync(resolve(ANNOTATOR, name), "utf8").includes("lucide-react"), - ); - expect(offenders).toEqual([]); - }); -}); diff --git a/frontend/ui-core/src/index.ts b/frontend/ui-core/src/index.ts index 9be45d05..695430c9 100644 --- a/frontend/ui-core/src/index.ts +++ b/frontend/ui-core/src/index.ts @@ -42,7 +42,7 @@ export type { LabelClass } from "./palette.js"; export { cn } from "./lib/cn.js"; -// Primitives — Radix + lucide only (decision H). +// Primitives — Radix behaviour under shadcn Nova styling, iconed with Tabler. export { Button, buttonVariants, type ButtonProps } from "./primitives/Button.js"; export { Card, diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index f06bb428..0190b8da 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -59,9 +59,6 @@ importers: '@visionset/ui-core': specifier: workspace:* version: link:../ui-core - lucide-react: - specifier: ^1.32.0 - version: 1.32.0(react@19.2.8) react: specifier: ^19.0.0 version: 19.2.8 @@ -174,9 +171,6 @@ importers: clsx: specifier: ^2.1.1 version: 2.1.1 - lucide-react: - specifier: ^1.32.0 - version: 1.32.0(react@19.2.8) openapi-fetch: specifier: ^0.17.0 version: 0.17.0 @@ -2511,11 +2505,6 @@ packages: lru-cache@5.1.1: resolution: {integrity: sha512-KpNARQA3Iwv+jTA0utUVVbrh+Jlrr1Fv0e56GGzAFOXN7dk/FviaDW8LHmK52DlcH4WP2n6gI8vN1aesBFgo9w==} - lucide-react@1.32.0: - resolution: {integrity: sha512-txX56hMFnRxPi1f9/nH69YN8uvAO6a7Y1KSWKjCDAtdD9+soEgmWuCt6iRm1pkxUZo2+YntSdsE1L6bIuKoY8Q==} - peerDependencies: - react: ^16.5.1 || ^17.0.0 || ^18.0.0 || ^19.0.0 - lz-string@1.5.0: resolution: {integrity: sha512-h5bgJWpxJNswbU7qCrV0tIKQCaS3blPDrqKWx+QxzuzL1zGUzij9XCWLrSLsJPu5t+eWA/ycetzYAO5IOMcWAQ==} hasBin: true @@ -5533,10 +5522,6 @@ snapshots: dependencies: yallist: 3.1.1 - lucide-react@1.32.0(react@19.2.8): - dependencies: - react: 19.2.8 - lz-string@1.5.0: {} magic-string@0.30.21: diff --git a/tests/scripts/design_tokens.test.mjs b/tests/scripts/design_tokens.test.mjs index 142a523f..1fcf05ff 100644 --- a/tests/scripts/design_tokens.test.mjs +++ b/tests/scripts/design_tokens.test.mjs @@ -318,6 +318,51 @@ test("components.json holds the schema-supported preset fields, and no others", assert.equal(config.tailwind.css, "src/styles.css"); }); +/** + * Tabler is the icon set, and the only one — the final state of a migration that + * ran through the primitives, the screens and the annotation workspace in turn. + * + * The interesting failure is a *return*, not the original debt: an editor + * auto-import, or a branch that predates the migration coming back through a + * merge. Now that no manifest declares the old package such an import fails to + * resolve, which is the loud half. The quiet half is the manifest — a dependency + * added back "because something imported it" restores the whole problem without + * anything else noticing, so both halves are asserted here. + * + * Assembled from fragments so this file never contains the package's name as a + * contiguous string, and a repository-wide sweep for it never mistakes its own + * guard for a lingering usage — the trick `HEX` and `RETIRED_DECLARATIONS` above + * already use. + */ +const RETIRED_ICON_PACKAGE = ["lucide", "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" }); + assert.equal(listed.status, 0, `git ls-files failed: ${listed.stderr}`); + const tracked = listed.stdout.split("\0").filter(Boolean); + + const manifests = tracked.filter((name) => /(?:^|\/)package\.json$/.test(name)); + assert.ok(manifests.length > 0, "no manifests were read, so this proves nothing"); + const declaring = manifests.filter((name) => + readFileSync(path.join(REPO, name), "utf8").includes(`"${RETIRED_ICON_PACKAGE}"`), + ); + assert.deepEqual( + declaring, + [], + "@tabler/icons-react is the frontend's icon set. A second one is a decision for " + + `DESIGN.md, not a dependency:\n${declaring.join("\n")}`, + ); + + const sources = tracked.filter((name) => SOURCE.test(name) && !GENERATED.test(name)); + assert.ok(sources.length > 0, "no frontend sources were read, so this proves nothing"); + const importing = sources.filter((name) => + new RegExp(String.raw`(?:from|require\()\s*["']${RETIRED_ICON_PACKAGE}["']`).test( + readFileSync(path.join(REPO, name), "utf8"), + ), + ); + assert.deepEqual(importing, [], `these draw from the retired icon set:\n${importing.join("\n")}`); +}); + test("the tokens have exactly one home, and it is the stylesheet", () => { const listed = spawnSync("git", ["ls-files", "-z"], { cwd: REPO, encoding: "utf8" }); assert.equal(listed.status, 0, `git ls-files failed: ${listed.stderr}`);