Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .agents/skills/frontend/nodejs-setup/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,8 @@ pnpm add -w -D <pkg> # 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.

Expand Down
11 changes: 5 additions & 6 deletions DESIGN.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
2 changes: 1 addition & 1 deletion docs/content/architecture/frontend/ui-core.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand Down
1 change: 0 additions & 1 deletion frontend/app/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
14 changes: 6 additions & 8 deletions frontend/app/src/demo/ToolStrip.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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 (
<svg
Expand Down
1 change: 0 additions & 1 deletion frontend/ui-core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@
"@visionset/annotator": "workspace:*",
"class-variance-authority": "^0.7.1",
"clsx": "^2.1.1",
"lucide-react": "^1.32.0",
"openapi-fetch": "^0.17.0",
"shadcn": "^4.18.0",
"sonner": "^2.0.8",
Expand Down
48 changes: 0 additions & 48 deletions frontend/ui-core/src/annotator/iconSet.test.ts

This file was deleted.

2 changes: 1 addition & 1 deletion frontend/ui-core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
15 changes: 0 additions & 15 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

45 changes: 45 additions & 0 deletions tests/scripts/design_tokens.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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}`);
Expand Down
Loading