fix(theme): add 'system' as a first-class theme preference#575
fix(theme): add 'system' as a first-class theme preference#575eliran-ops wants to merge 1 commit into
Conversation
SKY-823 bug 13: the keyboard shortcut 't' toggles light/dark theme but
the Preferences page Theme selector still shows 'System'. Root cause:
the stored Theme type was `'dark' | 'light'` only — there was no value
that meant "follow the OS", so the radar-hub-web Preferences page
exposed a 'System' option that didn't correspond to anything in the
app's state. The shortcut and the selector therefore could not stay in
sync.
This change extends the theme contract so the bug becomes fixable from
both sides:
- New pure helpers in `packages/k8s-ui/src/utils/theme.ts`:
- `Theme = 'dark' | 'light' | 'system'` and `EffectiveTheme = 'dark' | 'light'`
- `resolveEffectiveTheme(theme, prefersLight)` — pure resolver
- `isTheme(value)` — type guard for storage / HTTP reads
- `nextThemeForToggle(effective)` — always returns an explicit value;
the keyboard shortcut writes a deterministic dark/light so a
subsequent OS change can't silently undo the user's deliberate
choice. 7 unit tests pin the contract.
- `web/src/context/ThemeContext.tsx`:
- Re-exports `Theme` / `EffectiveTheme` from k8s-ui so existing
imports keep working.
- `useTheme()` now returns `{ theme, effectiveTheme, setTheme,
toggleTheme }`. `theme` is the stored preference (may be 'system');
`effectiveTheme` is what's currently applied to the document.
- Tracks the OS `prefers-color-scheme` media query continuously so
'system' updates `effectiveTheme` immediately.
- The server settings round-trip uses `isTheme` to narrow values
safely, including the new 'system'.
- All in-repo consumers updated to use `effectiveTheme` where they
want the concrete current value (CodeViewer, LogsViewer,
WorkloadLogsViewer, ThemeToggle button icon/tooltip).
The radar-hub Preferences "Theme" selector can now bind to `theme`
directly: when the keyboard shortcut fires, the selector updates from
'System' to 'Dark' or 'Light' in lockstep.
Linear: SKY-823
Made-with: Cursor
|
@nadaverell @hisco — ready for review. No outstanding cursor findings. Cursor Bugbot SUCCESS on current HEAD. |
Visual testTested locally on Default (light) theme renderedEmpty localStorage on first load. Press
|
|
@nadaverell a reminder |
2 participants
Summary
SKY-823 bug 13: the keyboard shortcut `t` toggles light/dark theme but the radar-hub Preferences page Theme selector still shows `System`. Linear: SKY-823.
Root cause
The stored `Theme` type was `'dark' | 'light'` only — there was no value that meant "follow the OS", so the radar-hub-web Preferences page exposed a `System` option that didn't correspond to anything in the app's state. The shortcut and the selector therefore could not stay in sync.
Approach
Extends the theme contract so the bug becomes fixable from both sides:
New pure helpers in `packages/k8s-ui/src/utils/theme.ts`:
`nextThemeForToggle` deliberately writes a deterministic dark/light when the keyboard shortcut fires — staying on `'system'` would let a future OS theme change silently undo the user's deliberate choice. 7 unit tests pin the contract.
`web/src/context/ThemeContext.tsx`:
Consumer updates: `CodeViewer`, `LogsViewer`, `WorkloadLogsViewer`, and the topbar `ThemeToggle` icon/tooltip now use `effectiveTheme` so they always reflect what's actually visible.
Cross-repo follow-up
The radar-hub Preferences `Theme` selector can now bind to `theme` directly: when the keyboard shortcut fires, the selector updates from 'System' to 'Dark' or 'Light' in lockstep. That part is a one-liner change in `radar-hub-web` — out of scope for this PR.
Test plan
Made with Cursor
Note
Medium Risk
Touches global theming state, persistence, and OS media-query handling; mistakes could cause incorrect theme application or preference drift across sessions/devices.
Overview
Fixes theme preference handling by making
systema first-class stored value and separating storedthemefrom derivedeffectiveTheme(dark/light), with shared pure helpers (resolveEffectiveTheme,isTheme,nextThemeForToggle) added to@skyhook-io/k8s-ui/utilsand covered by new unit tests.Updates
ThemeContextto persist/consumesystem, continuously track OSprefers-color-scheme, apply the resolvedeffectiveThemeto the document, and ensure the keyboard toggle always writes an explicit dark/light value; UI consumers (topbar toggle, logs viewers,CodeViewer) now useeffectiveThemeso icons/tooltips and forced-dark rendering match what’s actually displayed.Reviewed by Cursor Bugbot for commit a03367c. Bugbot is set up for automated code reviews on this repo. Configure here.