Conversation
VOY-138 Auto Theme not working
Voyager theme set: |
There was a problem hiding this comment.
Pull request overview
This PR adjusts the app’s theme synchronization logic in the root layout to better handle OS theme changes on Linux (notably in Tauri), adding a more reliable re-probe flow and fallbacks.
Changes:
- Introduces
autoMode()and refines when OS-driven theme updates should apply vs. when a user-selected theme should be respected. - Updates the Tauri
onThemeChangedhandling to re-read the OS theme via the existing probe instead of trusting the event payload. - Adds fallbacks to re-sync theme on media-query changes, focus, and visibility changes (Tauri-only).
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
@claude review opus |
|
Claude finished @piotrsac's task in 4m 21s —— View job 🦀 ReviewSeverity: 🔴 blocking · 🟡 should-fix · 🟢 nit
<verdict: 1 blocking, 1 should-fix, 0 nits> |
| const hint = window.__VOYAGER_OS_THEME__; | ||
| const stored = storedOs(); | ||
| let osTheme = isTheme(hint) ? hint : isTheme(stored) ? stored : media(); | ||
| let nativeOs = isTheme(hint) || isTheme(stored); |
There was a problem hiding this comment.
🔴 blocking: nativeOs is set from values that are not necessarily native. setOs writes every OS theme to sessionStorage — including ones derived from matchMedia — and seed_script always assigns __VOYAGER_OS_THEME__, falling back to matchMedia when the portal snapshot fails. So (a) in a plain browser, one OS theme change writes voyager:os-theme, and after the next reload nativeOs is true, the change listener on line 93 returns early, and auto theme stops following the OS for the rest of the session; (b) in Tauri on a desktop with no working settings portal, the seed hint is matchMedia-derived, nativeOs is true from the first paint and os_theme returns null, so the matchMedia fallback is dead and auto theme never updates — the exact VOY-138 symptom. Only flip nativeOs on a value that actually came from a native probe (e.g. a separate flag set by seed_script when the snapshot succeeded, plus setOsFromNative), and don't infer it from sessionStorage.
|
|
||
| /// DaisyUI `--color-base-200` used by `html, body` (`assets/css/app.css`). | ||
| /// dark: oklch(23.26% 0.014 253.1); light: oklch(98% 0.003 247.858). | ||
| const BASE_200_DARK: tauri::window::Color = tauri::window::Color(32, 36, 41, 255); |
There was a problem hiding this comment.
🟡 should-fix: BASE_200_DARK doesn't match the token the doc comment above cites. oklch(23.26% 0.014 253.1) converts to rgb(25, 30, 36) (#191E24), not rgb(32, 36, 41) — the light constant checks out, so the dark one is the outlier. The native window fill is then visibly lighter than the page's dark base-200, which is the seam background_color/set_surface exist to remove.
| const BASE_200_DARK: tauri::window::Color = tauri::window::Color(32, 36, 41, 255); | |
| const BASE_200_DARK: tauri::window::Color = tauri::window::Color(25, 30, 36, 255); |
No description provided.