Skip to content

Lint the window's stylesheet with Stylelint, and write its tokens bare - #610

Merged
zaerl merged 3 commits into
redesign/wpdsfrom
add/stylelint-design-tokens
Oct 4, 2026
Merged

zaerl merged 3 commits into
redesign/wpdsfrom
add/stylelint-design-tokens

Conversation

@zaerl

@zaerl zaerl commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

Why

#609 keeps a colour and an inline style out of the renderer's components. That left the place where all the styling now is, src/renderer/shell.css, checked by nothing: a colour typed there would pass. This closes item 9 of #557 for the stylesheet as well.

What changes

Stylelint over the window's stylesheet, with six rules and no preset:

  • The design system's own three, shipped in @wordpress/theme: a token that is used has to exist, a token is not given a value in the stylesheet, and a token has no fallback value written beside it.
  • Three of Stylelint's core rules: no hex colour, no colour by name, no function given a colour's channels (rgb(), hsl(), oklch(), color() and the like, in any case).

npm run lint and the lint workflow run it after ESLint. tests/unit/stylelint-config.test.cjs runs the configuration over styles that must pass and styles that must not.

The stylesheet's tokens are written bare. All 227 hand-written fallbacks (var(--wpds-…, #707070)) are gone. The design system forbids them because they drift, and six of ours had: a 24px gap beside a 32px token, #cc1818 beside a stroke that is #e0afa7, and four more. None was ever painting, since the tokens' stylesheet is in the same bundle, so nothing changes on screen from those.

New dev dependency: stylelint ^17.16.0. It is the peer the design system's plugins are written for. Nothing of it reaches the packaged app.

