fix(themes): auto-flip heading/body/card foreground on coloured surfaces (#496) - #637
Conversation
…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
…ix (#496) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ScK3jJFKQ7aWasg25dc7VS
|
Warning Review limit reached
Next review available in: 27 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Greptile SummaryThis PR fixes a cascade-sealing bug where
Confidence Score: 4/5Safe to merge; all three fix sites are consistent, the style query is correctly gated behind the same The logic across The Important Files Changed
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
%%{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
Reviews (1): Last reviewed commit: "docs(changelog): note coloured-surface h..." | Re-trigger Greptile |
| "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 { |
There was a problem hiding this comment.
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!
… 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
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 becausevar()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:<h2>stayed dark on a dark surface (measuredoklch(0.12)heading onoklch(0.22)secondary surface — invisible), and.sf-cardon 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 inthemes.css(#496); this PR completes that pattern for the heading/body/card aliases. It also honours the documented contract indocs/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-colorin the.sf-surfaceauto-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-activeflag ("inside an auto-contrast surface").core/themes.css— re-declare--sf-heading-color/--sf-body-coloralongside the existing--sf-card-bg/--sf-card-border-colorre-declarations in the shared:where([data-theme]…)block (covers both light and dark schemes in one place).optional/components.css— a.sf-cardkeeps the page surface as its own (non-adapting) background, so the surface's flipped foreground must not leak into it. Gated on--sf-surface-activevia a style query (not a.sf-surface .sf-carddescendant selector, which would misclassify the surface classes in the generated class reference), the card restores surface-appropriate tokens from the ambient neutral and setscolorfor inherited body text. A consumer's own--sf-color-headingoverride 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 byanimation-timeline: view()and@starting-stylerespectively, 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-carddescendant selector (universally supported, but pollutes the generated class metadata) is noted in the code comment.Verification
Empirically measured
getComputedStylein Chromium across 5 scenarios — normal page,[data-theme="dark"], dark surface (--secondary), light surface (--warning), and a custom--sf-color-headingancestor 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
Checklist
feat:,fix:,docs:, …) — enforced by commitlintnpm run lint:csspasses (stylelint)npm run buildrebuildsdist/(bundles are git-ignored; CI rebuilds and stamps headers)npm testpasses (unit 129/129; Playwright e2e 140/140 on chromium locally — see Notes)npm run check:version)macros.css, unregistered);check:llm-guidepassescheck:macros,check:registry,audit:checkpass; zero generated-file diff from this change)CHANGELOG.mdupdated under## UnreleasedNotes
tests/surface-generic.spec.js: a direct heading and a.sf-card(title + body) inside.sf-surface--secondarymust each clear 3:1 contrast against their own background. Both fail without the fix and pass with it (verified by reverting the source).@playwright/test1.61) via anexecutablePathoverride; CI runs the full chromium/firefox/webkit matrix.🤖 Generated with Claude Code
Generated by Claude Code