Repository navigation
docs(design): correct DESIGN.md against the current tokens and components - #85
Merged
Merged
Conversation
…ents `impeccable doctor` reports one finding (`design-md-coverage`), but reading DESIGN.md against the code turns up several places where the document describes something the code does not do. Those are corrected here; none of them changes a rendered pixel. Wrong about existing code: - The Form field recipe said the label is `text-xs` and the control `h-8`. `Label` / `FieldLabel` are `text-sm font-medium`, `Input` and the `Select` trigger are `h-9`, and the density table already said `h-9` — the recipe was contradicting both the components and the table. Error text was documented as `text-xs`; `FieldError` is `text-sm`. - The recipe described a hand-rolled label/input/error stack and never mentioned `components/ui/field.tsx`, which is what the settings surfaces actually use. The primitives are now named, including the `role="alert"` wiring `FieldError` already provides. Missing values the code already uses: - The title-bar density tier (28px tabs and icon buttons in a 44px bar) was undocumented, which made `TopNavigationBar` look like it was violating the `size-8` toolbar-icon-button rule instead of being a deliberate tier. - `Tabs` was documented as having exactly one form, the segmented control. The open-notebook strip in the title bar is a second form, and it expresses selection with the translucent fill rather than a surface step. - The `border-none` opt-out that #58 added to the panel-level empty states was not recorded, so the next reader would reintroduce the dashed outline there. Also: the legacy-alias list was incomplete, and it implied `--secondary` was one of those aliases. It is not — it holds its own literals, nothing renders it (`Button` and `Badge` `variant="secondary"` use `bg-muted`), and its light value is the only blue-tinted neutral in the palette. DESIGN.md now says so and marks it for deletion rather than adoption.
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.
What does this PR do?
Corrects
DESIGN.mdwhere it describes something the code does not do, or leaves out a value the code already uses. Documentation only — no rendered pixel changes.Follow-up to the merged #84: that PR added the Window chrome section and the Raised card recipe, and this pass is the rest of the same check.
Why?
impeccable doctorreports one finding (design-md-coverage), but that finding is about heading names, not content. Reading DESIGN.md against the current tokens and components turns up several places where the document is simply wrong about the code, and a few values the code uses that the document never states.Because DESIGN.md is the source of truth for UI decisions, a wrong recipe is worse than a missing one: the next agent copies it.
What changed?
Wrong about existing code
The Form field recipe contradicted both the components and the density table.
It said the label is
text-xsand the controlh-8:Labelistext-sm font-medium leading-none;FieldLabeland thehand-written labels in
ModelConnectionFormaretext-sm font-medium.Inputish-9and theSelecttrigger ish-9, and the density tablealready said
Input / Select trigger | h-9. The recipe'sh-8is thesmbutton height and the compact row height, not an input height.
text-xs;FieldErroristext-destructive text-sm font-normalwithrole="alert".components/ui/field.tsxwas never documented, yet it is what the settingssurfaces use (
GeneralSettingsrendersFieldLabel). The recipe described ahand-rolled label/input/error stack. The primitives are now named, and the
recipe points at
FieldErrorinstead of restating its classes.Missing values the code already uses
TopNavigationBarruns 28px tabs and 28px iconbuttons inside the 44px bar. Without a row in the density table, that reads as a
violation of
Toolbar icon button | size-8rather than a deliberate tier, sothe tier is now documented as stopping at the title bar.
Tabswas documented as having exactly one form — the segmented control,where "selection is expressed as a surface step". The open-notebook strip in the
title bar is a second, legitimate form: no track, and selection expressed with
the ordinary translucent
bg-surface-selectedfill. Both are now written down.border-noneopt-out that fix(ui): remove the dashed border from the knowledge base empty state #58 added to the panel-level empty states wasnever recorded, so the next reader would reasonably reintroduce the dashed
outline in the knowledge-base panel.
Smaller corrections
--secondarywas one ofthose aliases. It is not: it holds its own literals in both themes, nothing
renders it (
ButtonandBadgevariant="secondary"both usebg-muted), andits light value is the only blue-tinted neutral in the palette. DESIGN.md now
says so and marks it for deletion rather than adoption.
Open decisions this PR does not make
Found by the same pass; both need a call rather than a doc edit, so neither is in
this diff:
surface-sunken; it renderssurface-base.TopNavigationBarcarries no background class, so itinherits the page root and is the same tone as the canvas rather than a
distinct chrome band. Either the table row is wrong, or the bar should get
bg-surface-sunken. The second option is a visible change.(Everything else that row names — the settings
Sidebar, the settings dialogshell — does use
bg-surface-sunken.)--secondary/--secondary-foregroundare dead tokens. Now documented assuch; deleting them from
theme.cssis a separate mechanical change.How was this tested?
npx prettier --check DESIGN.md— clean.ui/label.tsx,ui/input.tsx,ui/select.tsx,ui/field.tsx,ui/tabs.tsx,common/TopNavigationBar.tsx,settings/ModelConnectionForm.tsx,notebook/source/DocumentList.tsx,assets/theme.css.npm run check:design— no violations.No code changed, so
npm test/npm run buildwere not re-run.Screenshots / recordings
Not applicable — documentation only.
Checklist
npm run typecheckpasses. (unchanged; no code touched)npm run buildpasses. (unchanged; no code touched)Desktop / build changes