Not in this PR: a preset (Stylelint's standard one would bring a few hundred formatting findings to a file written before it, the same reasoning as for Prettier in eslint.config.mjs), and the build plugin that injects generated fallbacks, which the app does not need.

How to test this

Platforms: any. From the repository root, after npm ci:

npm run lint
node --test tests/unit/stylelint-config.test.cjs

Both pass. To see it bite, add .x { color: #cc1818; } to src/renderer/shell.css and run npm run lint: Stylelint reports color-no-hex. I planted ten kinds of style this way (hex, a name, rgb(), RGB(), color(), an unknown token, a fallback, a token given a value, and controls that must pass); each came out as it should.

The config's test is red when any one of its rules is weakened: I switched off or loosened eight settings in turn and it failed each time.

Two things to look at by hand (any platform, the current head):

  1. Do anything that raises a confirmation in the bottom corner, such as saving a patch. It has no drop shadow now; it keeps its border and tint. See Risks.
  2. Apply a patch that conflicts. The conflicting lines are in the same monospace as the terminal and the diff.

What must not have happened: no colour, gap or size anywhere else changed. I checked that mechanically: the old stylesheet with its fallbacks stripped differs from the new one only in the header comment, the shadow and that one font line, and all 55 tokens the file uses are defined in the tokens stylesheet the bundle loads. All 99 journeys pass, including the ones that assert colours against tokens.

Risks and limitations

  • Review: 2 passes, 6 findings fixed here, 1 answered in a comment. Details below.
  • The confirmation's shadow is gone. It was written with --wpds-elevation-md, a token the design system has withdrawn while its elevation model is being defined, so the shadow was only ever the hand-written fallback. The prototype's confirmations have none. If one is wanted back, it has to be a hand-written value with the rule disabled on that line.
  • The conflict lines' font changes from ui-monospace, SFMono-Regular, Menlo to the design system's mono stack, like every other code pane.
  • What the rules do not hold the stylesheet to: a gap, a radius or a font that is not a token passes; so do the system's own colours (Canvas and the like), which nothing here uses.
  • One journey failure I could not reproduce. In one of three full local runs, toasts.spec.js:52 failed; it passed in the other two and 7 times out of 7 with its file run alone. I did not capture its error. The journeys take keyboard focus on a machine in use, which has failed this kind of test before, so I believe that is what it was, and CI will say otherwise if not.
  • npm ci --omit=dev would now install Stylelint's tree, because it is an optional peer of @wordpress/theme. Nothing in the repository installs that way.

Related

Part of #557, after #609.


Review outcome (required — see AGENTS.md)

Pass 1: 4 [fix here] · 1 [follow-up]. Pass 2 (on the fix-up): 1 [fix here] · 1 [follow-up]. 6 fixed, 1 answered in a comment.

  • Review: completed — a separate agent context, read-only; head 72c3814 (rebased since) / base 5b7e1fb; diffed the stylesheet with its fallbacks stripped, checked every token against the bundled tokens file, probed the rules with styles on stdin, read the lockfile diff and what electron-builder packs; 4 [fix here], 1 [follow-up].
    • Fixed: the function rule matched lower case only, so RGB() passed; a TESTING.md sentence said a gap and a font had to be tokens; the config's header leaned on files another pull request added; an unused lint:css script (the follow-up).
    • Answered in the config's comment, with no rule added: the system's own colours pass.
    • One statement in that review was wrong and is not acted on: that the shadow never painted. On the base the declaration carried a fallback, and the fallback painted.
  • Review: completed — a second separate agent context, read-only; head d13ad2c / base 1e9133c; 1 [fix here], 1 [follow-up], both fixed.
    • The stylesheet's header still said a gap, a radius and a font are tokens.
    • Nothing tested the configuration, which is now tests/unit/stylelint-config.test.cjs.
  • Since review: d13ad2c → 3c1edff is those two fixes, the conflict lines' font and four wordings the second review noted; lint, npm test, npm run test:electron and the journeys re-run on it.
Implementation notes
  • Lockfile: 82 packages added, none removed or re-resolved, all from the npm registry and none with an install script. Eight existing entries change from dev to devOptional, because stylelint and postcss are optional peers of @wordpress/theme.
  • The packaged app does not contain Stylelint: the asar listing has no entry for it, and the packaged smoke passes.
  • The lint job is still called eslint, so a required check keeps its name.
  • .stylelintignore keeps the built src/renderer/index.css out, which is every library's styles in one file and is not in Git.
  • Local checks on macOS: npm run lint clean, npm test and npm run test:electron pass (1920 tests), npm run test:e2e 99 pass in two runs of three (see Risks), npm run test:e2e:packaged 15 pass.
Screenshots or recording

Not attached: I have no way to upload images from the command line. The two steps under "Two things to look at by hand" show both visible changes.

🤖 Generated with Claude Code

zaerl and others added 3 commits October 4, 2026 11:28
The lint rules for the renderer's components (#557) left one place where a
colour could still be written by hand: src/renderer/shell.css, where all the
styling now is.

Stylelint, with the design system's own three rules and three of its core
ones, and no preset: a token has to exist, is not set here, has no fallback
value beside it, and a colour is not a hex value, a name or a function given
its channels. `npm run lint` and the lint workflow run it after ESLint.

The 227 fallbacks the stylesheet carried are gone. Four had drifted from
their tokens, and one stood in for a token the design system has withdrawn,
`--wpds-elevation-md`: the shadow under a confirmation was only ever the
fallback. It is removed, as the prototype has none.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…s rules check

Review fix-ups. `RGB()` passed where `rgb()` did not. TESTING.md said a gap
and a font had to be tokens, which nothing checks. The config's header no
longer leans on what another pull request adds, and says what passes on
purpose. The `lint:css` script nothing used is gone.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ld the stylesheet to

Second review pass. Nothing would have noticed a rule that stopped
reporting, and one is given a pattern that only works written exactly so:
tests/unit/stylelint-config.test.cjs runs the configuration over styles that
must pass and styles that must not. The stylesheet's header no longer says a
gap or a font has to be a token, which nothing checks, and the one font
stack still written out where the design system has a token uses it.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 4, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration
  • Configuration used: Repository: WordPress/contributor-toolkit/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 5d76b794-879b-4ce9-b943-a8d52728d283

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@zaerl
zaerl merged commit bda9083 into redesign/wpds Oct 4, 2026
8 checks passed
@zaerl
zaerl deleted the add/stylelint-design-tokens branch October 4, 2026 09:50
zaerl added a commit that referenced this pull request Oct 5, 2026
## Why

#542 replaces the app's interface with the one in the
[prototype](https://github.com/jameskoster/wordpress-contributor-toolkit-prototype),
drawn with the WordPress Design System. It was built in slices on
`redesign/wpds`, so that trunk, which releases are cut from, never held
a half-redesigned window. The window is whole now: the shell, the page
header and details, every card and dialog, and the tray. This pull
request is how the branch reaches trunk.

It lands before the last three sub-issues, on purpose. The settings
dialog (#559) and dark mode (#560) add to a finished window and do not
need a branch to hide in, and the guide (#561) is easier to rewrite
against what is on trunk. They follow as pull requests against trunk.

## What changes

Everything `redesign/wpds` holds; this pull request adds nothing of its
own. Each slice was reviewed in its own pull request against that
branch, and says there how to test it and what it leaves out.

- **The shell** (#555): a sites list, the page area and a footer, in a
larger window. #590
- **The page header and the details** (#556): the site's menu in the
header, and what the checkout is in a sidebar that can be put away.
#594
- **The site's page** (#557): the server and the build watch in the
header and the details
(#595); the ticket
card, the apply card with a preview dialog, and the list of a site's
tickets (#596,
#597,
#599); the
create-site, rename and delete dialogs
(#600,
#601);
confirmations as notices in the window's corner
(#602); the page's
notices, update card and setup checklist
(#605); the
terminal, logs, mail and the review dialogs painted with the design
system's tokens
(#606,
#608); and lint
rules that keep it so
(#609,
#610).
- **The tray** (#558): along the bottom of the window, holding the
terminal, the logs or the mail, with a count of unseen debug.log lines
on the footer
(#612,
#613,
#614,
#615).
- Along the way:
#611 (the site
menu's update item says why it is held), and three that tidy up before
landing: #616 (a
journey's race with the page's clock),
#617 (comments the
finished redesign made untrue) and
#618 (the server's
button by its present name in `AGENTS.md`).

**What a contributor will notice first:** the terminal, the logs and the
mail are no longer on the page but behind three buttons at the foot of
the window; the terminal is dark text on light; the server and the build
watch are started from the header; a patch is previewed in a dialog
before it is applied; and what worked is said in the window's corner.

**Not in this pull request:**

- The settings dialog (#559). Until it, nothing about how the window is
arranged is remembered across restarts: the details, the tray's height,
the apply card's fold.
- Dark mode (#560). The terminal reads its colours once, when it is
made, and will need to follow the theme.
- The user guide and its screenshots (#561), which describe the old
interface. They are published from release tags, so they are not wrong
until a release ships this; #561 is next, ahead of #559 and #560, for
that reason.
- How a mail is drawn, and how the window handles a link. Both are
unchanged and tracked separately.

## How to test this

Platforms: macOS and Windows, both. Buildkite builds signed artifacts
for this pull request; use the build for its current head, and check
that it is: a push to the branch makes earlier builds stale.

The suite, from the repository root, after `npm ci`:

```
npm run lint
npm test
npm run test:e2e
```

113 journeys walk every slice on macOS and Windows on each push. What
they do not do is run a real site: a clone of `wordpress-develop`, an
install, a build and a server are stood in for. The one test that does,
`real-setup.yml`, is run by hand and was run on this branch at `548c020`
(https://github.com/WordPress/contributor-toolkit/actions/runs/37298667056):
a Core site on macOS and on Windows and a Gutenberg site on macOS were
created, installed, built and served, and passed. **The Gutenberg site
on Windows did not finish.** Its first attempt was cancelled after 74
minutes inside the journey, a minute short of the limit that would have
failed it with evidence, so it says nothing about where it was; a second
attempt was still running when this was written and its result is not in
this description. The commits since `548c020` change a journey, comments
and prose.

**A pass by hand, on a real machine** (any platform; Apple Silicon for
the server, which no test on a pull request starts for real). It follows
the order a contributor meets things in:

1. Open the app with no sites. **Create new site**, a Core site, in a
folder with a space in its path. The setup checklist runs its steps; the
terminal's button at the foot of the window shows what each is doing.
2. Let the checklist's last step start the server, and finish
WordPress's own setup in the browser. Back in the app the header says
"Server running" and "Build watching". Stop the server and start it
again: from **Server running** in the header, or with **Stop** and
**Start** beside "Server" in the details.
3. Press the three buttons at the left of the footer in turn. Terminal:
type `help`. Logs: the server's lines, the build watch's, and Debug.log.
Email: where SMTP is listening. Drag the tray's top edge.
4. Link a ticket by its number. The card shows the ticket and its pull
requests and patches.
5. Apply one of them. The preview dialog names the files; **Apply and
rebuild** runs the steps on the card.
6. Edit a file under `src/`, then **Review & submit changes**. The diff
is the edit and nothing else.
7. Create a second site, or open one from before, and move between the
two from the sites list. Each has its own terminal, logs and mail.
8. **Update to latest trunk** from the site's menu.

**What must not have happened:** work lost in a link, an apply, a switch
or an update that did not say so; `node_modules` rebuilt where nothing
changed; a failure said only in a tray that stayed closed; one site's
output in another's tray; anything on the page under the tray.

**What cannot be tested by hand, and what was not:** none of the slices'
manual steps were run by a person. I looked at screenshots and drove the
app through the journeys; everything local was on macOS, with Windows
left to CI. This pass is the first one by hand.

## Risks and limitations

- **It is one commit on trunk**, since the repository allows only squash
merges. The slices' pull requests are its record. `redesign/wpds` should
not be branched from after this: it will have diverged from trunk.
- **Reviewed in slices, not as a whole.** Twenty-two pull requests, all
but a two-word change to `AGENTS.md` reviewed by a separate reviewer
context before they were opened, most in two passes; see below. The
branch was then swept once for what falls between slices, which found
comments and one unread field
(#617) and the
three points that follow.
- **Three questions and two failures still use the system's own
dialogs**: the discards ask through its confirm box, and a path that
could not be copied and a server started before there is a build are
said in its alert. #557 meant to replace them and did so for deletions
only.
- **Some dialogs are repainted, not rebuilt.** The review dialog and its
destinations, the mail dialog and the question an update asks about
loose edits have the design system's colours and type on the old
component library's parts.
- **A real Gutenberg setup on Windows is not shown to pass on this
branch.** See "How to test this". It passed on trunk before the
redesign, in 35 minutes; here it was past 46 on its second attempt.
Whether that is the runner or the branch is not known.
- **The guide describes the old interface** until #561. Do not cut a
release before it.
- **The bundle roughly doubled**
(#590): the sites
list brings `DataViews` and what it needs. Launch time was not measured.
- **Less is always in view.** A build's output, a new line in debug.log
and a caught mail are in a tray that may be closed. A failure opens the
terminal, or the logs if nothing else is open; unseen debug.log lines
are counted on the footer; a mail that arrives shows nowhere until its
tray is opened, which was decided and is not an oversight.
- **Translation is about half done.** The shell, the cards, the tray and
most dialogs go through the translator. Roughly 200 strings in
components and hooks do not yet (the review dialog and its destinations,
the setup checklist, the logs, most of what the hooks say), nor about
150 in the modules they take sentences from. Strings that are new are in
English in the shipped translations until they are translated.
- **Focus is lost in a few places**: when the app swaps what the tray
holds from under it, when a notice with a focused button goes, when a
row's own action removes the row.
- **Some failures are still worded by the main process** ("Error
invoking remote method…"): a failed rename, a failed setup.
- **Things carried over, not made by this:** a start that throws leaves
the server's button dead until a restart; erasing typed input in the
terminal breaks once the line wraps; a line written to debug.log while
it is being attached to can be shown twice.
- **#571 and
#603 will conflict
with trunk after this**, in the mail's journey and helpers and in the
dev server's hook.
- Each slice's own list has more, and says which of its limits were
chosen.

## Related

Closes #557. Closes #558. Part of #542, which stays open for #559, #560
and #561. #555 and #556 are closed already.

---

<details>
<summary>Design decisions and alternatives considered</summary>

- **An integration branch, and one landing.** A window drawn half by the
old view and half by the new one could not be released, and the slices
were too many to hold back one by one behind a flag. Fixes and changes
with no visible effect went to trunk first and were merged across, so
this diff is the redesign and little else.
- **Landing before #559 to #561.** The plan was to land when all seven
sub-issues were in the branch. The remaining three do not change what is
there: two add to it and one documents it. Holding a finished window on
a branch for them only lengthens the time trunk and the branch can drift
apart.
- **What the prototype lacks is kept and redrawn**, and what it has
somewhere else moves to where it puts it. Its labels are adopted. Where
a slice departs from it, the slice says so.

</details>

<details>
<summary>Review outcome (required — see AGENTS.md)</summary>

**No review of the whole diff was run. The record is the slices':
twenty-two pull requests into `redesign/wpds`, each but one reviewed
before it was opened by a separate, read-only reviewer context following
`.github/instructions/code-review.instructions.md`, most in two passes
(the change, then its fix-up), with the counts and the findings in each
pull request's own review outcome. The one is
#618, two words in
a line of prose.**

- **Review:** completed, per slice —
#590,
#594 to
#602,
#605,
#606,
#608 to
#617 (no review was
run for #618);
everything marked to fix in a slice was fixed in that slice, and what
was left is in its Risks.
- **Review:** completed, for leftovers only — a separate agent context,
read-only; `redesign/wpds` at `548c020` against trunk; not a review of
correctness, which the slices had: a search for dead CSS and code,
comments that waited for a slice that has landed, what is left of the
old component library and of the system's dialogs, strings not yet
translatable, and repository docs that name the branch. Nothing found in
the stylesheet, the dependencies or the docs; thirteen comments and one
unread field, fixed in
#617; the rest is
in Risks above.
- **CodeRabbit:** not run. Its check reads "Review skipped: automatic
reviews are disabled" on this pull request and on the slices, so the
slices' pre-PR reviews are the reviews of record, each for the head it
names.
- **Since review:** nothing was added to the branch outside a reviewed
slice, apart from three merges of trunk (`da19ac6`, `3344172`,
`23d46e7`).

</details>

<details>
<summary>Implementation notes</summary>

- `git diff origin/trunk...origin/redesign/wpds`: 148 files, about
14,800 lines added and 3,000 removed; about 6,300 of the added lines are
tests.
- The site's view, which was one component of five thousand lines, is
spread over the four regions the layout has: the header, the cards, the
details and the tray. Decisions are in `src/renderer/*.cjs` with unit
tests; components draw them.
- New for the suite: journeys per region under `tests/e2e/journeys/`,
lint rules of the repository's own (`eslint-rules/`), and Stylelint over
the window's stylesheet.
- The main process changed in one place for the redesign's sake: the
debug.log tail says which lines the file already held
(#614).

</details>

<details>
<summary>Screenshots or recording</summary>

Not attached: I have no way to upload images from the command line. The
prototype shows the design; the signed build shows the app.

</details>

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
@zaerl zaerl added this to the v2.0.0 milestone Oct 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant