refactor: migrate Button (next) colours to beta tokens - #5222
Conversation
…th the widened semantic block The widened semantic layer and the color-scheme scope rules both apply to [data-color-scheme] elements at equal specificity, so source order decides the three duplicate names tracked in #5221. With the previous alphabetical order the semantic block sorted last and won - and its --eds-border-focus is a self-reference, computing to the guaranteed-invalid value, which would have dropped focus outlines for the incoming Chip (#5225) and Button (#5222) migrations inside scoped subtrees (both use var(--eds-border-focus) for their focus ring). The bundler now ranks color-scheme/* after every other layer, so the scheme-specific values win on scoped elements and every resolved value matches the pre-widening state. Verified in the browser: --eds-border-focus resolves to the light/dark blue per subtree, and text-disabled/border-disabled keep their scheme values.
…#5239) * fix: emit the beta semantic token layer under the color-scheme scopes The Tokens Studio CSS export declared the semantic layer at :root while the scale aliases it references only exist under [data-color-scheme] scopes. Custom properties substitute where they are declared, so the semantic tokens resolved once at the root and never re-resolved inside [data-color-scheme] subtrees, breaking subtree colour-scheme switching (ADR-0004/ADR-0013). The saved EDS-CSS export configuration now sets rootSelector: ':root, [data-color-scheme]' (was the ':root' default), so every non-dimensional layer is also declared on any element carrying the attribute and var() references substitute against the nearest scope. Token content is unchanged - the diff is selector lines only. The <html>-mirror workaround in the Storybook decorator is no longer needed and is removed. Closes #5226 * chore: use beta semantic tokens for the storybook wrapper canvas The wrapper used the legacy --eds-color-bg-neutral-surface, which has no dark declaration in the 2.x bundle, so the story canvas stayed light in dark mode. The beta semantic tokens re-resolve per [data-color-scheme] subtree after the rootSelector change, so the canvas now follows the toolbar toggle. Legacy is left untouched - it is being replaced, not maintained. * fix: widen only the semantic layer to [data-color-scheme] Review follow-up on #5239. Widening every non-dimensional layer via the export's global rootSelector regressed the density cascade: the density base (comfortable) declares the same --eds-density-* names as the compact/relaxed variants, so re-declaring it on every [data-color-scheme] element clobbered a [data-density] ancestor's values for the whole subtree. The export format has no per-layer selector, so the saved EDS-CSS configuration is reverted to the default rootSelector (:root) and the semantic layer - the only layer that references the scheme-scoped scale aliases - is widened repo-side by scripts/widen-semantic-scope.mjs, chained before generate-css-bundle.mjs in the generate:css-bundle package script so the release workflow needs no change. The script is idempotent and fails loudly if the export layout changes. Also drops a comment reference to ADR-0013 (not merged yet, PR #5200) and documents the #5221 name-collision caveat in the bundle header. * chore: assert the semantic layer is widened before bundling The widen step is chained in the generate:css-bundle package script, but running the bundler directly would silently bundle an unwidened semantic layer and regress subtree colour-scheme switching (#5226). The bundler now fails loudly instead. * chore: harden the semantic-widening against formatting and export drift Review follow-up on #5239, round two. - The widened selector is matched with whitespace-tolerant regexes and emitted in Prettier's canonical one-selector-per-line form, shared between the widen script and the bundler via semantic-scope.mjs, so a prettier --write no longer breaks the pipeline with a misleading 'export layout changed' error. - Every semantic/*.css file is widened and asserted, mirroring the bundler's directory glob - a file the export adds later cannot slip through with the narrow :root selector. - src/tokens/* is added to .prettierignore (root and package-local): the directory is machine output and was never Prettier-conformant (the export emits double-quoted attribute selectors and no trailing newline, failing prettier:check on main too), and hand-formatting it would be reverted by the next export run. The package prettier:check now passes. * chore: widen nested semantic files and document the widening in the release workflow Review follow-up on #5239, round three. - The widen script now globs semantic/ recursively, matching the bundler's assertion glob - a nested file the export adds later gets widened instead of deadlocking the pipeline with a remedy message that is a no-op. - A selector that matches the tolerant regex but not the canonical form is normalised, so the committed bytes are a function of content only. - The release workflow's bundle step and auto-PR body now mention the semantic-layer widening, so the selector diff in tokens-release PRs is traceable to #5226 instead of looking like an export change. * fix: concatenate the color-scheme layer last so it wins collisions with the widened semantic block The widened semantic layer and the color-scheme scope rules both apply to [data-color-scheme] elements at equal specificity, so source order decides the three duplicate names tracked in #5221. With the previous alphabetical order the semantic block sorted last and won - and its --eds-border-focus is a self-reference, computing to the guaranteed-invalid value, which would have dropped focus outlines for the incoming Chip (#5225) and Button (#5222) migrations inside scoped subtrees (both use var(--eds-border-focus) for their focus ring). The bundler now ranks color-scheme/* after every other layer, so the scheme-specific values win on scoped elements and every resolved value matches the pre-widening state. Verified in the browser: --eds-border-focus resolves to the light/dark blue per subtree, and text-disabled/border-disabled keep their scheme values.
…colour-tokens # Conflicts: # packages/eds-core-react/.storybook/preview.mjs
|
@claude review |
|
Claude finished @pomfrida's task in 6m 41s —— View job Review:
|
| pre-PR (neutral appearance) | this PR | |
|---|---|---|
| primary bg | neutral-3 | neutral-2 |
| label | neutral-7 | neutral-5 |
| step delta | 4 | 3 |
WCAG 1.4.3 exempts inactive controls, so this isn't a violation — but it's a regression in legibility on the token values, not the CSS. Worth confirming with design or folding into #4741 rather than landing silently.
4. The export models hover/pressed text and border shifts; the component doesn't use them
These exist and are unreferenced:
--eds-text-interactive-accent-hover: var(--eds-accent-11);
--eds-text-interactive-accent-pressed: var(--eds-accent-12);
--eds-border-interactive-accent-emphasis-hover: var(--eds-accent-10);
--eds-border-interactive-accent-emphasis-pressed: var(--eds-accent-11);Secondary/ghost labels stay at -default (step 10) through hover and pressed, and the secondary border stays at -default (step 9). Since the token set explicitly models the shift, worth confirming the omission is intentional rather than inherited from the pre-PR CSS, which had no equivalent tokens to shift to.
5. #5283 won't be a no-op rename — flag that on the issue
accent and danger use text-interactive-{tone}-default = step 10. The neutral stand-in --eds-text-primary is neutral-12. So neutral secondary/ghost labels currently render two steps darker than their accent/danger siblings, and when --eds-text-interactive-neutral-default lands it will most likely be neutral-10 — i.e. lighter than what this PR ships. (Pre-PR neutral used --eds-color-text-subtle, lighter again.) Worth a line on #5283 so the swap isn't mistaken for a value-preserving rename.
6. data-color-appearance is now load-bearing for two different systems
Button.tsx:41 already set this attribute pre-PR, so it isn't introduced here — but this PR makes it the tone switch. It's also the attribute the legacy dynamic bundle keys off (build/css/color/dynamic/variables.css:11,37,63), and documentation/agent-instructions/BUILDING_EDS_2_COMPONENTS.md:62-84 says to put it on the smallest element, not the component root, precisely because descendants inherit it. Setting accent/danger on the button root means any /next child still reading dynamic --eds-color-* tokens inherits that appearance.
Harmless today because Button's own colours no longer read dynamic tokens. But as the reference migration, every subsequent component will copy this. Either decouple the tone switch onto its own attribute (data-tone) or add a comment in button.css recording that root-level data-color-appearance is a deliberate exception to the smallest-element rule.
7. disabled ? 'neutral' : tone is now dead for Button's own colours
Button.tsx:41. All three disabled states use appearance-agnostic *-interactive-disabled tokens, so the forced neutral no longer changes anything Button renders — it only changes what descendants inherit. Drop it or comment why it stays.
Nits
button.css:25-26duplicate168-169—--_color: var(--eds-text-on-emphasis)and--_bg-color: var(--_bg-emphasis)are declared identically at the root and under[data-variant='primary']. Pre-existing shape, but these exact lines are being rewritten anyway. Fix this →button.css:207-209is redundant —[data-variant='secondary']:focus-visiblesets the sameoutline-offset: var(--eds-sizing-stroke-thin)that the general:focus-visiblerule at line 245 already applies.- Focus ring mixes bundles — the width is legacy (
--eds-sizing-stroke-thick), the colour is beta (--eds-border-interactive-focus). Expected for a colours-only PR, just noting it for the sizing follow-up. - Pre-existing stylelint failures, not from this PR:
selector-class-patternrejects.labelat lines 68 and 78 ("Expected class '.label' to follow BEM syntax"). The stylelint config contradicts the flat-inner-class convention inAGENTS.md. Separate config fix.
Suggested before undraft
- Resolve or explicitly document the
data-color-schemeprerequisite (Michael is testing #1) — the missing focus ring is the part I'd not want to ship. - Track the disabled border/background collision (fake-bot added new tokens #2) upstream; it's a visible regression today.
- Confirm removed color. Should only trigger storefront? #4 and removed color. Should only trigger storefront? #5 with design, and note removed color. Should only trigger storefront? #5 on Semantic interactive tokens are missing success and info tones #5283.
Nothing here needs a change to the CSS structure — the --_ indirection pattern is sound and I'd keep it as the template for the remaining components.
• branch refactor/button-beta-colour-tokens
Follows the Button pattern (#5222): background-interactive-{tone}-{muted,emphasis}-*, text-interactive-{tone}-default, border-interactive-{tone}-emphasis-default, and border-interactive-focus for the focus ring. Neutral maps text to --eds-text-primary (no interactive neutral text/icon by design, #5283). success/info have no interactive group yet (#5283) and stay on raw steps mirroring the semantic ladders.
Follows the Button pattern (#5222): background-interactive-{tone}-{muted,emphasis}-*, text-interactive-{tone}-default, border-interactive-{tone}-emphasis-default, and border-interactive-focus for the focus ring. Neutral maps text to --eds-text-primary (no interactive neutral text/icon by design, #5283). success/info have no interactive group yet (#5283) and stay on raw steps mirroring the semantic ladders.
Follows the Button pattern (#5222): background-interactive-{tone}-{muted,emphasis}-*, text-interactive-{tone}-default, border-interactive-{tone}-emphasis-default, and border-interactive-focus for the focus ring. Neutral maps text to --eds-text-primary (no interactive neutral text/icon by design, #5283). success/info have no interactive group yet (#5283) and stay on raw steps mirroring the semantic ladders.
1 participant
Resolves #5208 (phase 1 of #5119). First component migrated onto the Tokens Studio beta colour tokens — the test case for the migration pattern.
Updated 2026-08-06 after tokens release #5280: the branch is merged with
mainand rewritten from the interim--_tone-Nstep indirection to the semanticinteractivetoken group that release added. The Storybook wiring has landed onmain(#5224/#5239), so the diff is nowbutton.cssonly.Updated 2026-08-07: design confirmed in #5283 that the missing neutral text/icon interactive tokens are deliberately out of scope — neutral interactive text uses
--eds-text-primaryby design, so the--eds-text-primarymapping in this PR is the intended final state, not a stand-in. That was the last token gap Button was waiting on.Token availability for consumers: the interactive token group is published —
@equinor/eds-tokens@3.0.0-beta.4(cut right after #5280 merged) ships it in the Tokens Studio bundle under the publish-time./next/css/*export (ADR-0009: consumers import@equinor/eds-tokens/next/css/variables.css). Note that the legacy./css/variablesexport does not contain the group, so apps on the /next beta need thenext/cssimport. Prod Storybook predates the #5224/#5239 wiring; the next Storybook deploy frommainpicks the tokens up automatically.Changes
button.css:--eds-background-interactive-{tone}-emphasis-{default,hover,pressed}; label:--eds-text-on-emphasis--eds-background-interactive-{tone}-muted-{hover,pressed}; text:--eds-text-interactive-{tone}-default--eds-border-interactive-{tone}-emphasis-default--eds-border-interactive-focus--eds-{background,text,border}-interactive-disabled— the legacy-token fallbacks and the TODO(🎨 Implement Alpha‑Channel approach for disabled states #4741) are gone. If 🎨 Implement Alpha‑Channel approach for disabled states #4741 ships alpha-channel values they land as value changes on these same tokens, with no component change needed.--_indirection remains at the component root purely sodata-color-appearancecan switch the accent/neutral/danger token set; states read those variables.Design notes
text-interactivegroup deliberately has no neutral variant (confirmed in Semantic interactive tokens are missing success and info tones #5283) — the neutral tone maps secondary/ghost text to--eds-text-primary(neutral-12) by design.Export gaps and bugs found (historical, since resolved)
Captured as learnings on #5208; fixed at the source via #5221/#5280:
No semantic action/fill group→ shipped as theinteractivegroup in feat: update tokens from Tokens Studio release #5280Semantic surface group accent-only; no accent text/border semantic tokens→ covered by theinteractivegroup--eds-border-focusself-reference cycle--eds-border-interactive-focusalias addedSemantic layer only resolved with→ fixed by scoping the semantic layer underdata-color-schemeon<html>[data-color-scheme](Emit the semantic token layer under the [data-color-scheme] scopes #5226/fix: emit the beta semantic token layer under the color-scheme scopes #5239)Missing neutral text/icon interactive tokens (Semantic interactive tokens are missing success and info tones #5283)→ confirmed deliberate; primary text/icon tokens are the intended mapping*-interactive-disabledtokens shipped in the meantimeVerification
button.css+main's token export over the deployed stories: before/after screenshots of All Variants / Tones / Disabled Variants in light and dark are visually equivalent (pixel diff < 2 %, mostly anti-aliasing). All 27 token references resolve in both schemes — none missing. Two deliberate value shifts from feat: update tokens from Tokens Studio release #5280 are visible on close inspection: emphasis fills sit one step lighter than legacy, and the dark-scheme disabled fill is now a dark subtle surface (oklch(0.226)on theoklch(0.19)canvas) instead of medium slate.