CORE-2720: Add a global CSS token file for theme values and sweep migrated CSS onto it

[OpenStaxClaude]

Summary

Follows up on Beth's review comment on #130: "it might be good to move theme variables we want to keep using (like the color palette) to a global css file that it can reference … seems like we could end up with a lot of hardcoded colors, spacing, etc."

She was right, and the evidence was already in the migrated files. Before this PR, the 22 migrated stylesheets contained 86 hardcoded hex literals, nearly all of them palette values re-typed by hand:

• palette.darkRed in four files in two casings; palette.paleRed as both #FBE7EA and #fbe7ea
• palette.white as #ffffff (16×), #fff (4×), and the bare keyword white (2×)
• Copilot's review on CORE-2004: Migrate NavBar components to plain CSS #130 caught three var() fallbacks that had already diverged from the JS values the component was binding. Nothing in CI would have caught them.

What changed

src/theme/theme.css — a :root block projecting the JS theme into --ox--prefixed custom properties (palette, colors.link, zIndex, padding.navbar). Prefixed so it can't collide with a consuming app's variables. Generated, not written: src/theme/themeCss.ts owns the projection and scripts/generate-theme-css.bash writes the file, as the first step of scripts/build.bash (and so of publish.bash, via build:clean). It is committed as well as generated, because jest and ladle read src/ directly and CI runs lint/test rather than build — the build covers the published artifact, the spec below covers everything else.

src/theme/tokens.spec.ts — the part that makes this stick. Fails the build if:

1. the committed theme.css is not what the generator produces from the JS theme
2. a component stylesheet writes a colour literal that duplicates a theme value
3. a component stylesheet introduces a colour that is neither a theme value nor on the KNOWN_OFF_PALETTE allowlist
4. a component stylesheet reads an --ox-* token that does not exist

Adding a new colour is therefore deliberate: put it in palette.ts, or in the allowlist with a reason.

The colour check parses declarations rather than grepping for hex, so it covers every syntax a colour can be written in — hex, rgb()/hsl()/oklch()/color(), and bare named colours wherever they appear, shorthands and gradient stops included. Functions that merely contain colours (var(), color-mix(), the gradients) are descended into rather than treated as literals. A translucent colour passes when its opaque channels are a theme value — rgba(0, 0, 0, 0.2) is black at 20% and has no token form — which still refuses a new hue smuggled in through rgba(). The checker has its own tests (40 cases, what it must flag and what it must leave alone), so the guarantee above is tested rather than asserted.

Four orphan colours moved into palette.ts. #be3c08, #b03808, #646464 and #4c4c4c were string literals in theme/buttons.ts with nothing recording that they're hover/active variants of palette.orange and palette.darkGray. They're now orangeHover, orangeActive, darkGrayHover, darkGrayActive, grouped under a // button variants comment — purely additive to the published palette. (Roy renamed these from my original darkOrange/darkerOrange/mediumGray/darkerGray in cbf780d; his names say what the colours are for rather than just where they sit on a lightness ramp, which is the better call.)

All 22 stylesheets swept onto tokens, including inside var() fallbacks.

Static colours no longer travel through React inline styles. The --component-* override hooks stay exactly as they were — only their defaults moved to the CSS side:

/* before: JS bound '--tabs-active-border-color': palette.darkGreen on every render */
.tabs [role="tab"] { border-color: var(--tabs-active-border-color, var(--ox-color-dark-green)); }

Genuinely dynamic bindings stay in JS: button/checkbox variant lookups, navbar height and maxWidth, disabled opacity, the dropdown caret colour.

README gained a Styling section documenting the pattern, so the remaining ~29 files are migrated against tokens rather than against hand-copied literals.

Two things worth a closer look

1. A latent bug fixed in passing. --button-shadow is shared by Button.css and DropdownMenu.css, and the JS binds palette.black for all three variants. Button's fallback had drifted to #424242 (neutralDarker) while DropdownMenu's was #000000. Unreachable today because the JS always sets the variable, so this is a zero-visual-change fix — but the two files would have rendered the same variant differently if it were ever reached.

