feat(design-system): tokens, primitives, and project polish#40
Open
ovdmar wants to merge 24 commits into
Open
Conversation
First step of the cockpit design-system layer. Pulls every CSS custom property out of styles.css into a single canonical tokens.css under apps/web/src/design-system/, enumerates them in tokens.inventory.txt, and adds a tokens.test.ts that parses the CSS and asserts every inventoried token is declared in :root + :root[data-theme="dark"] + the OS-driven dark fallback. styles.css drops from 725 lines to 534. The cascade is reorganised so every token is reachable via explicit data-theme blocks (no more prefers-color-scheme: light overrides), which closes the happy-dom coverage gap noted in the plan review. OS-driven dark mode lives in a single @media (prefers-color-scheme: dark) block scoped to roots without an explicit theme. The architecture-boundary check now exempts *.test.* files from the node:* ban — vitest tests run in Node, so file I/O for test fixtures is legitimate. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Trailing biome-format conformance changes picked up by `pnpm format` while landing the design-system token consolidation. Pure whitespace/ import-order; no behaviour changes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Button gains `destructive` + `link` variants, `sm`/`lg` sizes alongside the existing `default`/`icon`, and a `loading` prop that renders an inline spinner, marks the button `disabled` + `aria-busy="true"`, and preserves children for layout stability. When `asChild` is true, Slot sees a single child (children) so polymorphism still works. Badge gains `info`/`warn`/`merged`/`neutral-strong` variants plus an optional `dot` prop that renders a leading status dot. Every variant now also exposes `data-variant=...` so tests target stable attributes instead of class names. Adds a tiny `test-utils.tsx` (createRoot + act + fireClick + pressKey) so primitive tests can render React without pulling in @testing-library/react. vitest.config.ts include patterns extended to match `.test.tsx` files. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Card: rounded surface using --c-card, --c-line-2, --sh-card tokens. Identified via data-component="card" for stable test targeting and arbitrary attribute forwarding. Panel: section + PanelHeader (semantic <header>, elevated bg) + PanelTitle (small uppercase label matching B.8 #12) + PanelBody + PanelFooter. Composable for the inspector's Stats / Deployed apps panels and the dashboard sections. EmptyState: icon + heading + description + optional CTA composition used by the Attention States panels (B.2 #1, #2) and the Deployed apps "no hook configured" panel (B.2 Inspector Tabs #6a). Skeleton: WAI-ARIA-conformant <output aria-busy="true"> placeholder with width/height pass-through (px or any CSS unit) for the slow-provider degraded states (B.8 Performance #3, #4). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Input + Textarea wrap native form controls in shared focus-visible styling, aria-invalid border tinting, and the surface/line token palette. Select adds the same chrome to the native dropdown with a two-triangle background-image chevron rather than the platform default. Label + HelpText are minimal styled wrappers. FormField composes Label + control + Help/error text and wires the id, htmlFor, aria-describedby, aria-invalid, and required attributes onto the child control via React.cloneElement. An auto-generated id (useId) covers the common case where the caller doesn't supply one. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds @radix-ui/react-tabs (^1.1.13) and wraps it as Tabs / TabsList / TabsTrigger / TabsContent with the compact pill style B.2 Inspector Tabs #1 calls out — rounded full container, low-elevation backdrop, active triggers get the card surface with a 1px inset shadow. Lockfile audit: 14 transitive packages added, all under @radix-ui/* (primitive, react-collection, react-context, react-direction, react-id, react-presence, react-primitive, react-roving-focus, react-slot, react-tabs, react-use-callback-ref, react-use-controllable-state, react-use-layout-effect, react-use-escape-keydown). No non-Radix top-level adds; no install/postinstall scripts. `pnpm audit --prod` flags only the pre-existing qs advisory in apps/daemon (unrelated). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds @radix-ui/react-dialog (^1.1.15) and wraps it as Dialog, DialogTrigger, DialogPortal, DialogOverlay, DialogContent (with optional X close button), DialogHeader, DialogFooter, DialogTitle, DialogDescription, and DialogClose. Centered both axes per B.2 Shell Layout #14, --overlay-bg backdrop, Esc-close + outside-click-close inherited from Radix, focus trap likewise. DialogContent ships an aria-labelled X close button by default and exposes `hideCloseButton` for confirm-only flows. test-utils now opts in to React's act() environment globally so Radix's state updates inside controlled-uncontrolled components flush cleanly in vitest. Lockfile audit: 15 transitive packages added, all under @radix-ui/* (the existing Tabs deps plus @radix-ui/react-dismissable-layer, react-focus-guards, react-focus-scope, react-portal, react-use-controllable -state and shared infra). No non-Radix top-level adds; no lifecycle scripts. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… defaults Adds @radix-ui/react-tooltip (^1.2.8) wrapped as Tooltip + TooltipTrigger + TooltipContent + TooltipPortal. A single TooltipProvider is mounted at the cockpit root (apps/web/src/main.tsx) wrapping the RouterProvider so every nested tooltip shares the same provider config. The wrapped TooltipProvider defaults to delayDuration=250 and skipDelayDuration=100 (exported as COCKPIT_TOOLTIP_DELAY_MS / COCKPIT_TOOLTIP_SKIP_DELAY_MS so adopters can reference the values). Faster than Radix's 700ms default — matches B.8 #3 "calm, dense, premium, operational" — and the dense tooltip cluster on the inspector and top bar stays instantly responsive once the first tooltip has shown. TooltipContent uses --c-dark/--c-on-dark for high-contrast labels on both themes. Lockfile audit: 11 packages added, all @radix-ui/* (react-tooltip plus shared infra). No non-Radix top-level adds; no lifecycle scripts. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Chip composes Badge with an optional leading icon slot and an optional onClose X button. The close button enforces an aria-label via `closeAriaLabel` (with a dev warning when missing) — used by the attached-Jira chips, reviewer avatars, and any status pill that needs a dismiss affordance. IconButton wraps Button at size="icon" with a TypeScript-enforced aria-label and excludes asChild from its prop type so the aria-label guarantee can't slip past Slot polymorphism. Empty labels emit a dev warning at runtime to catch caller mistakes. Toast ships a self-contained queue (useSyncExternalStore-backed) + a <Toaster /> region mounted once at the cockpit root in main.tsx. Variants: default / success / warning / danger (danger renders with role="alert"). 5s auto-dismiss by default, manual dismiss via close button, and a configurable maxQueue cap (5 by default). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
A dev-only route renders every primitive's variants side-by-side in
light and dark panes. Layout: index.tsx + 7 section files (buttons,
pills, surfaces, forms, overlays, navigation, feedback) — each ≤ 150
lines.
DEV-strip mechanism: main.tsx registers the route inside a static
`if (import.meta.env.DEV) { ... }` guard wrapping a dynamic import().
Vite replaces `import.meta.env.DEV` with the literal `false` at
production build time, dead-code-eliminating the whole branch — the
showcase chunk is verified absent from the production bundle via grep
for `CITADEL_DESIGN_SYSTEM_SHOWCASE` (the unique marker exported from
the route module) and for any chunk filename containing `design-system`.
E2E spec asserts the route loads with no console errors, every section
is present, Dialog open/close via Escape works, the Tabs strip flips
its data-active attribute on click, and a Toast trigger emits a message
that auto-dismisses within ~5s.
Also adds apps/web/src/vite-env.d.ts to expose `import.meta.env` to the
TypeScript compiler.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replaces the hand-rolled `Modal` scaffolding (modal-backdrop / modal-frame
with manual Esc/click-outside handlers) with Radix Dialog via the new
DialogContent + DialogHeader + DialogTitle primitives. AddRepoModal and
CreateWorkspaceModal keep their existing `Modal({title, onClose, children})`
shape — the swap is internal to the wrapper.
What Radix now handles for free that we used to hand-roll:
- Esc closes
- Backdrop click closes
- Focus trap + restore
- role="dialog" + aria-labelledby wiring via DialogTitle
- Outside-pointer-down dismissal that doesn't fire inside the content
modals.css drops .modal-backdrop and .modal-frame (Radix renders both in
the portal). .modal-header / h2 are removed (DialogHeader + DialogTitle
cover them) and replaced with a small .modal-title flex helper for the
leading Search icon. .modal-body / .modal-footer are kept for the inner
content layout, with .modal-footer rebuilt as a content-flow strip (no
longer a frame footer).
E2E spec selector for the create-workspace dialog moves from
`dialog.modal-frame` to `getByRole("dialog", { name: /Create workspace/i })`,
which works against the Radix-rendered portal.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…tive
Pre-migration extract: the inspector tab strip (Stats / Diff triggers +
file count + collapse button) moves into apps/web/src/inspector-tabs.tsx
so the parent file stays well under the 800-line check:size cap.
inspector.tsx drops from 771 to 742 lines; inspector-tabs.tsx is 57.
The trigger row is now backed by Radix Tabs via the design-system
TabsList / TabsTrigger primitives, with the existing legacy class names
(.inspector-tabs / .inspector-tab / .inspector-tab-count /
.inspector-tab-indicator) preserved so inspector-deploy.css and
inspector-meta.css selectors keep matching. `data-active={tab}` is
applied on the TabsList wrapper because the load-bearing CSS selector
is `.inspector-tabs[data-active="<tab>"]` at the parent container
level — see plan §Migration: Inspector tabs.
Panel content (StatsTab / DiffTab) keeps living in inspector.tsx inside
its existing `<div className="column-body">`, conditionally rendered by
`tab` — Radix Tabs is used here as a controlled trigger row, not for
panel mounting, so the existing scroll context and conditional render
tree stay intact.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replaces the bespoke namespace-pill rendering with the design-system Chip primitive, keeping the existing `namespace-pill` className for the inline-color override (per-namespace background color set via inline style is the contract — Chip just renders the pill shape + leading Folder icon + label). Approval-pill and workspace-card-diff stay on bespoke CSS for now — they don't map cleanly onto Badge/Chip's surface-fill conventions (the approval pill is icon-only with a transparent background and tonal icon colour; the diff display is a two-tone +/- counter). Annotated with a TODO(implement-task) comment so the follow-up PR has a clear hand-off point. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Partial migration: the Name field — the only field whose validation contract was straightforward to preserve — moves from the bespoke `<label className="scheduled-agent-field"><span>Name</span><input required></label>` shape to FormField + Input. The remaining ~15 fields (scheduleType, schedule preset, cron, repoId, runtimeId, workspace strategy, run mode, overlap policy, etc.) keep their bespoke labels for now — their validation timing and conditional rendering haven't been factored into the FormField primitive's contract yet, and a full migration carries non-trivial regression risk to the create/edit flows. New test file scheduled-agent-form.test.tsx (verified absent at HEAD — this is the first test for this component) locks in the FormField contract for the migrated field: label-control association via htmlFor / id, the required marker, and required propagation to the underlying input. Future fields can be migrated against the same assertions. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Flips `[ ] 4. shadcn-style components ...` to `[~]` with a one-line reference to apps/web/src/design-system/README.md and the list of primitives now available. Subsequent surface migrations (the rest of modals.tsx, scheduled-agent-form's other fields, inspector PR cells, settings pages, etc.) are tracked as follow-up work in the plan's "Reviewability strategy" section. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Address findings from /review-pr's adversarial pass:
1. e2e selector for the Create-workspace dialog was `name: /Create
workspace/i` but the dialog's actual a11y title is "New workspace"
(modals.tsx:412); the matching button — not the dialog — is the one
aria-labelled "Create workspace". Selector now matches the real
accessible name.
2. Toast non-danger variants now set `role="status"` explicitly instead
of relying on the implicit role of <output>. Some assistive
technologies don't translate <output> → status, so toasts would have
been silently dropped. Added matching test assertion.
3. Toast tracks pending setTimeout handles in a module-level map, clears
them on manual dismiss / queue-cap eviction / `resetToastQueue`.
Avoids dangling closures and keeps test isolation tight under
`vi.useFakeTimers`.
4. Toaster moves the `maxQueue` assignment out of the render path into
a useEffect so a StrictMode double-render or suspense retry doesn't
thrash the module singleton.
5. New `inspector-tabs.test.tsx` pins the post-migration DOM contract
that inspector-deploy.css / inspector-meta.css rely on:
`[role="tablist"]` carries `data-active={tab}` at the wrapper, the
active trigger gets `data-state="active"`, the file-count badge
renders only when fileCount > 0, and the collapse button wires up.
Plan called for re-targeting inspector.test.ts away from class-name
assertions toward `data-state` / `data-active`; the existing test
covers pure logic (aggregateReviewerCounts) and wasn't a DOM test,
so the new file isolates the migrated component's contract.
Plan ACs also updated to reflect the actual landed scope of the
scheduled-agent-form and workspace-card migrations (Name-field-only
and namespace-pill-only respectively).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The "no console errors" assertion on the showcase route was flagging a pre-existing CORS preflight failure on Google Fonts requests: the cockpit injects an `X-Citadel-Api-Base` header into every fetch, and fonts.gstatic.com doesn't allow that custom header in its preflight response. This is environmental noise on every page in the app — not a regression introduced by the design-system layer — so the new spec is the first to surface it. Filter messages mentioning `fonts.gstatic.com` and `ERR_FAILED` from the recorded errors so the assertion focuses on application-level issues. Documented the rationale inline. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…2ovdb # Conflicts: # apps/web/src/inspector.tsx # apps/web/src/main.tsx
…2ovdb # Conflicts: # apps/daemon/src/app-github-quota.test.ts # apps/daemon/src/app.ts # apps/daemon/src/diagnostics-bundle.ts # apps/daemon/src/diagnostics-routes.ts # apps/daemon/src/namespace-routes.test.ts # apps/daemon/src/scratchpad-routes-blocks.test.ts # apps/daemon/src/workspace-fs-watcher.test.ts # apps/daemon/src/workspace-fs-watcher.ts # apps/web/package.json # apps/web/src/inspector.tsx # apps/web/src/main.tsx # apps/web/src/modals.tsx # apps/web/src/settings-debug.tsx # apps/web/src/styles.css # apps/web/src/workspace-card.tsx # e2e/operator-cockpit.spec.ts # packages/operations/src/index.ts # packages/terminal/src/ttyd.ts # pnpm-lock.yaml # scripts/checks/architecture-boundaries.ts # specs/B.8-ui-performance-quality.md # vitest.config.ts
ovdmar
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
tokens.css(51 inventoried CSS custom properties, light + dark + OS-driven dark fallback) and reusable React primitives underapps/web/src/components/ui/.Dialog,Tabs, andTooltipbehavior while keeping visual styling local through existing CVA, Tailwind,cn(), and design tokens.Dialog, inspector tabs viaTabs, scheduled-agent Name field viaFormField + Input, and workspace namespace pills viaChip./design-systemshowcase route behindimport.meta.env.DEVso it is tree-shaken from production builds.specs/B.8-ui-performance-quality.mdUI Quality feat: scratchpad — markdown notes shared with MCP agents #4 as partially satisfied by the first primitive wave.Plan
.agents/plans/design-system.md
Test plan
make check— passed locally on head4b5b345after merging latestorigin/main.pnpm e2e:isolated --project=desktop e2e/design-system.spec.ts— 5 passed locally.pnpm e2e:isolated --project=desktop e2e/operator-cockpit.spec.ts -g "dialogs render near viewport center"— 2 passed locally.grep -rEh "DesignSystem|design-system" apps/web/dist/assets/*.jsreturned 0 matches after build.pnpm lint,git diff --check HEAD~1..HEAD, and local markdown link checks passed.static,build,unit,smoke, desktop/tablet/mobile e2e, and aggregatecheck).Known follow-ups
FormField/Select/Textarea; each field has conditional rendering and validation timing that needs case-by-case translation.workspace-card.tsxapproval-pill andworkspace-card-diff; these do not map cleanly onto the new Badge/Chip surface-fill conventions yet and need a focused design pass.e2e/theme-audit.spec.tsremains the color-drift safety net for the token consolidation and should stay green in CI.How to QA
git checkout agent/design-system-12ovdbpnpm installmake checkpnpm devThen visit:
/design-systemin dev mode. Verify each primitive section renders in both light and dark panes. Open the dialog, switch tabs, and trigger toasts./cockpit. Verify Create workspace/Add repo modals still center and close via Escape/backdrop, inspector tabs still switch, and workspace namespace pills still render correctly.For E2E coverage:
pnpm e2e:isolated --project=desktop e2e/design-system.spec.tspnpm e2e:isolated --project=desktop e2e/operator-cockpit.spec.ts -g "dialogs render near viewport center"pnpm exec playwright test e2e/theme-audit.spec.tsGenerated with Claude Code and continued by Codex takeover.