Add a design-token layer and convert the site header onto it by ryanwelcher · Pull Request #277 · WordPress/contributor-toolkit

ryanwelcher · 2026-08-11T14:24:33Z

First of a stack. This is Phase 1 of a seven-phase UI pass. Phases 2–7 build on it, one PR each: first-run path, setup checklist, notices, Trac card, patch/terminal panes, sidebar and cleanup.

Why

Controls float. "Start dev server" and "Review & submit changes" sat in open space between the header and the Trac card, belonging to nothing.

That is architectural, not cosmetic. index.jsx is styled by 317 inline style={{}} objects, so every panel picks its own values: eight marginTops, three border greys, two muted greys, three radii.

Before and after

The path, its copy button and "Open directory in" were three stacked rows. They are now one meta block. The two buttons sit in a bounded row under a hairline.

Before

After

The content column, on a wide window

The only shot taken at 1600px. Every other image uses 1200px, where the content area is ~855px — narrower than the 880px cap, so the cap has no visible effect and no screenshot could show it.

Setup wizard

The checklist below the header is untouched — that is Phase 3.

Before

After

What changes

src/renderer/styles/tokens.css — one place for spacing, colour, radius and type. Imported after the @wordpress/components stylesheet; esbuild concatenates CSS imports in order, so the cascade is right with no build change. Values follow WPDS.

src/renderer/ui/ — Section, ActionRow, MetaText, StatusBadge. Each replaces something the file already did several ways.

The site header, converted onto both.

Three decisions worth naming:

