fix(ui): stop the theme switch repainting the page half in each theme - #2653
Conversation
Switching Light <-> Dark left the app painted in two themes at once for around half a second: a light header band and light sidebar chrome over dark panels, with row headings still carrying the light-theme ink colour on a dark surface. Cause: the `theme-transitioning` block declared, with `!important`, a 200ms colour transition on `html.theme-transitioning` AND `html.theme-transitioning *` AND both pseudo-element tiers. Every node in the document therefore started a transition on the same frame. Measured in Chromium 1194 on the settings dialog at 1440x900, Light -> Dark: 313ms cls=1 anims=1226 body=rgb(255,255,255) header=rgb(255,255,255) 859ms cls=0 anims=27 body=rgb(11,14,17) header=rgb(38,44,50) 1226 concurrently running animations, then 546ms with no frame presented. The surviving frame is the mixed-theme paint users see. The crossfade it was paying for never rendered either: sampled colours had already snapped to their dark end on the first frame after the click. Fading only the root was tried and rejected: the ground then interpolated for the full 200ms while every panel above it snapped, leaving a near-white page background behind dark chrome (body rgb(234,235,235) at 190ms with the header already dark). A partial fade reads worse than no fade. The class now suppresses transitions instead of adding them, which is what this block's own prefers-reduced-motion branch already did for the identical selector - and reduced motion is the one path this was never reported from. That branch is removed because it is now the unconditional behaviour, so reduced-motion users see exactly what they saw before. After, same probe, two runs: 62ms cls=1 anims=0 body=rgb(11,14,17) header=rgb(38,44,50) 104ms cls=1 anims=0 body=rgb(11,14,17) header=rgb(38,44,50) Zero animations, every surface dark on the first frame sampled, frames presented every 17-65ms. No mixed-theme frame exists. The 2026-08-02 CWV audit (RB-3) flagged this selector and judged it acceptable because it is user-triggered; that judgement is recorded in the new comment as wrong - being user-triggered is what made it visible. Gates: check:design-system-contract exit 0; check:design-drift-ratchet exit 0; theme-transition-timer, mobile-chrome-paint-contract, ui-overlay-css-contract, clinical-dashboard-merge-artifacts and design-token-contract suites 74/74. Full unit suite 16995 passed with 3 pre-existing failures (clinical-hazard-controls, privacy-readiness-contract, rag-plan-package-parity), reproduced identically on a stashed clean tree. verify:ui NOT run - plan:browser selects the full suite for a shared foundation, and this container's Chromium is rev 1194 against the repo's pinned 1234 (#255 drift); browser proof delegated to CI. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mp72U91fKSqD6S12R9ijZd
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: 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 |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_b5b075f7-84a6-4b84-800c-f518a21aca76) |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_ff5745a9-ebe0-47dc-b78f-6d5482368962) |
Summary
Switching Light <-> Dark left the app painted in two themes at once for roughly half a second: a light header band and light sidebar chrome over dark panels, with row headings still carrying the light-theme ink colour on a dark surface.
Cause: the
theme-transitioningblock declared, with!important, a 200 ms colour transition onhtml.theme-transitioningandhtml.theme-transitioning *and both pseudo-element tiers, so every node in the document started a transition on the same frame.Measured in Chromium 1194 on the settings dialog at 1440x900, Light -> Dark, sampling every
requestAnimationFrame:1226 concurrently running animations, then 546 ms with no frame presented. The frame that survives that stall is the mixed-theme paint. The crossfade it was paying for never rendered either — sampled colours had already snapped to their dark end on the first frame after the click.
The
!importantwas the other half of the cost: it replaced every element's owntransition-propertyfor the whole window, so any transform/opacity animation in flight during a theme switch was dropped mid-run.Fix: the class now suppresses transitions instead of adding them. That is what this block's own
prefers-reduced-motionbranch already did for the identical selector, and reduced motion is the one path this was never reported from. The branch is removed because it is now the unconditional behaviour, so reduced-motion users see exactly what they saw before.Rejected alternative, recorded in the code comment: fading only the root. The ground then interpolated for the full 200 ms while every panel above it snapped, leaving a near-white page background behind dark chrome (
bodymeasuredrgb(234,235,235)at 190 ms with the header already dark). A partial fade reads worse than no fade.After, same probe, two consecutive runs:
Zero animations, every surface dark on the first frame sampled, frames presented every 17-65 ms. No mixed-theme frame exists.
use-theme.tsis unchanged: the class and its 200 ms timer keep their behaviour andtests/theme-transition-timer.dom.test.tsxstill passes as written.The 2026-08-02 CWV audit (RB-3) flagged this selector and judged it acceptable because it is user-triggered. The new comment records that judgement as wrong — being user-triggered is exactly what made it visible.
Verification
npm run verify:pr-local— Verification not run: heavy scope selectsbuildplus the full unit suite for asrc/app/globals.csschange; the CSS-scoped gates below were run individually instead and CI runs the aggregate.npm run check:design-system-contractexit 0 —Design-system contract passed (1232 production files; raw colors 0; literal shadows 0; legacy tap classes 0; sub-floor interactive min-heights 5; edge conflicts 5; 1px shadow spreads 0).npm run check:design-drift-ratchetexit 0 — both ceilings held.npx vitest run tests/theme-transition-timer.dom.test.tsx tests/mobile-chrome-paint-contract.test.ts tests/ui-overlay-css-contract.test.ts tests/clinical-dashboard-merge-artifacts.test.ts tests/design-token-contract.test.ts—Test Files 5 passed (5) / Tests 74 passed (74).npm run test—Test Files 3 failed | 1157 passed | 1 skipped (1161) / Tests 3 failed | 16995 passed. The three failures areclinical-hazard-controls,privacy-readiness-contractandrag-plan-package-parity; all three reproduce identically on a stashed clean tree, so they are pre-existing and unrelated to this diff.npm run test:focused -- --files src/app/globals.css— no test files selected (CSS is not in the Vitest import graph), which is why the contract suites above were run by name.npm run verify:ui— UI verification not run:npm run plan:browserselectslevel: fullbecausesrc/app/globals.cssis a shared foundation, and this container's Chromium is rev 1194 against the repo's pinned 1234 (feat(ui): compact phone bottom search bar on search/result views #255 drift), so a local run would not be the gate. Browser proof delegated to CI; please keep this PR out of draft so the Production UI job runs. Direct instrumented Chromium measurement of the changed behaviour is quoted above.Risk and rollout
src/app/globals.css; no TypeScript, no component, no token value changed. The behaviour the block now produces for everyone is the behaviourprefers-reduced-motionusers already had.Notes
🤖 Generated with Claude Code
https://claude.ai/code/session_01Mp72U91fKSqD6S12R9ijZd
Generated by Claude Code
Note
Low Risk
Single CSS block in
globals.css; no token or TS changes, and reduced-motion users already had this paint behavior.Overview
Fixes Light ↔ Dark painting the app in two themes at once (light header/sidebar over dark panels with mismatched text colors).
The
html.theme-transitioningblock no longer forces a 200ms!importantcolor transition on every node (html,*, and pseudo-elements). That pattern spawned ~1200 concurrent animations, stalled frames, and produced a visible half-themed frame while the intended crossfade never actually rendered.Change: while the class is on
<html>(unchanged inuse-theme.ts), CSS now appliestransition: none !importanton the same selectors so all surfaces flip in one frame. The oldprefers-reduced-motionbranch is removed because that suppression is now the default for everyone.A long inline comment documents measured failure modes and warns against re-adding transitions here.
Reviewed by Cursor Bugbot for commit 3b337d4. Configure here.