Skip to content

refactor: migrate Chip (next) colours to beta tokens - #5225

Open
pomfrida wants to merge 2 commits into
mainfrom
refactor/chip-beta-colours
Open

refactor: migrate Chip (next) colours to beta tokens#5225
pomfrida wants to merge 2 commits into
mainfrom
refactor/chip-beta-colours

Conversation

@pomfrida

Copy link
Copy Markdown
Collaborator

Closes #5223. Part of #5119 (phase 1 — colour). Second component onto the Tokens Studio beta tokens, following the Button test case (#5222). Chip has no disabled state, so it migrates completely — no legacy colour references remain.

Stacked on #5224 (Storybook wiring); the base retargets to main automatically when that merges.

Changes

chip.css only:

  • All six tones (neutral/accent/success/info/warning/danger — every scale exists in the beta export) via the --_tone-N step indirection established in Button; data-color-appearance selects the scale, root defaults to neutral (Chip's default tone)
  • Semantic tokens where they fit: --eds-text-on-accent (high-contrast text), --eds-border-focus and --eds-background-container-canvas-default (focus ring)
  • Fills follow the semantic subtle ladder: rest/hover/pressed = steps 2/4/6 (matching --eds-background-surface-accent-subtle-*); high-contrast = 9/10/11

Deliberate deviations (design sign-off wanted)

  • Resting subtle fill: step 2 (legacy resolved to step 3)
  • Pressed subtle fill: step 6 (legacy resolved to step 5)

Both align Chip with the semantic subtle rest/hover/pressed trio; visually indistinguishable in the matrix comparison.

Learnings

  • selected turns out to have no colour styling in Chip (check icon only), so the semantic selected groups (background-surface-accent-selected-*, border-selected) were not exercised here after all — a later component (or a Chip design update) needs to validate those.

Verification

  • 45/45 Jest tests pass
  • Visual comparison against prod Storybook (ToneVariantMatrix, all 3 variants × 6 tones): identical in light, consistent in dark

@pomfrida

Copy link
Copy Markdown
Collaborator Author

@claude review

@claude

claude Bot commented Jul 23, 2026

Copy link
Copy Markdown

Claude finished @pomfrida's task in 3m 43s —— View job


Review — Chip (next) beta token migration

Read the full chip.css, the diff against origin/chore/storybook-beta-tokens-wiring, Chip.tsx, Chip.types.ts, and the beta token bundle to verify names and values.

Verdict: clean, well-scoped migration. The --_tone-N indirection reads well, the deviations are documented up front, and it follows the EDS 2.0 conventions (--_ pseudo-private vars, override the variable not the property, data-* for variants). Everything below is minor — nothing blocking.

✅ Verified

  • All beta token names resolve. --eds-{neutral,accent,success,info,warning,danger}-{2,4,6,8,9,10,11,12}, plus --eds-text-on-accent, --eds-border-focus, and --eds-background-container-canvas-default all exist in packages/eds-tokens/src/tokens/css/variables.css.
  • Tone coverage is exact. ChipTone is neutral | accent | success | info | warning | danger. Root defaults the --_tone-* vars to the neutral scale and there's an override block for the other five — so tone="neutral" correctly falls through to the root (no dead [data-color-appearance='neutral'] block), and an unexpected value degrades gracefully to neutral. Nice.
  • Subtle ladder claim checks out. --eds-background-surface-accent-subtle-{default,hover,pressed} = accent 2/4/6 in semantic/default.css, matching the 2/4/6 fills here.
  • High-contrast contrast holds across tones (I dug into this). My first concern was that a fixed --eds-text-on-accent over a per-tone --_tone-9 fill could fail contrast on lighter hues (e.g. warning/orange). It doesn't, because the palette uses uniform lightness per step — every tone's step-9 is oklch(0.52 …) in light and oklch(0.82 …) in dark — and --eds-text-on-accent flips with the mode (light-gray-14 L≈0.90 in light, dark-north-sea-14 L≈0.33 in dark). So the text/fill lightness pairing is constant regardless of tone. Worth one manual contrast spot-check on the brightest fills (warning/success step-9), but structurally it's sound.

🟡 Questions / nits

  1. Undocumented text-colour change (default + outlined). Label colour moved from --eds-color-text-subtle--_tone-12, and for neutral --_tone-12 == --eds-text-primary (neutral-12). That's a shift from subtle to primary text. The PR carefully flags the fill deviations (step 2 vs 3, 6 vs 5) but not this one. Was the label always effectively primary-strength in prod, or is this a (probably desirable, higher-contrast) change worth noting alongside the other deviations? — chip.css:127, chip.css:143

  2. --eds-text-on-accent naming vs usage. Functionally correct as "light-on-emphasis for any tone", but the semantic name reads oddly on a danger/warning high-contrast chip. Harmless given the uniform-lightness design; just flagging in case a tone-neutral alias (e.g. an --eds-text-on-emphasis) exists or is planned in the beta set. — chip.css:160

  3. Repetition. Six ~8-line tone blocks (~48 lines) are inherent to CSS custom properties (no loops) and this matches the Button pattern, so it's the right call for consistency — noting only that adding a 7th tone or extra steps multiplies the edit surface. No action needed.

Everything else (focus ring retarget to --eds-background-container-canvas-default / --eds-border-focus, outlined border → --_tone-8, the active fills) is consistent and correct.

  • Read PR diff and full chip.css
  • Compare against Button/--_tone-N pattern and tone type
  • Verify beta token names + contrast reasoning
  • Post review
    refactor/chip-beta-colours

@pomfrida

Copy link
Copy Markdown
Collaborator Author

Re. review point 1 (text colour on default/outlined moving from --eds-color-text-subtle to --_tone-12): this is not a subtle → primary change. The legacy --eds-color-text-subtle was a dynamic token that resolved to step 12 of the active appearance scale — the published bundle defines it as var(--eds-color-{tone}-12) per data-color-appearance scope (e.g. --eds-color-text-subtle: var(--eds-color-neutral-12) under the neutral appearance). --_tone-12 preserves that mapping exactly; only the semantic name is gone, since the beta export has no equivalent dynamic token yet.

The absolute step-12 values differ marginally because the whole beta palette is regenerated, but that applies to every step and is covered by the visual parity check against prod (see PR description).

Points 2 (--eds-text-on-accent naming on danger/warning) and 3 (tone-block repetition) are known: the naming gap is tracked in #5221, and the repetition matches the Button pattern by design.

@pomfrida

Copy link
Copy Markdown
Collaborator Author

Correction to the parity claim above: the visual check against prod turned out to only cover light mode for Chip (dark was compared local-only). I've now completed the dark-mode comparison against prod with computed colours, and it is not full parity:

Matches in dark

  • Outlined: text (step 12, L 0.91 vs legacy ≈0.90) and border (step 8, L 0.76) match.
  • Default/outlined text: step 12 matches legacy.
  • High-contrast fill: step 9 (L 0.82) matches legacy exactly.

Deviations in dark

  1. Default-variant resting fill is dramatically darker. Legacy uses step 3, which in dark is L 0.47 — a clearly visible fill. The beta semantic subtle ladder (--eds-background-surface-*-subtle-default = step 2) is L 0.226 in dark, nearly identical to the canvas (step 1 = L 0.19), so default chips almost disappear into the background. The "step 2 vs 3" deviation noted in the PR description is trivial in light but large in dark.
  2. The dark scale looks like it has a generation bug: step 6 (L 0.47) is identical to step 3, and step 4 is L 0.52 — so the dark hover/pressed ladder becomes 0.226 → 0.52 → 0.47 (huge jump on hover, pressed lighter than hover). This sits in the semantic subtle ladder itself (2/4/6), not in this PR's mapping.
  3. High-contrast text: legacy is near-black in dark (rgb 3,3,3); beta --eds-text-on-accent resolves to north-sea-14 (L 0.33) — noticeably lighter on the pastel fills. Same class of deviation already flagged for light.

Items 1–3 are token-export issues rather than mapping mistakes — the PR deliberately follows the semantic subtle ladder. Flagged in #5221 for the token/design side. Whether this PR should stay on the semantic ladder (and let the fix land in the tokens) or pin steps per scheme is a design call.

Base automatically changed from chore/storybook-beta-tokens-wiring to main July 27, 2026 10:25
pomfrida added a commit that referenced this pull request Jul 28, 2026
…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.
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