feat(tokens): adopt the design system colour system - #8594
talissoncosta wants to merge 16 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 📝 WalkthroughWalkthroughThe change revises palette values and theme colour mappings in JSON and SCSS. It adds token groups and updates the generator to emit their CSS custom properties. Fallback colours and Storybook palette displays are updated, and the SCSS primitive palette file is removed. Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: 🟡 Moderate · up to Several syntax colours may be difficult to read in the light theme. Correct their contrast before merging. 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. Comment |
2cd6cdc to
7dd88c7
Compare
Dragos's Primary scale and our purple are the same ramp: each of his six
values sits within 0.007 lightness of one of our eleven steps. So his values
go onto our steps rather than beside them as a second scale, which would
have given six colours two names each.
purple-50 unchanged, already his value
purple-100 #e8dbff -> #e7e1f4 greyer, chroma 0.050 -> 0.026
purple-200 #d4bcff -> #d4beff imperceptible
purple-400 #906af6 -> #9168fd imperceptible
purple-600 unchanged, already his value
purple-900 #2a2054 -> #1a0a78 muted plum to saturated indigo
300, 500, 700, 800 and 950 are steps he has not picked, so they stay as they
are. That is where the action hover and pressed states live, which his scale
cannot express: it goes from #6837FC straight to #1A0A78 with nothing between.
The two steps that change visibly, 100 and 900, have no references and back no
semantic token. 400 backs six semantic tokens, so their values move with it;
leaving them behind would silently unlink them from the ramp, since the
generator resolves a primitive reference by matching the hex.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Four of Dragos's eight Neutrals are already our exact values: slate-0, 50,
500 and 600. Three more sit within 0.01 lightness of one of our steps, so
they move onto them.
slate-100 #eff1f4 -> #f3f4f5
slate-200 #e0e3e9 -> #e1e2eb
slate-950 #101628 -> #0e1629
Each backs one semantic token, whose value moves with it so the reference
survives; the generator matches a primitive by hex, so leaving the semantic
value behind would turn var(--slate-100) into a hardcoded colour.
His #BFC0C5 has no step of ours to land on, so it gets its own at slate-250.
It sits at L 0.808, almost exactly between slate-200 at 0.915 and slate-300
at 0.716, and the ramp already carries a half-step at slate-850.
Neither neighbour could take it. slate-300 backs six tokens and does opposite
jobs per theme: dark secondary text would improve from 7.16 to 9.90, while
light disabled text would drop from 2.51 to 1.82 on white. slate-200 backs
surface-emphasis, which is not a panel but nine bar tracks and chips; there
the swap helps the track read against the page, 1.29 to 1.82, and hurts it
against its own fill, 4.60 to 3.27.
So slate-250 lands with nothing referencing it, which is deliberate. It is a
value Dragos has published rather than a step we invented, and it waits for
him to name a role for it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
His danger/900 #7A0E18 and our red-900 #701116 are the same step: 0.019 apart in lightness, 0.8 degrees in hue. red-900 backs no token and is referenced nowhere, so it takes his value. His other two danger steps do not follow. #E61B26 is already red-600 exactly, so there is nothing to do. #FFEDDB cannot join the ramp: it sits at hue 67, a peach, while his own danger/500 sits at 26. Putting it on red-50 would pull our red 44 degrees toward orange to accommodate a value that is not red in his own scale either. It is worth asking him whether that is deliberate. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Our success colour is a teal at hue 178; his Supporting success is a green at
162. Three of his values anchor the ramp exactly, placed by lightness:
green-50 #eef9f6 -> #f0fff2
green-400 #56ccad -> #6ad0a1
green-900 #0c3a3b -> #1b392b
The other eight keep their lightness and chroma and move only in hue,
interpolated between those anchors. So the ramp is ours and the anchors are
his, not the other way round.
It also fixes a fault of our own. The old ramp spread 25 degrees of hue with a
20 degree jump between 500 and 600, which is why our teal turned bluer as it
darkened. His spread is 13, so the rebuilt ramp is more consistent than the one
it replaces.
Contrast moves by less than 0.1 on every step that backs a token, in both
themes, so this is a hue decision rather than an accessibility one. Semantic
and chart values move with the primitives, including the one alpha form behind
surface-success, so nothing silently unlinks from the ramp.
chart-3 and chart-8 are drawn from this ramp and move with it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
His Supporting warning is a yellow, and our gold is the ramp that can hold it.
Three anchors, placed by lightness:
gold-100 #fdf6e0 -> #fff7cd
gold-600 #e5c55f -> #ffbc05
gold-950 #64511e -> #744800
The rest keep their lightness and take hue and chroma interpolated between the
anchors, so the ramp is ours and the anchors are his.
Gold was the right host because it is nearly free: code.builtin is its only
consumer, at gold-400 in dark and gold-700 in light. Contrast there moves from
1.88 to 1.91 in light and not at all in dark.
This does not make his warning the app's warning. text-warning, border-warning
and icon-warning still point at orange, and moving them is a separate decision
with a visible result, since orange also backs chart-4, chart-9 and
code.variable.
Worth raising with him first: his warning is his least consistent ramp, 28
degrees of hue against 7 for our gold and 13 for our orange. Its 900 at hue 70
is nearer our orange than our gold, so the scale spans both. Whether #FFBC05 is
a deliberate move from orange to yellow should come from him rather than be
inferred from a variables panel.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
His Supporting set does include info, contrary to the dark-mode mapping sheet,
which lists only success, warning and danger. The Supporting colours page has
all four. Three anchors, placed by lightness:
blue-100 #d6eef5 -> #dff3ff
blue-500 #0aaddf -> #0fa5fc
blue-900 #094456 -> #023078
The rest keep their lightness and take hue and chroma interpolated between the
anchors. Our blue was a cyan at hue 217 to 228; his info is a true blue at 234
to 260, so the ramp turns bluer as it darkens rather than staying cyan.
Contrast moves by at most 0.2 on the steps that back tokens, in both themes.
Two things this does not fix. text-info in light mode is blue-500 on white at
2.61, and 2.69 after; it fails 4.5 either way and wants its own change rather
than a ramp move. And his info has no dark-mode value, since the mapping sheet
omits the group entirely.
chart-1 and chart-6 are drawn from this ramp and move with it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
….json web/styles/_primitives.scss was a second copy of the palette that nothing regenerates, and it had drifted badly: 41 of its 80 values disagreed with tokens.json once the ramps were re-anchored on the design system. Nothing imported it as SCSS. Its only consumer was the Storybook palette page, which raw-loaded and parsed it, so that page showed the old colours while the app shipped the new ones. It now reads tokens.json, the same source the CSS custom properties are generated from, so it cannot drift again. The Storybook manager palette keeps its literals, since the manager runs outside the app and cannot read CSS variables, but its two stale slate values are corrected and the comment points at the real source. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
7dd88c7 to
1a88143
Compare
The four dark status surfaces resolved darker than the page, 1.04 to 1.05, so a tinted panel looked like a cut-out. Each state now takes the 100 and 900 of its own ramp, a surface and its dark counterpart. Three status text values move so each still clears 4.5 on its own tint as well as on the page. That also fixes text-info at 2.69 on white, which predates this branch. Dark surfaces now clear the page by 1.43 to 2.29. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Warning takes his Supporting yellow: gold-600 for border and icon, gold-950 for text, gold-100 and gold-950 for the surface. It sits 60.9 degrees from danger in hue where our orange sat 39.1, so the two are easier to tell apart, and warning text improves in both themes, 4.85 to 7.87 on white and 8.83 to 10.68 on the dark page. It is weaker as a border on white, 2.04 to 1.69, though both fail the 3:1 for non-text graphics. Orange keeps chart-4, chart-9 and code.variable. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
His field-error design settles what the banner left open: the banner pairs 900
text with a saturated icon, but the field error has no icon, just a 500 border
and 900 helper text. So 500 is the graphic and 900 is the text wherever status
appears.
text-danger light #bb1720 -> red-900
text-success light #35795a -> green-900
text-info light #0767b9 -> blue-900
Contrast improves from 5.21-6.45 to 11.03-12.60 on white, and all three still
clear 4.5 on their own tint. border-danger and icon-danger also take his 500.
Success does not follow: his success/500 is 1.88 on white, under the 3:1 for a
non-text graphic, so its border keeps green-500.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
red-100 becomes his danger/100 #FFEDDB, which surface-danger already points at, completing his Supporting set at twelve of twelve. It sits at hue 67 where the rest of our red is 21 to 26, but his own ramp has the same break, 43.7 degrees against 12.8, 26.0 and 28.2 for the other three, so the warmth is his intent. red-50 and red-200 are unused and nothing renders these steps beside each other, so the jump shows only on the palette page. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1a88143 to
2ffd023
Compare
slate-1000 becomes his Surface/Dark #080C17. Surface is a separate collection from Neutrals in his system, and #080C17 is darker than every step of that ramp, so repointing slate-1000 rather than adding a step keeps his Neutrals complete at eight of eight. The four shadows and the two light overlays derive from it. In light mode nothing reaches the 2.3 just-noticeable threshold; in dark mode the three heavier shadows land between 3.43 and 4.68 across a wide blur. surface-default stays on slate-950. Moving it to #080C17 while body.dark still paints #101628 from the legacy $bg-dark500 made every input darker than the page behind it, which is the same hole effect this branch set out to fix. The page and the surfaces that sit on it have to move together, so that goes with the change that migrates body.dark onto the token. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2ffd023 to
826825b
Compare
His Primary is addressable by his own step numbers, with light and dark
values, so a design that names Primary/600 has something to map onto.
--primary-50 100 300 400 500 600 900
Dark mirrors light around 400, exactly as his light-to-dark sheet does:
900 pairs with 50, 600 with 100, 500 with 300, and 400 holds. Each step
resolves to a purple primitive rather than repeating a hex, so there is
still one palette.
purple-700 moves from #4E25DB to his Primary/600 #4F28D8 to complete the
mapping, a dE of 2.77 affecting surface-action-hover in light and
surface-action-active in dark.
No utilities. A .bg-primary-500 in a component would bypass the semantic
layer and stop following whatever that role is later defined as, and nothing
needs class-level access yet; var(--primary-500) already works in component
SCSS. They can be added when there is a real consumer.
Two corrections to earlier commits on this branch. Primary/600 is a real step
in his file, not one I invented: his light-to-dark sheet prints #6837FC in
that slot, which is why it read as empty. And his step numbers are 50 to 900
on a seven-step scale, not the eleven ours uses.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
His Neutrals addressable by his own step numbers, with light and dark values.
--neutral-0 50 100 300 400 500 600 900
Singular, because one token names one colour, and it keeps our own groups
reading the same way: --primary-500, --neutral-500. His collection is
"Neutrals"; the mapping to --neutral- is trivial for anyone reading a design.
Dark mirrors light: 900 pairs with 0, 600 with 50, 500 with 100, 400 with
300. Each step resolves to a slate primitive rather than repeating a hex.
This gives slate-250 its first consumer. It was added for his #BFC0C5, which
had no step of ours to land on, and has sat unreferenced since.
The step numbers come from the v3.0 design system page: 0, 50, 100, 300, 400,
500, 600, 900. Reading variables off that frame also reports a Neutrals/700
and a Neutrals/900 of #171717, neither of which appears in the swatch set, so
those come from the frame's own chrome rather than the palette.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
His Supporting collection, addressable by state and step.
--danger-100 500 900
--success-100 500 900
--warning-100 500 900
--info-100 500 900
100 and 900 swap between themes; 500 holds in both, which is what his
light-to-dark sheet shows with #E61B26 mapping to itself. Each step resolves
to a primitive rather than repeating a hex.
Named by state rather than by his collection. --supporting-informational-500
carries no more meaning than --info-500 and is twice the length, and nothing
else here is called danger or info. "info" also matches the vocabulary the
semantic tokens already use.
The tradeoff is that --danger-500 sits beside --red-600 as a second name for
the same colour, where a supporting- prefix would have kept that visible.
That is the cost of his system and ours coexisting; the semantic layer is
still the one components should reach for.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
--always-primary purple-600, both themes
--always-white slate-0, both themes
Neither appears in the .dark block, which is the point: they express a colour
pinned across themes, and nothing else here does that. --color-surface-action
goes purple-600 to purple-400 between modes; Always-primary holds.
Only these two of his four Surface entries are added. Surface/Light already
exists as --color-surface-default: it reads #FFFFFF on the light frames and
#080C17 on the dark banner, so it is the page rather than a fixed colour.
Surface/Dark is left out because no frame I can read uses it in dark mode, so
whether it pins or inverts is unknown, and slate-1000 already holds its value.
Worth asking him what Surface/Dark does in dark mode. If it pins it is a third
always-*; if it inverts it duplicates Surface/Light.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Advanced
Run ID: becff8c7-2b5a-4bf2-8152-8ec67c138383
⛔ Files ignored due to path filters (1)
frontend/documentation/TokenReference.generated.stories.tsxis excluded by!**/*.generated.*
📒 Files selected for processing (8)
frontend/.storybook/docs-theme.scssfrontend/.storybook/manager.jsfrontend/common/theme/tokens.jsonfrontend/common/theme/tokens.tsfrontend/documentation/ColourPalette.stories.tsxfrontend/scripts/generate-tokens.mjsfrontend/web/styles/_primitives.scssfrontend/web/styles/_tokens.scss
💤 Files with no reviewable changes (1)
- frontend/web/styles/_primitives.scss
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.
| "literal": { "cssVar": "--color-code-literal", "light": "#0b82d7", "dark": "#4cb5fd" }, | ||
| "string": { "cssVar": "--color-code-string", "light": "#47aa7f", "dark": "#6ad0a1" }, | ||
| "variable": { "cssVar": "--color-code-variable", "light": "#d06907", "dark": "#efb47c" }, | ||
| "title": { "cssVar": "--color-code-title", "light": "#0aaddf", "dark": "#7ecde2" }, | ||
| "builtin": { "cssVar": "--color-code-builtin", "light": "#d4b050", "dark": "#f9dc80" } | ||
| "title": { "cssVar": "--color-code-title", "light": "#0fa5fc", "dark": "#7bc9fe" }, | ||
| "builtin": { "cssVar": "--color-code-builtin", "light": "#e8a705", "dark": "#fcdb80" } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
rg -n 'color-code-(literal|string|title|builtin)|colorCode(Literal|String|Title|Builtin)|code.*background|color-surface-code' frontend/common frontend/web frontend/documentation | head -110
sed -n '108,130p' frontend/common/theme/tokens.jsonRepository: Flagsmith/flagsmith
Length of output: 5290
Use darker light-theme syntax colours.
The light-theme literal, string, title and builtin colours do not meet the 4.5:1 text contrast requirement on the code background. Select darker light-theme values and regenerate the SCSS tokens.
Changes
Adopts the colour system from Figma. Rather than adding his ramps alongside ours, our existing ramps are re-anchored on his values, so there's one palette rather than two names for the same colour.
Every value he specifies now exists in our primitives: Primary 6/6, Neutrals 8/8, Supporting 12/12, Surface 3/3. Only his Content palette is left, which has no dark values yet.
The bug this fixes. The four dark status surfaces resolved darker than the page they sit on (1.04:1), so a tinted panel read as a cut-out. They now take the 100 and 900 of their own ramp and clear the page by 1.43–2.29:1.
What changes visually
#6AD0A1, ours was a teal at hue 178#FFBC05; orange keepschart-4,chart-9,code-variable#7A0E18, contrast 6.45 → 11.03slate-1000becomes his#080C17, butsurface-defaultstays onslate-950: the page and the surfaces sitting on it have to move together, so both go in a follow-upWorth knowing
slate-250(#BFC0C5) is deliberately unreferenced — it'll be consumed when we build tokens for his system._primitives.scsswas a second copy of the palette that had drifted in 41 of 80 values. Deleted; the Storybook page readstokens.jsonnow.Open with design
500s are under 3:1 as borders on white (success 1.88, warning 1.69, info 2.69). His field-error pattern leans on that border as the signal.How did you test this code?
npm run typecheck— 913, matchingmainnpm run test:unit— 710 pass🤖 Generated with Claude Code