Skip to content

fix(themes): auto-flip heading/body/card foreground on coloured surfaces (#496) - #637

Merged
jackgranatowski merged 4 commits into
mainfrom
claude/root-alias-token-recalc-q1ej0w
Jul 20, 2026
Merged

fix(themes): auto-flip heading/body/card foreground on coloured surfaces (#496)#637
jackgranatowski merged 4 commits into
mainfrom
claude/root-alias-token-recalc-q1ej0w

Conversation

@jackgranatowski

Copy link
Copy Markdown
Contributor

Summary

:root-only alias overrides (--sf-heading-color: var(--sf-color-heading), --sf-body-color, --sf-card-border-color) seal their value at the declaring element because var() resolves on :root. Any heading/body/card that reads the alias therefore read the frozen page value and ignored the semantic token re-declared by the .sf-surface--* auto-contrast block and the [data-theme] section blocks — so:

  • a direct <h2> stayed dark on a dark surface (measured oklch(0.12) heading on oklch(0.22) secondary surface — invisible), and
  • a .sf-card on a coloured surface computed its title/body/border for the wrong background (light-on-light — a pre-existing, un-noticed bug for .sf-card__title/body, since demos never nest a card in a coloured surface).

This is the exact class of bug already flagged for the [data-theme] block in themes.css (#496); this PR completes that pattern for the heading/body/card aliases. It also honours the documented contract in docs/theming.md ("on named variants, every surface-derived token follows the variant's --sf-color-text--on-*, so that single override fixes every descendant").

What changed

  • core/macros.css — re-declare --sf-heading-color / --sf-body-color in the .sf-surface auto-contrast cascade so direct headings/body re-resolve against the surface's adapted --sf-color-heading / --sf-color-text. Add an internal, inherited --sf-surface-active flag ("inside an auto-contrast surface").
  • core/themes.css — re-declare --sf-heading-color / --sf-body-color alongside the existing --sf-card-bg / --sf-card-border-color re-declarations in the shared :where([data-theme]…) block (covers both light and dark schemes in one place).
  • optional/components.css — a .sf-card keeps the page surface as its own (non-adapting) background, so the surface's flipped foreground must not leak into it. Gated on --sf-surface-active via a style query (not a .sf-surface .sf-card descendant selector, which would misclassify the surface classes in the generated class reference), the card restores surface-appropriate tokens from the ambient neutral and sets color for inherited body text. A consumer's own --sf-color-heading override on a non-surface ancestor is still inherited (the flag is unset there).

Why a style query

@container style(--sf-surface-active: 1) is within the declared floor and raises it by zero: Safari 18.0 and Firefox 129 are already mandated by animation-timeline: view() and @starting-style respectively, and style-query support (Chrome 111 / Safari 18.0 / Firefox 128) sits at or below those. Below the floor the whole colour system already fails, so the card reset is not a distinct degradation. Trade-off vs. a plain .sf-surface--* .sf-card descendant selector (universally supported, but pollutes the generated class metadata) is noted in the code comment.

Verification

Empirically measured getComputedStyle in Chromium across 5 scenarios — normal page, [data-theme="dark"], dark surface (--secondary), light surface (--warning), and a custom --sf-color-heading ancestor override — 0 contrast failures, and normal-page / [data-theme] cards are byte-for-byte unchanged. The custom-override case confirms the style query does not reset outside a surface (title stays the overridden colour).

Type

  • fix
  • feat
  • docs
  • chore / tooling

Checklist

  • Conventional Commit messages (feat:, fix:, docs:, …) — enforced by commitlint
  • npm run lint:css passes (stylelint)
  • npm run build rebuilds dist/ (bundles are git-ignored; CI rebuilds and stamps headers)
  • npm test passes (unit 129/129; Playwright e2e 140/140 on chromium locally — see Notes)
  • Version references in sync if any version-related file changed (npm run check:version)
  • LLM guide reviewed — no new/renamed/deleted PUBLIC tokens (the flag is INTERNAL, defined in macros.css, unregistered); check:llm-guide passes
  • Generated artifacts regenerated, not hand-edited (check:macros, check:registry, audit:check pass; zero generated-file diff from this change)
  • CHANGELOG.md updated under ## Unreleased
  • Breaking changes include migration docs — n/a (purely additive token re-declarations; no default-mode values change)

Notes

  • Adds two regression tests in tests/surface-generic.spec.js: a direct heading and a .sf-card (title + body) inside .sf-surface--secondary must each clear 3:1 contrast against their own background. Both fail without the fix and pass with it (verified by reverting the source).
  • Full e2e was run on the chromium project locally (the sandbox has Chromium 1194, not the 1228 pinned by @playwright/test 1.61) via an executablePath override; CI runs the full chromium/firefox/webkit matrix.

🤖 Generated with Claude Code


Generated by Claude Code

claude added 2 commits July 20, 2026 11:45
…ces (#496)

:root-only alias-overrides (--sf-heading-color: var(--sf-color-heading),
--sf-body-color, --sf-card-border-color) seal their value at the declaring
element because var() resolves on :root. Any heading/body/card that reads the
alias therefore read the frozen page value and ignored the semantic token
re-declared by the .sf-surface--* auto-contrast block and the [data-theme]
section blocks — leaving headings dark on a dark surface and card
title/body/border computed for the wrong background.

Complete the #496 pattern:

- core/macros.css: re-declare --sf-heading-color / --sf-body-color in the
  .sf-surface auto-contrast cascade so direct headings/body re-resolve against
  the surface's adapted tokens. Add an internal, inherited --sf-surface-active
  flag signalling "inside an auto-contrast surface".
- core/themes.css: re-declare --sf-heading-color / --sf-body-color alongside the
  existing --sf-card-bg / --sf-card-border-color re-declarations in the shared
  [data-theme] block (covers both schemes).
- optional/components.css: a .sf-card keeps the page surface as its own
  (non-adapting) background, so the surface's flipped foreground must not leak
  into it. Gated on --sf-surface-active via a style query (not a
  `.sf-surface .sf-card` descendant selector, which would misclassify the
  surface classes in the generated class reference), restore the
  surface-appropriate tokens on the card from the ambient neutral and set
  `color` for inherited body text. A consumer's own --sf-color-heading override
  on a non-surface ancestor is still inherited (flag unset there).

Purely additive token re-declarations — no default-mode values change. Adds
regression tests: a direct heading and a card (title/body) inside
.sf-surface--secondary must clear 3:1 contrast against their own background;
both fail without the fix.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ScK3jJFKQ7aWasg25dc7VS
@coderabbitai

coderabbitai Bot commented Jul 20, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@jackgranatowski, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 27 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 16f88d1d-60bf-4de6-8f8a-d41f018ff953

📥 Commits

Reviewing files that changed from the base of the PR and between f98c11e and 8a7880a.

📒 Files selected for processing (6)
  • CHANGELOG.md
  • core/macros.css
  • core/themes.css
  • optional/components.css
  • tests/bundle-size.spec.js
  • tests/surface-generic.spec.js
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/root-alias-token-recalc-q1ej0w

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Jul 20, 2026

Copy link
Copy Markdown

Greptile Summary

This PR fixes a cascade-sealing bug where :root-only alias tokens (--sf-heading-color, --sf-body-color) froze their var() substitution at :root, causing headings to remain dark on dark .sf-surface--* variants and .sf-card titles/body to render against the wrong background inside coloured surfaces. The fix is applied in three places that each need it for a distinct reason.

  • core/macros.css re-declares --sf-heading-color / --sf-body-color inside the auto-contrast surface cascade so direct headings re-resolve against the surface's adapted semantic tokens, and introduces the internal --sf-surface-active: 1 inheritance flag.
  • core/themes.css adds the same two aliases to the existing [data-theme] re-declaration block, completing the parity with --sf-card-bg / --sf-card-border-color already there.
  • optional/components.css uses a @container style(--sf-surface-active: 1) style query to restore neutral-derived foreground tokens on .sf-card (which keeps the page surface as its own background and therefore must not inherit the surface's flipped colours); light-dark() formulas mirror core/tokens.css's "Resolved color tokens" block and are guarded by the same @supports fence already required by the rest of the colour system.

Confidence Score: 4/5

Safe to merge; all three fix sites are consistent, the style query is correctly gated behind the same @supports fence that governs the rest of the colour system, and the new regression tests are self-contained and deterministic.

The logic across macros.css, themes.css, and components.css is coherent and the light-dark() formulas in the style-query block faithfully mirror the originals in core/tokens.css. The only thing keeping this below a clean score is a cosmetic off-by-one indentation on the .sf-card selector inside the @container block — a minor inconsistency with the 2-space nesting convention used throughout the file.

The @container style() block in optional/components.css (lines 439–467) carries a maintenance note (SL-001) that the light-dark() formulas must stay in sync with core/tokens.css; any future change to the resolved color token formulas there will need a corresponding update here.

Important Files Changed

Filename Overview
core/macros.css Adds --sf-heading-color, --sf-body-color, and --sf-surface-active: 1 re-declarations inside the auto-contrast surface cascade so alias tokens re-resolve against the surface's adapted semantic tokens rather than freezing at :root.
core/themes.css Extends the existing [data-theme] alias re-declaration block to cover --sf-heading-color and --sf-body-color, completing the parity with the --sf-card-bg/--sf-card-border-color fix already present.
optional/components.css Adds a style-query gated reset block to restore neutral-derived foreground tokens on .sf-card when inside an auto-contrast surface; the light-dark() formulas mirror core/tokens.css and the approach is sound, but contains a minor 5-space indentation (should be 6) on the .sf-card selector.
tests/surface-generic.spec.js Adds two regression tests (heading contrast and card title/body contrast on .sf-surface--secondary) using the established canvas-luminance probe pattern; correctly fails without the fix and passes with it.
CHANGELOG.md Adds a clear "Bug Fixes" entry under Unreleased describing the alias-freezing root cause and all affected surfaces.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    ROOT[":root\n--sf-heading-color: var(--sf-color-heading)\n--sf-body-color: var(--sf-color-text)\n--sf-card-border-color: var(--sf-color-border)"]

    THEME["[data-theme] block\ncore/themes.css\nRe-declares all 4 aliases\n(re-substitutes against theme tokens)"]

    SURFACE[".sf-surface--* block\ncore/macros.css\nRe-declares --sf-heading-color\n--sf-body-color\nSets --sf-surface-active: 1"]

    HEADING["Direct heading (h1-h6)\nReads --sf-heading-color\n✅ now re-resolved"]

    BODY["Direct body text\nReads --sf-body-color\n✅ now re-resolved"]

    CARD_QUERY["@container style(--sf-surface-active: 1)\noptional/components.css\n.sf-card gets neutral-derived tokens\nRestores --sf-color-heading, -text, -border\nRe-seals all 3 aliases\nResets color property"]

    CARD_TITLE[".sf-card__title\ncolor: var(--sf-color-heading)\n✅ neutral-derived"]

    CARD_BODY["p inside .sf-card\ninherits color\n✅ neutral-derived"]

    ROOT --> THEME
    ROOT --> SURFACE
    SURFACE --> HEADING
    SURFACE --> BODY
    SURFACE --> CARD_QUERY
    CARD_QUERY --> CARD_TITLE
    CARD_QUERY --> CARD_BODY
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    ROOT[":root\n--sf-heading-color: var(--sf-color-heading)\n--sf-body-color: var(--sf-color-text)\n--sf-card-border-color: var(--sf-color-border)"]

    THEME["[data-theme] block\ncore/themes.css\nRe-declares all 4 aliases\n(re-substitutes against theme tokens)"]

    SURFACE[".sf-surface--* block\ncore/macros.css\nRe-declares --sf-heading-color\n--sf-body-color\nSets --sf-surface-active: 1"]

    HEADING["Direct heading (h1-h6)\nReads --sf-heading-color\n✅ now re-resolved"]

    BODY["Direct body text\nReads --sf-body-color\n✅ now re-resolved"]

    CARD_QUERY["@container style(--sf-surface-active: 1)\noptional/components.css\n.sf-card gets neutral-derived tokens\nRestores --sf-color-heading, -text, -border\nRe-seals all 3 aliases\nResets color property"]

    CARD_TITLE[".sf-card__title\ncolor: var(--sf-color-heading)\n✅ neutral-derived"]

    CARD_BODY["p inside .sf-card\ninherits color\n✅ neutral-derived"]

    ROOT --> THEME
    ROOT --> SURFACE
    SURFACE --> HEADING
    SURFACE --> BODY
    SURFACE --> CARD_QUERY
    CARD_QUERY --> CARD_TITLE
    CARD_QUERY --> CARD_BODY
Loading

Reviews (1): Last reviewed commit: "docs(changelog): note coloured-surface h..." | Re-trigger Greptile

Comment thread optional/components.css Outdated
"Resolved color tokens" block — keep them in sync if the formulas change. */
@supports (color: oklch(from red l c h)) {
@container style(--sf-surface-active: 1) {
.sf-card {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Indentation off-by-one inside @container block

.sf-card { is indented 5 spaces instead of the expected 6 (the file uses 2-space nesting: @supports at 2 → @container at 4 → rule at 6). Every other selector in this file at two levels of at-rule nesting uses 6 spaces. While stylelint may not enforce this specifically, it creates a visual inconsistency that could confuse future maintainers editing this section.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

claude added 2 commits July 20, 2026 12:17
… reset

The coloured-surface .sf-card foreground re-derivation duplicates the
light-dark() heading/text/border formulas, tipping the already-edge full
bundle a few bytes over the 22.1kB gzip guardrail (22.123kB).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ScK3jJFKQ7aWasg25dc7VS
…block

Align the .sf-card rule and its declarations to the file's 2-space nesting
(@supports 2 → @container 4 → rule 6 → decls 8), per review feedback.
No behavioural change — minified output is byte-identical.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ScK3jJFKQ7aWasg25dc7VS
@jackgranatowski
jackgranatowski merged commit ba6d8bd into main Jul 20, 2026
13 checks passed
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.

2 participants