2. One edge-case behaviour change. Removing an inline default changes who wins the cascade. If a consumer set, say, --tabs-border-color on an ancestor element via their own stylesheet, the inline default used to beat it; now the ancestor's value applies. That is arguably the fix rather than the regression, and passing style={{'--tabs-border-color': ...}} to the component itself — the documented override path, covered by specs — is unaffected. Flagging it because it is the only way this refactor is observable from outside.

Verification

• npm test — 365 passed, 40 suites, 115 snapshots. 45 snapshots regenerated; every diff is an inline style attribute losing a static custom property. No DOM structure changes.
• npm run typecheck and eslint over all changed files — clean.
• npm run build — succeeds; theme.css lands in both dist/esm/theme/ and dist/cjs/theme/, and the side-effect imports survive as import '../theme/theme.css' (ESM) and require("../theme/theme.css") (CJS).
• Specs that asserted the inline default were rewritten to assert what actually matters — that the override still works — with a comment pointing at tokens.spec.ts as what now guards the defaults.
• Ladle pixel diff — all 105 stories captured on this branch and on main with Playwright at 1280×800, animations paused, then compared. 100/105 pixel-identical. The 5 that differ (loader--default, loader--large, error-modal--default, forms--data-references-in-namespaces, sidebar-nav--using-body-portal) differ by the same amount when main is captured twice and diffed against itself, so they are animation-frame and timestamp nondeterminism, not this change. That control run is the reason I trust the other 100.

A pre-existing flake, now fixed upstream. ProfileMenu › matches snapshot with user initials failed 3/3 runs with a cold jest cache, on data-focus-visible / data-focused being absent from the trigger — react-aria's focus state is timing-sensitive and the snapshot captured it. I first hit it when jest -u stripped those attributes and reddened a push (restored in 542b66e3), and originally wrote it up as an environment quirk of my machine; that was wrong, it reproduced on main independently of this branch.

#138 has since landed on main and replaces that snapshot with targeted assertions, which is the proper fix. It is not in this stack yet — see the note below — so a cold-cache run here still hits it.

Plumbing note

scripts/build.bash rsyncs .css 1:1 with no CSS bundler, so a CSS @import would be at the mercy of each consumer's resolver. Component .tsx files import theme.css alongside their own stylesheet instead — same pattern already in use, covered by the existing sideEffects entry, and no action needed from consuming apps.

Review follow-ups

Both threads on the first review are addressed in b9657d7, which touches only the theme plumbing — no component CSS or snapshots changed, so the pixel-diff evidence above still holds.

• Roy — generate theme.css from the JS theme. Done, as described above. Removes the class of mistake rather than detecting it: you can no longer add a palette entry and forget the CSS. The one deliberate loss is the per-token trailing comments (/* darkGray, hover */), which would have needed either scraping palette.ts source or a second copy of the notes; they remain on the palette entries themselves.
• Copilot — non-hex colour syntaxes bypassed the check. Correct, and the README claimed a guarantee the code didn't provide. Fixed by the declaration parser above. #00000033 came off the allowlist as a result — it now passes on the alpha rule instead of by exception, leaving just the two genuinely off-palette greys.

Second review, addressed in f12703b:

• Roy — "it would have to be part of the build process." build.bash now regenerates theme.css before either tsc pass and before the rsync into dist/. Verified by corrupting the file: spec fails, npm run build restores it, both dist trees get the 48 tokens. ~1.7s on a build that already runs tsc twice.
• Roy — "or have both files generated from some raw data file." Considered and not taken, with reasoning on the thread: theme.ts isn't pure data (zIndex is computed, defaultFocusOutline is a CSS fragment), so a data file would split the source of truth rather than centralise it; and palette.ts is published API whose as const literal types and per-entry comments consumers rely on. It is a cheap change to make later if the palette ever needs a non-TS consumer — themeCss.ts is the only thing that would move.

Third review, addressed in 652ed43:

