Repository navigation
CORE-2720: Swap hand-copied colour literals for theme tokens - #165
Merged
Merged
Conversation
This was referenced Oct 5, 2026
Contributor
Author
|
@bethshook I had Claude break up #146 into three PRs that are somewhat more focused so that you didn't have 55 files to review all together. It didn't occur to me until after that maybe you were working your way through the files. If so, I'm really sorry I threw your process off. I was trying to make it better. Also, I can farm some of the reviewing out to others. |
Contributor
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The substitutions and required imports consistently preserve the existing defaults while extending enforcement to all stylesheets.
Review effort: Balanced
Findings: None
What changed in this PR
Replaces duplicated component color literals with generated theme tokens while preserving existing override hooks and defaults.
Changes:
- Migrates 18 stylesheets to
--ox-*tokens. - Adds required
theme.cssimports. - Enforces duplicate-literal checks across all stylesheets.
| File | Description |
|---|---|
README.md |
Removes obsolete sweep exemption documentation. |
src/theme/tokens.spec.ts |
Removes PENDING_SWEEP and checks every stylesheet. |
src/components/Tree/TreeCheckbox.tsx |
Imports theme tokens for shared checkbox CSS. |
src/components/Tooltip.tsx |
Imports theme tokens. |
src/components/Tooltip.css |
Replaces palette literals with tokens. |
src/components/ToastContainer.tsx |
Imports theme tokens. |
src/components/ToastContainer.css |
Tokenizes the toast z-index. |
src/components/Toast.tsx |
Imports theme tokens. |
src/components/Toast.css |
Tokenizes toast colors. |
src/components/Text.tsx |
Imports theme tokens. |
src/components/Text.css |
Tokenizes text colors. |
src/components/Tabs.tsx |
Imports theme tokens. |
src/components/Tabs.css |
Tokenizes tab colors. |
src/components/Radio.tsx |
Imports theme tokens. |
src/components/Radio.css |
Tokenizes radio colors. |
src/components/Overlay.tsx |
Imports theme tokens. |
src/components/Overlay.css |
Tokenizes overlay text color. |
src/components/NavBarMenuButtons.tsx |
Imports theme tokens. |
src/components/NavBarMenuButtons.css |
Tokenizes menu colors. |
src/components/NavBarButton.tsx |
Adds the retained theme import. |
src/components/NavBar.tsx |
Imports theme tokens. |
src/components/NavBar.css |
Tokenizes navbar colors, spacing, and z-index. |
src/components/NavBar.stories.tsx |
Imports tokens for story CSS. |
src/components/NavBar.stories.css |
Tokenizes the story color. |
src/components/Modal.tsx |
Imports theme tokens. |
src/components/Modal.css |
Tokenizes modal colors and z-index. |
src/components/forms/uncontrolled/inputTypes.tsx |
Imports theme tokens. |
src/components/forms/uncontrolled/inputTypes.css |
Tokenizes validation text color. |
src/components/forms/uncontrolled/formComponents.css |
Documents an allowed off-palette color. |
src/components/DropdownMenu.tsx |
Imports theme tokens. |
src/components/DropdownMenu.css |
Tokenizes dropdown colors. |
src/components/CloseModalButton.tsx |
Imports theme tokens. |
src/components/CloseModalButton.css |
Tokenizes close-button colors. |
src/components/Checkbox/Checkbox.tsx |
Imports theme tokens. |
src/components/Checkbox/Checkbox.css |
Tokenizes checkbox colors. |
src/components/ButtonBar.tsx |
Imports theme tokens. |
src/components/ButtonBar.css |
Tokenizes button-bar colors. |
src/components/Button.tsx |
Imports theme tokens. |
src/components/Button.css |
Tokenizes button colors. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
RoyEJohnson
marked this pull request as ready for review
October 6, 2026 00:24
RoyEJohnson
force-pushed
the
CORE-2720-sweep-literals-to-tokens
branch
from
October 7, 2026 14:52
8b1688b to
5172255
Compare
RoyEJohnson
force-pushed
the
CORE-2720-sweep-literals-to-tokens
branch
from
October 7, 2026 15:19
5172255 to
e8bf9c6
Compare
Contributor
|
All good! If I was working through it then I've already forgotten - but I usually try to finish in one or two sittings. |
bethshook
approved these changes
Oct 8, 2026
The stylesheets migrated before the token file existed still carried literals copied from the palette. They now read the --ox-* tokens, with the component override hooks unchanged: only their fallback moved, for example `var(--tabs-border-color, var(--ox-color-pale))`. Each module whose stylesheet now reads a token imports theme.css, which the token import rule requires. PENDING_SWEEP is emptied, so the duplicate-literal check covers every stylesheet in src/. Genuinely off-palette values (#ccc, #ddd) stay on the allowlist with a reason. Nothing changes on screen and no snapshot changes: the components' JS still sets the same values inline. Dropping those inline defaults is a separate change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
RoyEJohnson
force-pushed
the
CORE-2720-sweep-literals-to-tokens
branch
from
October 8, 2026 19:22
e8bf9c6 to
2f48f48
Compare
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This branch was successfully deployed
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.
Jira: CORE-2720
1 of 3. The sweep that was one PR (#143) is now three, so each can be reviewed and merged on its own. All three are cut from
mainand are independent:Merged together, in any order, they produce exactly the tree #143 had (tree
65a8d3f).What this does
The stylesheets migrated before the token file existed still carried literals copied from the palette. They now read the
--ox-*tokens. The component override hooks are unchanged; only their fallback moved:theme.css, as the token import rule requires.PENDING_SWEEPis emptied, so the duplicate-literal check covers every stylesheet insrc/.#ccc,#ddd) stay on the allowlist with a reason. Snapping them to the nearest palette entry is a design decision, not a refactor.Nothing changes
The components' JS still sets the same values inline, so nothing renders differently and no snapshot changes. Dropping those inline defaults is the other two PRs, and that is where the one behaviour change is.
Verification
801 tests, typecheck and lint pass on this branch alone, with all 114 snapshots unchanged. The Ladle pixel diff that covered the combined change (105 stories, 100 identical; the other 5 differ by the same amount when
mainis compared with itself) is in the original #143 description, preserved in a comment there. I have not re-run it per PR.Heads-up for other open PRs
Once this lands, the duplicate-literal check applies to stylesheets that were exempt. #163 adds a
#026AA1fallback toCheckbox.css, which will fail it;var(--ox-color-medium-blue)fixes it. Any PR that adds a colour literal to one of these stylesheets needs the same.Notes
NavBarButton.tsxgains atheme.cssimport that its stylesheet does not need, becauseNavBarButton.cssreads no tokens. It was in #143 and is kept so the three PRs reproduce it exactly. Dropping it is a one-line change.🤖 Generated with Claude Code