fix: use JS-based theme detection for loading shell instead of prefers-color-scheme media query - #4102
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
PR changed again? Review this PR in Change Stack to compare snapshots and stay oriented. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe loading shell HTML inside ChangesLoading shell initialization with theme detection
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Confidence Score: 5/5Safe to merge — the change is isolated to the static loading shell in index.html and does not touch any React component, routing, or data path. The inline script correctly handles the four meaningful localStorage states ("dark", "light", "system", and null), the nested try/catch guards against SecurityError and environments where matchMedia is unavailable, and the localStorage key "theme" matches next-themes' default storageKey. No production code paths outside the pre-React shell are affected. No files require special attention. Important Files Changed
Reviews (2): Last reviewed commit: "chore: made the initial loading shell th..." | Re-trigger Greptile |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@ui/index.html`:
- Line 13: Add a short HTML comment immediately above the <div id=root> minified
loading shell that documents the un-minified source location (e.g.,
ui/loading-shell/shell.html or repo path), the minification tool/command used
(e.g., html-minifier, terser for JS/CSS, with flags), and brief update steps
(edit source → run minifier → replace the <div id=root> / <div id=bifrost-shell>
block). This comment should reference the elements "root" and "bifrost-shell" so
future editors can find the minified block and the workflow to regenerate it.
- Line 13: The HTMLHint warning is a false positive from inline script content
in the inline <script> that reads localStorage.getItem("theme") and toggles
document.getElementById("root").classList.toggle("shell-dark", ...); fix by
either (A) moving the anonymous IIFE into an external JS file (e.g., create a
small bootstrap script that runs the same logic using
matchMedia("(prefers-color-scheme:dark)") and reference it via <script
src="...">) or (B) silence the linter for this file by disabling the
spec-char-escape rule for ui/index.html; pick one approach and update the file
to either replace the inline IIFE with a src reference to the new
module/function or add the linter-disable directive.
- Line 13: Summary: Two <img> elements (.logo.logo-light and .logo.logo-dark)
are both downloaded because CSS only hides one, doubling payload; fix by loading
only the needed logo. Update the inline theme-detection IIFE (the anonymous
script that reads localStorage and toggles "shell-dark" on the `#root` element) to
pick one logo source dynamically instead of embedding two <img> tags: remove one
of the <img> elements and, in that IIFE, querySelector for the remaining .logo
element (or create it) and set its src to "/bifrost-logo.webp" or
"/bifrost-logo-dark.webp" based on the same theme boolean before appending to
`#bifrost-shell`; alternatively replace both <img> tags with an inline SVG element
and toggle colors via the same script if you prefer vector optimization.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 561a7bb2-37c5-4b88-93e7-73fcf71919b1
📒 Files selected for processing (1)
ui/index.html
a9b9674 to
bbb0f45
Compare
Merge activity
|
…rs-color-scheme` media query (#4102) ## Summary The loading shell shown before React mounts was using a CSS `prefers-color-scheme` media query to determine dark/light mode, which caused a flash of the wrong theme for users who have explicitly set a theme preference in the app. This PR fixes that by reading the stored theme from `localStorage` at shell render time and applying the correct dark/light styles immediately. ## Changes - Replaced `@media (prefers-color-scheme: dark)` CSS rules in the inline loading shell with a `.shell-dark` class-based approach, so dark mode styles are controlled by a class on `#root` rather than the OS-level media query alone. - Added an inline script that runs synchronously before the shell renders, reads `localStorage` for a saved `"theme"` value, falls back to `matchMedia` if none is set, and toggles the `shell-dark` class on `#root` accordingly. - Replaced the `<picture>`/`<source>` element used for the logo with two `<img>` tags (`logo-light` and `logo-dark`), toggled via the `.shell-dark` class, since `<picture>` media queries cannot respond to a JS-applied class. - Minified the inline shell HTML, styles, and script to reduce initial payload size. ## Type of change - [ ] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [x] UI (React) - [ ] Docs ## How to test ```sh cd ui pnpm i || npm i pnpm build || npm run build ``` 1. Open the app and set the theme to dark via the UI settings. 2. Hard-reload the page and observe that the loading shell renders in dark mode immediately, with no flash of the light theme. 3. Repeat with the theme set to light and with no stored preference (should fall back to OS preference). ## Screenshots/Recordings Before: The loading shell always used the OS `prefers-color-scheme` setting, ignoring any user-selected theme stored in `localStorage`, causing a visible flash on reload for users whose app theme differed from their OS theme. After: The shell correctly reflects the user's stored theme preference from the first paint. ## Breaking changes - [ ] Yes - [x] No ## Related issues ## Security considerations The inline script only reads from `localStorage` and calls `matchMedia` — no user input is evaluated or injected into the DOM. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [ ] I added/updated tests where appropriate - [ ] I updated documentation where needed - [ ] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Streamlined the initial loading shell to reduce markup and minify the placeholder DOM. * Improved early theme handling so the site applies dark/light mode immediately on load. * Simplified logo handling to switch between dark and light variants faster during startup. <!-- end of auto-generated comment: release notes by coderabbit.ai -->

Summary
The loading shell shown before React mounts was using a CSS
prefers-color-schememedia query to determine dark/light mode, which caused a flash of the wrong theme for users who have explicitly set a theme preference in the app. This PR fixes that by reading the stored theme fromlocalStorageat shell render time and applying the correct dark/light styles immediately.Changes
@media (prefers-color-scheme: dark)CSS rules in the inline loading shell with a.shell-darkclass-based approach, so dark mode styles are controlled by a class on#rootrather than the OS-level media query alone.localStoragefor a saved"theme"value, falls back tomatchMediaif none is set, and toggles theshell-darkclass on#rootaccordingly.<picture>/<source>element used for the logo with two<img>tags (logo-lightandlogo-dark), toggled via the.shell-darkclass, since<picture>media queries cannot respond to a JS-applied class.Type of change
Affected areas
How to test
Screenshots/Recordings
Before: The loading shell always used the OS
prefers-color-schemesetting, ignoring any user-selected theme stored inlocalStorage, causing a visible flash on reload for users whose app theme differed from their OS theme.After: The shell correctly reflects the user's stored theme preference from the first paint.
Breaking changes
Related issues
Security considerations
The inline script only reads from
localStorageand callsmatchMedia— no user input is evaluated or injected into the DOM.Checklist
docs/contributing/README.mdand followed the guidelinesSummary by CodeRabbit