feat: formalize design system on existing shadcn/Tailwind foundation - #326
Open
bmersereau wants to merge 3 commits into
Open
feat: formalize design system on existing shadcn/Tailwind foundation#326bmersereau wants to merge 3 commits into
bmersereau wants to merge 3 commits into
Conversation
|
|
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
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
Formalizes the design system that already exists rather than changing it: documents the current tokens and primitives, promotes three duplicated patterns into the shared layer, and fixes the accessibility gaps in the primitives. No visual redesign — the only pixel-level changes are the accessibility fixes listed below.
Closes #322
Scope note: catalog tool deferred
The component catalog (acceptance-criteria item 2, Storybook or Ladle) was intentionally deferred to a follow-up issue and is not in this PR. As a result this PR adds no new dependencies —
frontend/package.jsonandpackage-lock.jsonare untouched.1.
docs/design-system.mdDocuments the existing foundations, measured off
globals.cssand all ~120 component files rather than assumed:app-*surface tokens, and the shadcn semantic tokens (~45 usages, effectively primitives-only). Plus the accent blue, and the five files that legitimately use raw hex..darktoken set exists but nothing ever adds the class, anddark:variants appear only in the three vendored shadcn primitives. Worth writing down so nobody assumes it works.font-serif, and the de facto type scale (text-xsat ~200 uses is the workhorse; this is a dense, table-heavy app).shadow-[...]recipes are never retyped, since that is this codebase's main source of copy-paste drift.2. Consolidated primitives
Chosen from a grep-and-read pass over the feature components, not invented. Each was verified as genuinely the same markup before promoting it:
ui/glass-icon-button.tsxModal,DocumentSidePanel,TRSidePanel,TREditColumnMenuui/empty-state.tsxTableEmptyStateProjectAssistantTable,ProjectReviewsTable,ProjectsOverview(×2),WorkflowList(×3),WorkflowDetailPageui/check-square.tsxAddProjectDocsModal,QuickActionsModal,FileDirectory(×3)Deliberately not consolidated, to keep this honest:
Loader2spinners — 43 sites with real size/colour variation at each; a wrapper would trade one line for one line.tabular/pillUtils.ts.ProjectReviewsTable's load-error state and the short "no results found" variants — superficially similar but a different heading size and a deliberately smaller state.Two design decisions worth flagging:
EmptyStaterenders no wrapper, so it drops straight into the existingTableEmptyStatecontainer; andCheckSquareis deliberately role-free and ARIA-free, because call sites disagree about whether the checkbox semantics belong on the square or on the row button wrapping it, and adding a role would double up on the three that already declare one.3. Accessibility fixes
All applied, not just reported, and each covered by a test:
cite-button— was missingtype="button"(so it submitted any surrounding form) and had no accessible name at all when rendered icon-only. Now labelled only when there is no visible text, so the visible "Cite" stays the accessible name (WCAG 2.5.3).search-bar— the input had no accessible name (a placeholder is not a name, WCAG 4.1.2), and its only focus indicator was awhite/70→white/90shift, which is imperceptible (WCAG 2.4.7).tab-pill-button— the inactive state usedtext-gray-400on a translucent white pill, ~2.5:1, failing WCAG 1.4.3. Nowtext-gray-500at ~4.8:1.toggle-switch— the off state was a white thumb on agray-100track, ~1.07:1, effectively invisible (WCAG 1.4.11). Fixed withring-1 ring-gray-300rather than a border, so the absolutely-positioned thumb doesn't shift by 1px.dropdown-menu— menu items setoutline-hiddenand relied onfocus:bg-accent, but theliquid-dropdownskin overrides that with a near-white hover colour (~1.02:1), leaving keyboard users no focus indicator. Added a background-independent ring. Also, a selected radio item was marked bybg-gray-100on white (~1.03:1) and by colour alone — now also carriesfont-mediumas a second, non-colour channel (WCAG 1.4.1).pill-button— had no focus-visible ring.--muted-foregroundis ~4.6:1 on the light background) andbutton/input, which already ship upstream shadcn focus andaria-invalidhandling.4.
CONTRIBUTING.mdOne additive
## Frontend UIsection on checkingcomponents/ui/and the shadcn registry before hand-rolling a component. Placed in its own section and does not restructure the file, so it should not conflict with #324's separate addition.Test plan
frontend/src/app/components/ui/(new primitives + the ARIA/focus/contrast assertions), matching the existing house style: queried by role and accessible name, asserting only the classes that carry meaning.src/app/lib/mikeApi.test.ts(blob.text is not a function) are pre-existing and reproduce on an unmodified tree — a local Node 24Response.blob()difference.CIandCode Qualityare green onmainat this branch's base commit (1af9231), and CI runs Node 22, so no issue was filed.npm run test:coverage(the CI gate) — thesrc/app/lib/**ratchet floor is untouched by this change.npm run lintwith the cache cleared — 0 errors, and no warnings in any file this PR touches.npm run build— passes with the same placeholder env vars CI uses.tsc --noEmitclean.