The dev-server button drops its 15px text and 12px radius for the stock WPDS shape. .next-action-cue then reads --wpct-radius-control, because a 10px glow around a 2px button looked like a rounded box behind a square one.
The cue's colour becomes --wpct-cue — pixel-identical to the #f0b849 it replaces, and deliberately not the warning amber. The cue means "do this next", and it is often drawn around a warning notice.
status-tone.cjs maps status to tone only. setupStepLabel owns the checklist words and distinguishes "Ready" from "In progress" (The current setup step reads "In progress" before its action starts #257); restating them here would be a second source of truth.

Not in this PR: the checklist, notices, Trac card, patch and terminal panes, sidebar. Two header notices keep their inline styling for the same reason — converting one now leaves two notice styles on screen until Phase 4.

Also here, and not UI polish: the screenshot harness now produces the same images twice (c0f8a00). Three PNGs used to change on every run with no code change — CSS animations caught mid-frame, and the linked-pull-request lookup photographed mid-flight. It is in this PR because this branch already edits both harness files and eleven of the screenshots, so a separate PR would collide with it on rebase, and a binary conflict has no resolution but to pick a side and re-capture — the exact failure the commit prevents. It matters now because six more PRs stack on this branch. The commit message has the full reasoning; #326 covers what is left.

How to test this

Platforms: any (renderer-only).

Starting state:

An initialized site with the wizard skipped, so the header and action row both render.
A stale site, to check the amber dot and the divider under a notice.

What must not have happened:

Rename, copy-path and "Open directory in" all still work. Open the menu twice — it detects applications on each open.
The header h1 is still the page's only h1. The rename pencil sits beside the heading, not inside it.
The next-action glow still lands on one target and still scrolls it into view.
On a maximised window the content column stays centred and stops at 880px.

Risks and limitations

The content column was already capped at an inline maxWidth: 1040; this moves it to a token and tightens it to 880px.
/self-review found 4, fixed 3. See the review-outcome section below.
Inline styles: 317 → 292.

The screenshot harness gained a per-shot window size rather than a second global one. Layout that only appears past a breakpoint is invisible to a harness with one window, which is how the cap went unphotographed. Bounds are set before every shot, not only the ones that override, so a widened window cannot leak into the next image.

Dark mode is out of scope, and test/color-scheme.test.cjs holds that line. Every surface is hand-painted light and @wordpress/components ships light styles only.

Review outcome

Two passes. Round 1: 3 [fix here] · 1 [follow-up] — all 3 fixed in fed86fc. Round 2: 1 [fix here] · 3 [follow-up] — all 4 fixed in fcc2940.

🟡 architecture — a sentence laid out as a flex row, gapping every word fragment. MetaText gains a flow prop.
🔵 accessibility — the path chip at 4.44:1, under AA. .wpct-meta code also styled words it shouldn't have; it becomes an explicit .wpct-chip and its text moves to --wpct-text.
🔵 tests — statusTone read through Object.prototype, so 'constructor' and '__proto__' returned a non-tone. Now Object.hasOwn; both new cases fail on the old lookup.

Deferred: StatusBadge renders an empty pill for a status whose label is deliberately blank. Unreachable here — only initialized/uninitialized are used, both labelled — and it is the API Phase 3's checklist conversion will call, so it is decided there.

Round 2 re-reviewed the two commits the first pass never saw — the screenshot-harness change most of all, since six more PRs will build on it.

🔵 architecture — a shot's window was substituted for the default, not merged. setBounds takes a partial rectangle, so { width: 1600 } with no height would inherit the previous shot's — the exact bug the call was added to prevent, as a height instead of a width.
🔵 architecture — window was undocumented in the entry-shape contract and silently ignored in the live tier. Now documented, and a live shot that declares one fails loudly.
🔵 tests — the wide screenshot was asserted with existsSync, which could never fail again once committed. It now reads the PNG's IHDR width against the width the shot declares.
🔵 tests — the round-1 CSS fixes shipped with no regression test. test/meta-text.test.cjs computes the chip's contrast from the tokens rather than hard-coding it.

Every assertion added across both rounds was checked against a mutation that should break it, because an assertion that has never failed has not been tested.

Findings 1 and 2 of round 1 were introduced by this PR, not inherited.

Implementation notes

.next-action-cue lives in index.html's <style> block, emitted before the <link> to the bundled index.css that defines :root. That is fine — custom properties resolve at computed-value time. Verified rather than assumed: a decoded pixel diff of site-view.png across the colour change shows 64 differing subpixels, all in rows 411–422, which is the animated "Checking GitHub…" spinner.

test/content-column.test.cjs pins the cap, the class still being applied, the absence of a competing inline maxWidth, and the wide shot still existing and still being wide enough to prove anything. Each assertion was checked against a mutation that should break it — a widened token, a deleted rule, a renamed shot, a removed window override — because an assertion that has never failed has not been tested.

Rebased onto trunk, which moved nine commits during this work. Three reconciliations: "Forget this site" was removed from the header menu (#266); the cueProps wrappers and the wp-admin link (#250) were preserved inside the new ActionRow; and the step labels moved to setupStepLabel (#257), which is why status-tone.cjs carries no checklist words.

The renderer is styled by 317 inline style objects, so every panel picks its own spacing, border grey and radius and no two agree. The visible result is that controls float: "Start dev server" and "Review & submit changes" sat in bare whitespace between the header and the Trac card, owned by nothing. Introduces src/renderer/styles/tokens.css as the one place those decisions are made, four primitives in src/renderer/ui/ that consume it, and converts the site header as the first region to prove the approach end to end. The header's three stacked rows for path, copy and "Open directory in" become one meta block, and the two buttons move into a bounded action row under a hairline, so they read as this site's actions. The dev-server button loses its hand-picked 15px/12px-radius treatment for the stock WPDS shape, which in turn makes .next-action-cue read the control radius: a 10px glow around a 2px button looked like a rounded box floating behind a square one. The cue's own colour becomes --wpct-cue, pixel-identical to the #f0b849 it replaces. It is deliberately not the warning amber: the cue says "do this next" rather than "something is wrong", and it is often drawn around a warning notice, where a glow in that notice's own border colour would disappear into it. The content column's cap was already there as an inline maxWidth: 1040; it now reads --wpct-content-max-width and is tightened to 880px. status-tone.cjs maps a status to a tone and nothing else. It deliberately does not restate the checklist's words, because setupStepLabel already owns them and makes a distinction the map cannot see (#257). Two notices in the header keep their inline styling: every notice box in the window is converted together in a later pass, and doing this one now would leave two notice styles on screen until the rest caught up.

Contrast and layout, both introduced by the token pass itself, plus a lookup that could return a non-tone. `.wpct-meta code` styled every `code` element in a meta line. That was written for the path chip, but it also caught the `admin` / `password` in the dev-server credentials line, which are words in a sentence rather than a value to copy. It becomes an explicit `.wpct-chip` class on the path alone, and its text moves from --wpct-text-muted to --wpct-text: #6c6f72 on the chip's #f0f0f1 is 4.44:1, under the 4.5:1 AA threshold at 12px, where trunk's #3c434a was 8.81:1. The same credentials line was also a sentence inside a flex row, so each run of text became its own flex item — five items with a gap between every one, including before the full stop, each free to wrap separately. MetaText gains a `flow` prop for prose. statusTone read through Object.prototype: 'constructor' resolved to the Object constructor and '__proto__' to the prototype itself, both destructuring to an undefined tone and rendering a badge classed `wpct-badge--undefined`. Only already-lowercase keys could reach it, since the lookup lowercases first, but the fallback this function documents has to hold for every string. Both new test cases fail on the old lookup and pass on Object.hasOwn. A fourth finding — StatusBadge rendering an empty pill for a status whose label is deliberately blank — is left for the phase that first calls it that way.

The cap was the one part of this change nothing could show. Every screenshot is taken at 1200px, where a 280px sidebar and 32px of padding a side leave a content area of about 856px — narrower than the 880px cap. At that size the cap has no effect, so removing it entirely would change no image in docs/, while the app ran headings and meta lines to the edge of a maximised window. The harness had one window size for every shot, which is why. A shot may now carry its own `window`, and `site-view-wide` uses 1600px, where the content area is 1256px and the cap visibly bites. The bounds are set before every shot rather than only for the ones that ask, so a widened window cannot leak into the next image. test/content-column.test.cjs pins both halves: the rule and its token, that the scrolling container still applies the class, that no inline maxWidth is reintroduced to override it, and that the wide shot still exists and is still wide enough to prove anything. Each assertion was checked against a mutation that should break it — a widened token, a deleted rule, a renamed shot, a removed window override. The screenshot on its own is a regression net only if someone looks at it. This is the half that fails without being looked at.

The harness is about to be built on by six more PRs, so an API that is easy to misuse costs more here than in a leaf change. A shot's `window` was substituted for the default rather than merged over it. `setBounds` takes a partial rectangle, so a shot declaring only `{ width: 1600 }` — the natural thing to write when only the width matters — would have kept whatever height the shot before it left, and `--only=<slug>` would have produced a different image than a full run. That is the same "a shot inherits another's size" bug the call was added to prevent, reappearing as a height. `window` was also a fifth entry key documented only inside the one shot using it, and silently ignored in the live tier. It is now in the contract at the top of shots.cjs, and a live-tier shot that declares one fails loudly instead. The wide screenshot was asserted with existsSync alone, which could never fail again once the file was committed: an image captured before the cap regressed, or re-captured at the default width, would both pass. It now reads the PNG's IHDR width and compares it to the width the shot declares. The two CSS fixes from the first review shipped without regression tests, in a repo that has the pattern for exactly this. test/meta-text.test.cjs computes the chip's contrast ratio from the tokens rather than hard-coding it, and pins the prose layout — including that .wpct-meta--flow still comes after .wpct-meta, since equal specificity means source order is what makes it win. Each assertion was checked against a mutation that should break it: the chip colour reverted, the descendant selector reintroduced, `flow` dropped from the credentials line, the flow rule moved above the rule it overrides, the shot's declared width changed out from under the committed image, and the cap token deleted. The last one previously died with a TypeError instead of a message. Also corrects the content-area figure in two comments from 856px to the measured 855px — the sidebar's right border is a pixel.

Copilot

Pull request overview

Introduces reusable design tokens and UI primitives, then migrates the site header and action area onto them.

Changes:

Adds tokenized styling and reusable layout/status components.
Refactors the site header, metadata, and actions.
Extends screenshot tooling and regression coverage.

Review: 2 [fix here] findings.

Reviewed changes

Copilot reviewed 13 out of 24 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
`src/renderer/styles/tokens.css`	Adds design tokens and shared styles.
`src/renderer/status-tone.cjs`	Maps statuses to semantic tones.
`src/renderer/ui/Section.jsx`	Adds section layout primitive.
`src/renderer/ui/ActionRow.jsx`	Adds grouped-action primitive.
`src/renderer/ui/MetaText.jsx`	Adds metadata layout primitive.
`src/renderer/ui/StatusBadge.jsx`	Adds status badge primitive.
`src/renderer/index.jsx`	Migrates the site header and actions.
`src/renderer/index.html`	Tokenizes next-action cue styling.
`scripts/screenshots/shots.cjs`	Adds the wide-window screenshot.
`scripts/screenshots/capture.cjs`	Supports per-shot window dimensions.
`test/status-tone.test.cjs`	Tests status-tone behavior.
`test/meta-text.test.cjs`	Tests metadata styling invariants.
`test/content-column.test.cjs`	Tests the content cap and screenshot.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+async function setWindow(app, bounds) {
+	await app.evaluate(({ BrowserWindow }, size) => {
+		const win = BrowserWindow.getAllWindows()[0];
+		win.setBounds({ x: 40, y: 40, ...size });


+.wpct-badge--neutral {
+	background: var(--wpct-surface-subtle);
+	border-color: var(--wpct-border);
+	color: var(--wpct-text-muted);


Three PNGs changed on every run with no code change at all. Two causes, both fixed here. CSS animations. The "Checking GitHub…" spinner was photographed at whatever angle it happened to be at, so `page.screenshot` now passes `animations: 'disabled'`, which freezes animations and transitions and rewinds them to their first frame. The linked-pull-request lookup. Selecting a site with a linked ticket fires a network call, and the panel spins until it answers, so whether a shot caught the spinner or the result was a coin flip. `selectSite` now waits for the spinner to go. That fixes a real docs bug as well as the churn: the same run produced site-view.png mid-check and trac-ticket-panel.png already resolved, which is two different answers to the same question in one set of images. The wait is bounded and swallowed, because a site with no ticket never shows the spinner at all, and a harness that hangs on a slow GitHub is worse than one that photographs a spinner. The three re-captured images are the fix taking effect. This is not UI polish, and it is here rather than on its own branch because the alternative is worse. This branch already edits both harness files and eleven of the screenshots, including these three. A separate PR touching the same files would collide with this one on rebase, and a binary conflict has no resolution except to pick a side and re-capture — which is the exact failure this commit exists to stop. It matters now because the phases after this one stack on this branch. A full run is still occasionally not reproducible on site-menu.png and stale-site-notice.png, while each shot is reproducible on its own under `--only`. That points at network and cache state shared across shots in one app instance, and ending it needs the fixture to stop reaching GitHub at all. Left for its own issue.

Five conflicts needed a decision rather than a pick. `src/renderer/index.jsx` — trunk moved the Adminer control out of the action row and into the URL line as `DB inspect (Adminer)`, while this branch still had it as a button in the row it was tokenising. Keeping both would have shipped two ways into Adminer, so the button goes and the row closes as `</ActionRow>` rather than trunk's `</div>`, since the opener here is the tokenised one. `scripts/screenshots/capture.cjs` — the two sides added different functions at the same point, `setWindow` here and `expandHome` on trunk. Both are called, so both stay. `scripts/screenshots/shots.cjs` — trunk moved `setup-wizard` and `site-view` to the live tier (#298) because seeded state cannot produce what they photograph. That move stands. `site-view-wide` stays a fixture shot: what it depends on is the window width, not the Trac facts that forced the others across, and other fixture shots still drive `selectSite` the same way. The three test files move to `tests/unit/` with the rest, and their paths out to `src/`, `scripts/` and the repo root gain the level that move costs them. The screenshots take this branch's versions. Both sides changed all seven, and these are the ones showing the header this branch exists to change — but three of them are live-tier now, so only their author can recapture them. --- Generated with the help of Claude Code, https://code.claude.com Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

mokagio · 2026-08-21T02:43:17Z

@ryanwelcher thank you for this PR. You'll notice I just merged trunk into your branch. It's to sync with a few structural and security changes that landed on trunk since you opened this. Cheers.

ryanwelcher mentioned this pull request Aug 11, 2026

Reconsider whether the next-action cue should reach beyond the setup checklist #278

Open

ryanwelcher added 3 commits August 11, 2026 10:39

ryanwelcher marked this pull request as ready for review August 11, 2026 17:02

juanmaguitar requested a balanced review from Copilot August 12, 2026 06:24

Copilot started reviewing on behalf of juanmaguitar August 12, 2026 06:27 View session

Copilot AI reviewed Aug 12, 2026

View reviewed changes

ryanwelcher mentioned this pull request Aug 12, 2026

Screenshot harness is still not reproducible across a full run #326

Open

mokagio mentioned this pull request Aug 21, 2026

Validation only (do not merge): trunk merge for #358 #391

Closed

mokagio mentioned this pull request Aug 21, 2026

Validation only (do not merge): trunk merge for #277 #393

Closed

Navigation Menu

Uh oh!

Add a design-token layer and convert the site header onto it - #277

Add a design-token layer and convert the site header onto it#277
ryanwelcher wants to merge 6 commits into
trunkfrom
ui-polish-phase-1-site-header

ryanwelcher commented Aug 11, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

mokagio commented Aug 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

ryanwelcher commented Aug 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why

Before and after

The content column, on a wide window

Setup wizard

What changes

How to test this

Risks and limitations

Related

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

mokagio commented Aug 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ryanwelcher commented Aug 11, 2026 •

edited

Loading