Dark theme: full token migration + data-theme switch, contrast as a test (#548) - #549
Conversation
77 distinct literals across 269 occurrences outside :root — a dark theme can only flip what routes through tokens, so this is the load-bearing half of the child. Pure refactor: light rendering is unchanged except that near-identical one-off whites and tints are collapsed onto their family representative (channel deltas of a step or two). New tokens: surfaces (--surface-2/--hover/--line-soft), text (--text-secondary/--on-strong — color:#fff on strong fills is deliberately NOT a surface), per-status -soft/-line/-deep families for green/red/amber/ blue, --purple (previously fallback-only), --chart-dim, and the white topbar veils. Exact-value matches reuse existing tokens (--panel, --line-strong, --green, --amber, the --band-* ramp). color-scheme: light declared so the dark block can flip native form controls with it. Zero literal colors remain outside :root; 491/491 dashboard/hub tests unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
One [data-theme="dark"] block overrides every color token. Digital-Twin- adjacent palette: deep navy surfaces, cool light text, brighter status hues so strokes and pills clear their floors on dark panels. color-scheme flips with it, so selects and scrollbars follow. Two tests carry the weight: Contrast is computed, not eyeballed — WCAG ratios for twenty load-bearing token pairs (text on surfaces, muted copy, status text on its own tint, strokes and fills as UI components) are asserted in BOTH themes. The dark palette passed on the first run; the LIGHT theme failed one pair — the #546 density bars sat at 1.76:1 on white, below the 3:1 graphical-object floor. Darkened. The new theme's test fixed a defect in the old one. Completeness is enforced — a color token the dark block forgets silently keeps its light value, which is how a white panel ends up in a dark page. The test failed on its first run too: I had forgotten --band-very-high. The switch: explicit choice in localStorage wins; with no choice the OS decides and KEEPS deciding (a mid-session OS flip is honoured until the human picks). Topbar toggle with a constant label and aria-pressed carrying state — the #432 lesson, where a next-action label contradicted the attribute. A pre-paint head snippet applies the theme before the stylesheet renders, so a dark reader never gets a white flash. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Six journeys: OS-dark renders dark untouched; the toggle flips, persists and survives reload; an explicit light choice beats an OS-dark preference; the theme attribute is set before first paint (the head snippet, not the bundle); the #546 charts and #544 glyph stay visible against dark panels; axe serious/critical clean on command/approvals/live-feed in dark. The dark screenshot caught what the first pair list missed: the Approve button rendered near-white on pastel green. In dark, every strong fill is a light pastel and --ink flips light, so text ON them flips dark — the Material dark-surface convention. Two pairs added to the contrast test (on-strong/green, on-strong/ink) so the class is pinned in both themes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Strix is installed on this repository, but we couldn't run this PR security review because this workspace's trial has ended. Add a card to resume code reviews here. |
|
Warning Review limit reached
Next review available in: 37 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: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe dashboard adds persisted light and dark themes, system preference detection, an accessible theme toggle, dark-theme CSS tokens across dashboard surfaces, and unit and browser coverage for contrast, persistence, initialization, visibility, and accessibility. ChangesHub dark theme
Possibly related issues
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
PR Summary by QodoHub dark theme via full token migration, data-theme switch, and contrast tests
AI Description
Diagram
High-Level Assessment
Files changed (6)
|
Code Review by Qodo
Context used✅ Compliance rules (platform):
300 rules✅ Skills:
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| --panel: #111a2d; | ||
| --soft: #0d1626; | ||
| --line: #243250; | ||
| --line-strong: #3d4e70; | ||
| --line-soft: #1e2a42; |
There was a problem hiding this comment.
5. Dark --line-strong contrast too low 📜 Skill insight ☑ Accessibility
In the dark theme, --line-strong (#3d4e70) against --panel (#111a2d) is ~2.09:1, below the 3:1 WCAG AA requirement for UI components/boundaries. This reduces the perceptibility of component borders and violates the contrast compliance rule.
Agent Prompt
## Issue description
The dark theme defines `--line-strong: #3d4e70` while `--panel` is `#111a2d`, which yields an estimated WCAG contrast ratio of ~2.09:1—below the required 3:1 for UI components/boundaries.
## Issue Context
`--line-strong` is used for visible borders on `--panel` backgrounds (e.g., `.policy-action` uses `border: 1px solid var(--line-strong)` with `background: var(--panel)`), so insufficient contrast makes UI boundaries harder to perceive.
## Fix Focus Areas
- src/observability/dashboard/ui/styles.js[96-104]
- src/observability/dashboard/ui/styles.js[1882-1903]
- tests/hub-dark-theme-548.test.js[64-91]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/observability/dashboard/ui/index.js`:
- Line 81: Move the inline theme initialization script from the end of the body
into the document head, placing it immediately after the
<style>${styles}</style> block. Preserve its existing localStorage,
system-preference, and error-handling behavior so the data-theme attribute is
set before any body markup can be parsed or painted.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ed270664-22df-4db1-ae06-0f964de44fcf
📒 Files selected for processing (6)
src/observability/dashboard/ui/client.jssrc/observability/dashboard/ui/index.jssrc/observability/dashboard/ui/lib.jssrc/observability/dashboard/ui/styles.jstests/browser/dashboard-548-dark.test.jstests/hub-dark-theme-548.test.js
CodeRabbit (Major), Qodo, and a CI failure all converged on the same thing: the "pre-paint" theme snippet sat at the END of body — after the entire shell markup — where it prevents exactly nothing. Moved into <head> after the stylesheet, which is what "before first paint" actually requires. The no-flash journey is rewritten (its reload-inside-evaluate contraption died on context destruction in CI) to assert on the SERVED HTML that the snippet precedes </head>, plus the attribute at domcontentloaded; a unit pins the placement structurally. Qodo, all real: - Any truthy localStorage garbage counted as an explicit choice, silently disabling OS-following forever. Only the literal 'dark'/'light' are choices now, in both the snippet and hubThemeChoice(). - A toggle that failed to persist (private mode) was overridden by the next OS flip. An in-memory session choice now backs the stored one. The first draft of the test for this forgot to break storage and passed through localStorage — rewritten to genuinely throw, and mutation-checked (removing the in-memory choice fails it). - Legacy engines exposing only MediaQueryList.addListener never followed OS flips. Fallback wired and unit-tested against an addEventListener-less media object. - The topbar chips are ~28px tall: hit areas expand via the #539 ::after pattern (zero layout change), for all three chips, not just the toggle. Declined with measurement (reply on the PR): raising dark --line-strong to 3:1 — dark's border contrast (2.09:1) already exceeds what light ships (1.46:1), and control identification does not rest on the border alone. A both-themes border redesign belongs with the standing contrast debt. 2010/2010 core, 73/73 browser, lint 0 errors. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Fixed in
|
The logo <img> replaces the R brand mark, so its ink-fill/on-strong rules go with it — the badge keeps that pairing and the contrast pair stays, comment updated. Everything else auto-merged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… counts While preparing 2.3.1, PR #549 (dark theme, epic #501 now 5/8) landed on origin/main from another session — merged cleanly (no conflicts; the brand-mark logo change from the previous session's work survived intact). Updates the 2.3.1 changelog entry to describe it honestly rather than calling this a behavior-free patch, and refreshes README's test-count badge and CLI example output (2010 tests, up from a stale 1454 reference that had drifted for several releases). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2 participants
Closes #548 — epic #501's largest child. Three bisected commits.
1. The load-bearing half: a full token migration
A dark theme can only flip what routes through tokens, and
ui/styles.jscarried 77 distinct hardcoded colors across 269 occurrences. All migrated: the recurring families became semantic tokens (surfaces, hover, per-status-soft/-line/-deeptints,--on-strongfor text on strong fills,--chart-dim, the topbar veils), exact-value matches reuse existing tokens, and near-identical one-off whites/tints are collapsed onto their family representative (channel deltas of a step or two — the light theme renders visibly unchanged, screenshot-verified). Zero literal colors remain outside:root.color: #fffwas deliberately split from background#fff: text on a strong fill is not a surface, and dark proves why (below).2. Dark palette, with contrast as a test instead of an eyeball
One
[data-theme="dark"]block overrides every color token — Digital-Twin-adjacent: deep navy surfaces, cool light text, brighter status hues. Two tests carry the weight, and both earned their keep before the PR opened:--band-very-high.color-schemeflips with the theme, so selects and scrollbars follow natively.3. The switch
Explicit choice in
localStoragewins; with no choice the OS decides and keeps deciding — a mid-session OS flip is honoured until the human picks. Topbar toggle with a constant label andaria-pressedcarrying state (the #432 lesson: a next-action label contradicts the attribute). A pre-paint head snippet applies the theme before the stylesheet renders — no white flash for a dark reader, asserted atdomcontentloaded.What the dark screenshot caught that the tests did not
The Approve button rendered near-white text on pastel green. In dark, every strong fill becomes a light pastel and
--inkflips light — so text on them must flip dark (the Material dark-surface convention). Fixed, and the pair class pinned in the contrast test (--on-strong/--green,--on-strong/--ink) so it holds in both themes. That's the fourth time this wave a screenshot found what green assertions walked past.Verification
var()must resolve).Out of scope
The Studio's own palette (already themed, twin/classic). The remaining #501 children (toasts, mobile action bar, portal card).
Summary by CodeRabbit
New Features
Tests