Skip to content

docs: add ADR for attribute-scoped colour-scheme switching over light-dark() - #5200

Open
pomfrida wants to merge 1 commit into
mainfrom
docs/adr-attribute-scoped-not-light-dark
Open

docs: add ADR for attribute-scoped colour-scheme switching over light-dark()#5200
pomfrida wants to merge 1 commit into
mainfrom
docs/adr-attribute-scoped-not-light-dark

Conversation

@pomfrida

@pomfrida pomfrida commented Jul 20, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Summary

ADR-0013 (status Accepted) records a decision that is already implemented in both token pipelines but was only documented in a commit message, a CLAUDE.md pitfall note, and a README section: token CSS expresses colour schemes exclusively as attribute-scoped custom-property blocks, and light-dark() must never appear in published CSS.

Why it deserves a permanent record:

  • The fix: Switch has poor contrast in dark mode; disabled state appears more prominent than enabled #4863 production regression is the evidence: downstream bundlers (Vite 8 / Rolldown / lightningcss without targets) polyfill light-dark() into a :root-baked var() pattern that silently breaks subtree-scoped dark mode. fix: emit explicit dark-scope rules instead of light-dark() in tokens #4864 rewrote the 2.x output to explicit scopes and added the assert-no-light-dark guard.
  • light-dark() accepts only colour values, so density/spacing/typography modes need attribute scoping regardless — one switching mechanism for every axis (per ADR-0004).
  • The 3.0 Tokens Studio output is attribute-scoped by construction (zero light-dark( occurrences). Without an ADR, the per-scheme duplication looks like something a future maintainer or code agent should "modernize" back into light-dark() — reintroducing the bug class.

The ADR also captures the consequences: no automatic system-dark (apps set data-color-scheme themselves), the FOUC/inline-script setup-docs requirement, reusing the assertion guard on the future 3.0 bundle, and an explicit revisit criterion.

Related: ADR-0004, ADR-0010 (#5199), #4863 / #4864

Tracking issues

@pomfrida pomfrida self-assigned this Jul 20, 2026
This was referenced Jul 20, 2026
Closed
Open
Open
@claude claude Bot mentioned this pull request Jul 20, 2026
Open
@pomfrida

pomfrida commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator Author

@claude review

@claude

claude Bot commented Jul 20, 2026 •
edited
Loading

Copy link
Copy Markdown

Claude finished @pomfrida's task in 1m 59s —— View job

Review: ADR-0013 — attribute-scoped custom properties, not light-dark()

  • Read the ADR
  • Verify referenced files & scripts exist
  • Verify cross-referenced ADRs (0004, 0010) & numbering
  • Verify factual claims (light-dark absence, guard behaviour)
  • Post review

Verdict: strong, accurate ADR — approve with a couple of minor notes. This is a docs-only change; every factual claim I could check holds up against the code.

What I verified ✅

Claim in ADR Check Result
assert-no-light-dark.mjs guard exists packages/eds-tokens/scripts/assert-no-light-dark.mjs ✅ present; asserts on variables.min.css for both light-dark( and --lightningcss-* markers, plus min scope counts
build-dark-scope.ts rewrite exists packages/eds-tokens-build/src/scripts/build-dark-scope.ts ✅ present
3.0 output: "zero occurrences across src/tokens/" grep -rl "light-dark(" packages/eds-tokens/src/tokens/ ✅ zero matches
ADR-0004 is the attribute-driven theming convention (data-color-scheme) 0004-component-conventions-for-eds-2.md:45,111,116 ✅ documents data-color-scheme light/dark
Structure matches the ADR template 0000-template.md ✅ Context / Decision Drivers / Options Considered / Decision / Consequences / Related all match

One thing worth calling out because it looks like a contradiction but isn't: build/css/color/color-scheme/color-scheme.css still contains ~98 light-dark( literals. That's the intermediate compiled CSS (build step 2), which build-dark-scope rewrites before the minified variables.min.css ships (step 3d/3f), exactly as eds-tokens/CLAUDE.md describes. The ADR is careful to scope its "must not appear" claim to published CSS, so this is consistent. Nicely worded.

Minor notes (non-blocking)

1. ADR-0010 is a dangling reference in this repo. The ADR folder currently jumps 0008 → 0013; there is no 0010 file (it's proposed in #5199, per the PR body). The ADR consistently marks it "(proposed)", which is the right call, but until #5199 merges the link resolves to nothing. Two things to confirm:

  • That 0010 is the number docs: add ADR proposing a single bundled css entry for eds-tokens 3.0.0 #5199 will actually land on (so the reference doesn't break).
  • That 0013 is a reserved/coordinated number — with 0009–0012 presumably in flight across parallel PRs, there's a real risk of two open PRs claiming the same integer. Worth a quick check against the other open ADR PRs before merge.

2. Consider a short ### Confirmation section. The template includes it (this ADR omits it, as does 0008, so it's optional). But here you have an unusually concrete confirmation mechanism worth naming explicitly: the CI-enforced assert-no-light-dark.mjs guard is the thing that keeps this decision from silently regressing. A one-liner ("Confirmed in CI by assert-no-light-dark.mjs, which fails the build if any light-dark( literal reaches variables.min.css") would make the enforcement path first-class rather than only mentioned in the Decision prose.

3. Tiny precision nit (optional). Line 26 summarises the guard as asserting "no light-dark( literal ships." True, but the guard actually does three things — no light-dark(, no --lightningcss-* polyfill markers, and a minimum count of [data-color-scheme] scope rules. The --lightningcss-* check is arguably the more interesting one given #4863 is specifically a lightningcss-polyfill regression. You may want the Context or Decision to note that the guard also catches the polyfilled form, not just the source literal — that's the failure mode that actually bit production.

Nits I deliberately did not flag

  • Em-dash usage — the "no em-dashes" rule is for component docs (COMPONENT_DOC_STYLE.md), not ADRs; peer ADRs use them freely.
  • Duplication-cost framing ("gzip absorbs it") — reasonable and matches how peer design systems justify the same trade-off.

Great record to have written down — the reasoning (light-dark() only accepts <color>, so you'd need attribute scoping for density/typography anyway) is the part most likely to stop a well-meaning "modernization" later, and it's stated clearly.

@pomfrida pomfrida mentioned this pull request Jul 21, 2026
Open
pomfrida added a commit that referenced this pull request Jul 27, 2026
@pomfrida
fix: widen only the semantic layer to [data-color-scheme]
cb4119b 
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.
@pomfrida pomfrida mentioned this pull request Jul 27, 2026
Merged
pomfrida added a commit that referenced this pull request Jul 30, 2026
@pomfrida
fix: emit the beta semantic token layer under the color-scheme scopes (…
470a35e 
…#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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vnys vnys Awaiting requested review from vnys vnys is a code owner
@millus millus Awaiting requested review from millus millus is a code owner
@Chibuzor-Nwemambu Chibuzor-Nwemambu Awaiting requested review from Chibuzor-Nwemambu Chibuzor-Nwemambu is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

@pomfrida pomfrida

Labels

None yet

Projects

None yet

Milestone

[2026-Q2] Refine the EDS colour and t...

Development

Successfully merging this pull request may close these issues.

1 participant

@pomfrida