• Copilot — as string casts on describeColor(...).hex. Right, and the more dangerous of the two, because the cast is compile-time only so nothing would have thrown. An unresolvable theme colour would have entered themeValues under a null key and silently stopped being recognised — every stylesheet using it would then report it as off-palette, a failure a long way from its cause. The map now filters unresolvable entries out and a spec asserts there are none. Verified by pointing palette.red at oklch(): the suite fails naming --ox-color-red instead of misbehaving downstream.
• Copilot (suppressed) — dead tokenised flag. Also right, as a consequence of Roy removing the var() short-circuit in 635f854. Field and guard removed; a no-op, since an unset optional was always falsy. Added a case pinning the behaviour that replaced it — rgba(var(--channels), 0.2) is asserted to be flagged rather than skipped, matching the reasoning in Roy's comment.

Fourth round — two defects I found and reported myself, fixed in 08ee93a. Both surfaced while answering a review question on the rex-web sibling (#3133), where Copilot raised them; the checker there is the same design. Tests were written failing first, so both are demonstrated rather than asserted.

• parseHex validated the expanded length, not the digits. #ggg expanded to gggggg, passed the length test and came back as a colour — defeating unresolvableThemeColors, whose whole job is to stop a malformed palette value entering themeValues under a key nothing can match. Now checked against the hex grammar first.
• The declaration walk discarded the property, so any identifier that happens to be a CSS named colour was read as one: animation-name: red reported as off-palette, font-family: white as duplicating --ox-color-white with a suggested fix that would break the declaration. The walk returns the property now, and a bare identifier is only read as a colour where one can go. Only bare identifiers are gated — hex and the colour functions stay in scope everywhere, including animation-name: #d5d5d5. Four cases pin that the gate didn't weaken real detection.

Both were latent — nothing in this repo's CSS triggers either — but the second would have bitten the first component that animates something named after a colour.

Stacked on #137

Base is CORE-2710-compose-render-props-style, per Roy's request. Rebased rather than merged, so history stays linear.

main
└── #137  CORE-2710
    ├── #141  CORE-2006 (unchanged)
    └── #143  CORE-2720  ← this PR
        └── #140  CORE-2005

The two overlapping files resolved in opposite directions:

• Tree/TreeCheckbox.tsx merged in substance — CORE-2710: Compose render-callback style in react-aria-components wrappers #137's composeRenderProps wrapper kept, carrying only the bindings this PR leaves in JS. All of CORE-2710: Compose render-callback style in react-aria-components wrappers #137's TreeCheckbox tests pass untouched.
• NavBarMenuButtons.tsx genuinely collides. This PR removes every custom property that component set, so nothing is left for composeRenderProps to merge style with; style is no longer destructured and passes through in ...props, where react-aria handles both forms. Three of CORE-2710: Compose render-callback style in react-aria-components wrappers #137's tests asserted the inline defaults this PR deletes (expect(...).toBeTruthy() on --navbar-menu-item-* / --navbar-popover-border-color). Those assertions are removed and the rest of each test kept, so the callback-vs-object contract is still covered and tokens.spec.ts guards the defaults. This is me editing tests from another PR — worth a look, and trivially reversible if the preference is to keep those variables in JS.

#140 was based on this branch, so it was rebased onto the new head (3973291 → 6194fd1) and its base restored; its content is unchanged and 380 tests pass there. GitHub blocked the base change until the auto-created stack #144 was dissolved with gh stack unstack; #137/#141's stack was not touched.

Keeping up with the base

#137 was force-pushed onto a newer main, so this branch was rebased onto its new head (c466a0af → 7f8bdfb3).

#139 landed on main and deletes HelpMenu/__snapshots__/index.spec.tsx.snap, which this PR edited because the sweep changed values inside it — a modify/delete conflict, resolved by taking the deletion. That is the ordering outcome predicted when this was planned: with #139 in first, this part of the diff disappears instead of being churn thrown away later. One fewer changed file here.

#138 is queued to do the same to the ProfileMenu snapshot. #137's base is c6ed1cd8 (#139's merge), one commit behind main, so it does not yet include #138. This PR still edits that snapshot — correctly, since the file is live at its current base — and will hit the identical conflict once #137 catches up, resolved the same way (take the deletion, which also makes 542b66e3 moot). Rebasing #137 onto current main clears that and the flaky test with it; not done here because #141 is also based on #137 and would have to move too.

Sequencing

This wants to land ahead of the remaining migration subtasks. Done now, the remaining ~29 files are written against tokens at no extra cost; done at the end, all 51 get re-touched in a second pass of pure find-and-replace noise. #140 (CORE-2005) has already adopted the tokens and is stacked on this PR. #141 (CORE-2006) sits alongside this PR on #137 and will need a small follow-up to adopt them.

Related

• Jira: CORE-2720 (subtask of CORE-1777)

🤖 Generated with Claude Code

[OpenStaxClaude]

Stacked onto #137 as asked. Base is now CORE-2710-compose-render-props-style; the branch was rebased, not merged, so the history stays linear and your three commits (df59d0e, 635f854, cbf780d) are preserved with your authorship.

main
└── #137  CORE-2710
    ├── #141  CORE-2006 (unchanged)
    └── #143  CORE-2720  ← this PR
        └── #140  CORE-2005

Pre-rebase SHA was 08ee93a if you want to unwind it.

The overlap was real, and one part of it needed a decision

NavBarMenuButtons.tsx and Tree/TreeCheckbox.tsx conflicted, in opposite ways.

TreeCheckbox merged cleanly in substance. #137 wraps the style object in composeRenderProps; this PR removes the static entries from that object. Result keeps both: your wrapper, carrying only the bindings that genuinely vary at runtime. All of #137's TreeCheckbox tests pass untouched.

NavBarMenuButtons is where they genuinely collide. This PR removes every custom property that component set, so after the sweep there is nothing left for composeRenderProps to merge style with. style is no longer destructured at all — it passes through in ...props, and react-aria handles both the object and callback forms itself. The bug #137 fixes can't occur here anymore, because its cause was us overwriting the caller's style, which we no longer do.

That made three of #137's new tests fail — the ones asserting the component still sets its own variables inline:

● NavBarMenuItem › merges a render-callback style
● NavBarMenuItem › keeps merging an object style, caller last
● NavBarPopover › merges a render-callback style

Each failed on an expect(...getPropertyValue("--navbar-menu-item-hover-bg")).toBeTruthy()-style assertion. I removed those assertions and kept everything else in those tests — the render-callback and object forms are still asserted to reach the element, which is the contract #137 exists to protect, and tokens.spec.ts now guards the defaults centrally. There are comments at each site so it doesn't read as coverage quietly dropped.

Flagging it prominently because it is me editing tests from your PR. If you'd rather keep those assertions and have the NavBar variables stay in JS, say so and I'll reverse that part — it's a one-file change.

#140 came along

Its base was this branch, so rewriting my history would have left it showing a garbage diff. I rebased it onto the new head and force-pushed (3973291 → 6194fd1); its base is back to CORE-2720-global-css-theme-tokens and its own commits, including your a4546195, are intact. Nothing about its content changed.

Worth knowing: GitHub refused the base change outright — "Cannot change the base branch because the pull request is part of a stack" — because #143 and #140 had been auto-grouped into stack #144. I dissolved that grouping with gh stack unstack 144, retargeted, then rebuilt it. Both PRs stayed open throughout and no other stack was touched (#137/#141's stack #142 is untouched).

Verification

• CORE-2720: Add a global CSS token file for theme values and sweep migrated CSS onto it #143: 362 tests, 40 suites, 116 snapshots — all pass. typecheck, lint, build:clean clean; both dist trees at 48 tokens.
• CORE-2005: Migrate navigation components (SidebarNav, ButtonNav, Pagination) to plain CSS #140 on the new base: 380 tests pass, typecheck and lint clean.
• No snapshot regeneration was needed on either — the conflict resolutions produce identical rendered output.

[RoyEJohnson]

Resolve the conflicts

[OpenStaxClaude]

Resolved — MERGEABLE again.

The cause was that #137 was force-pushed (e18e5ec0 → 590bda0d) onto a newer main, so this branch was sitting on a commit that no longer existed in its base. Rebased onto the new head; nothing here was merged, so history stays linear and your three commits keep their authorship.

c466a0af → 7f8bdfb3. Pre-rebase SHA is on backup/pre-137-refresh-143 if you want to compare.

The one real conflict

#139 landed on main and deletes HelpMenu/__snapshots__/index.spec.tsx.snap — the file this PR edits, because the token sweep changed values inside it. Classic modify/delete.

Resolved by taking the deletion. This is the good outcome I'd flagged when we discussed ordering: with #139 in first, that part of this PR's diff simply disappears rather than being churn that gets thrown away. #139's replacement spec asserts behaviour directly and touches no styles, so there was nothing to carry over. Net effect here is one fewer changed file.

Everything else replayed clean.

One thing you should know before merging the stack

#137 is one commit behind main — based on c6ed1cd8 (#139's merge) rather than ed2e43f5a, so it doesn't yet include #138, which deletes ProfileMenu/__snapshots__/index.spec.tsx.snap.

Two consequences:

1. The same modify/delete conflict is queued up. This PR still edits that ProfileMenu snapshot, correctly, because the file is live at its current base. The moment CORE-2710: Compose render-callback style in react-aria-components wrappers #137 picks up CORE-2715: replace flaky ProfileMenu snapshot with targeted assertions #138, it'll conflict exactly like HelpMenu just did, and resolves the same way — take the deletion, which also makes 542b66e3 ("Restore react-aria focus attributes…") moot.
2. The flaky test is still in this stack. A cold-cache full run here still fails ProfileMenu › matches snapshot with user initials — the flake I reported earlier, and the one CORE-2715: replace flaky ProfileMenu snapshot with targeted assertions #138 exists to fix. It passes warm and on CI, but it's noise for anyone running the suite locally on this branch.

Rebasing #137 onto current main clears both. I haven't done it: #141 is also based on #137, so moving it drags that PR along too, and that's your call on merge order rather than mine to make unasked. Say the word and I'll do #137, #141, #143 and #140 in one pass.

Verification

• CORE-2720: Add a global CSS token file for theme values and sweep migrated CSS onto it #143: 365 tests, 40 suites, 115 snapshots pass (warm). typecheck, lint, build:clean clean; both dist trees at 48 tokens.
• CORE-2005: Migrate navigation components (SidebarNav, ButtonNav, Pagination) to plain CSS #140 rebased along with it (6194fd1 → f1f4ad9), content unchanged, 383 tests pass, your a4546195 intact.
• Both PRs report MERGEABLE.

[OpenStaxClaude]

Rebased onto the current #137 head to clear the conflicts that were blocking this and #140.

This branch was still carrying a stale copy of #137's commit (590bda0d) from before #137 was force-pushed onto a newer main — one that now includes #138 and #139. Those two deleted the ProfileMenu and HelpMenu snapshots that this PR's token sweep was editing, which is where the modify/delete conflicts came from.

Replayed the ten commits here onto the real #137, dropping the stale duplicate:

• ProfileMenu/__snapshots__/index.spec.tsx.snap — took the deletion from CORE-2715: replace flaky ProfileMenu snapshot with targeted assertions #138. Verified ProfileMenu/index.spec.tsx has no toMatchSnapshot calls on the new base, so keeping the file would have restored a snapshot nothing asserts.
• 542b66e3 "Restore react-aria focus attributes in the ProfileMenu snapshot" — dropped. It's one file, two lines, entirely inside that snapshot, so it has no purpose once the file is gone.
• The NavBarMenuButtons / TreeCheckbox conflicts the merge preview showed didn't appear during the rebase — f9cf8331 "reconcile the CORE-2710 style tests with the token sweep" already covers that interplay and applied cleanly against the real CORE-2710: Compose render-callback style in react-aria-components wrappers #137.

7f8bdfb → f9cf8331, 10 commits → 9, no content change beyond the two resolutions above.

368 tests pass on the new base (40 suites, no failures), typecheck and lint clean, checks green, and this now reports MERGEABLE/CLEAN. #140 has been rebased to follow. Old head was 7f8bdfb36 if you want to compare.