diff --git a/packages/k8s-ui/src/utils/index.ts b/packages/k8s-ui/src/utils/index.ts index c04f5c2ab..de1a699dc 100644 --- a/packages/k8s-ui/src/utils/index.ts +++ b/packages/k8s-ui/src/utils/index.ts @@ -12,3 +12,4 @@ export * from './api-resources' export * from './skeleton-yaml' export * from './k8s-errors' export * from './parse-go-time' +export * from './theme' diff --git a/packages/k8s-ui/src/utils/theme.test.ts b/packages/k8s-ui/src/utils/theme.test.ts new file mode 100644 index 000000000..60dc3dbc3 --- /dev/null +++ b/packages/k8s-ui/src/utils/theme.test.ts @@ -0,0 +1,67 @@ +import { describe, it, expect } from 'vitest' +import { + resolveEffectiveTheme, + isTheme, + nextThemeForToggle, +} from './theme' + +// SKY-823 bug 13: keyboard 't' toggled the theme but the Preferences +// "Theme" selector still showed "System" because the codebase had no +// 'system' value in the stored preference type. These tests pin the +// new contract: 'system' is now a first-class stored preference, +// resolved against the OS at render time, and the shortcut writes a +// deterministic explicit value so the selector stays in sync. + +describe('resolveEffectiveTheme', () => { + it('returns the explicit value when the user picks dark or light', () => { + expect(resolveEffectiveTheme('dark', true)).toBe('dark') + expect(resolveEffectiveTheme('dark', false)).toBe('dark') + expect(resolveEffectiveTheme('light', true)).toBe('light') + expect(resolveEffectiveTheme('light', false)).toBe('light') + }) + + it('follows the OS preference when the user picks system', () => { + expect(resolveEffectiveTheme('system', true)).toBe('light') + expect(resolveEffectiveTheme('system', false)).toBe('dark') + }) + + it('keeps the explicit choice stable across OS changes', () => { + // Explicit "dark" must NOT silently flip to light when the OS + // prefers-color-scheme media query changes. This is the contract + // the keyboard shortcut depends on. + expect(resolveEffectiveTheme('dark', true)).toBe('dark') + expect(resolveEffectiveTheme('dark', false)).toBe('dark') + }) +}) + +describe('isTheme', () => { + it('accepts dark/light/system', () => { + expect(isTheme('dark')).toBe(true) + expect(isTheme('light')).toBe(true) + expect(isTheme('system')).toBe(true) + }) + + it('rejects garbage so server/disk reads can be safely narrowed', () => { + expect(isTheme('auto')).toBe(false) + expect(isTheme('Dark')).toBe(false) + expect(isTheme('')).toBe(false) + expect(isTheme(null)).toBe(false) + expect(isTheme(undefined)).toBe(false) + expect(isTheme(42)).toBe(false) + }) +}) + +describe('nextThemeForToggle', () => { + it('flips dark and light to their opposite', () => { + expect(nextThemeForToggle('dark')).toBe('light') + expect(nextThemeForToggle('light')).toBe('dark') + }) + + it('always returns an explicit value (never "system")', () => { + // The point of pressing the toggle key is to make a deliberate + // change. Returning 'system' here would let a future OS switch + // silently undo the user's intent — exactly the SKY-823 bug. + const next = nextThemeForToggle('dark') + expect(next === 'dark' || next === 'light').toBe(true) + }) +}) diff --git a/packages/k8s-ui/src/utils/theme.ts b/packages/k8s-ui/src/utils/theme.ts new file mode 100644 index 000000000..7f9109faf --- /dev/null +++ b/packages/k8s-ui/src/utils/theme.ts @@ -0,0 +1,49 @@ +/** + * Theme preference + resolution. Lifted out of the radar-app + * `ThemeContext` so the rules are unit-testable in this package + * (where vitest runs in CI) and so external consumers — notably the + * `radar-hub-web` Preferences page — can reuse the same vocabulary + * instead of inventing their own. + * + * Background: previously `Theme = 'dark' | 'light'` only — there was + * no way to express "follow the OS" in the stored preference. The + * radar-hub Preferences page therefore had a `'System'` selector + * option that didn't correspond to anything in the app's state, so + * the keyboard shortcut `t` could toggle dark/light without the + * selector ever updating. (SKY-823 bug 13) + */ + +export type Theme = 'dark' | 'light' | 'system' +export type EffectiveTheme = 'dark' | 'light' + +/** + * Resolves a stored preference + the OS preference into the concrete + * theme that should be applied. Pure. + */ +export function resolveEffectiveTheme( + theme: Theme, + prefersLight: boolean, +): EffectiveTheme { + if (theme === 'system') return prefersLight ? 'light' : 'dark' + return theme +} + +/** Type guard / narrowing helper for stored values from disk or HTTP. */ +export function isTheme(value: unknown): value is Theme { + return value === 'dark' || value === 'light' || value === 'system' +} + +/** + * Computes the *next stored preference* for a "toggle theme" + * keyboard shortcut. The toggle should always switch what the user + * sees, so: + * - if currently effective dark → next is 'light' (explicit override) + * - if currently effective light → next is 'dark' (explicit override) + * - we deliberately do NOT preserve 'system' across a toggle: + * the user pressed the key to make a deliberate change; staying + * on 'system' would let a future OS switch silently undo their + * intent. + */ +export function nextThemeForToggle(effective: EffectiveTheme): Theme { + return effective === 'dark' ? 'light' : 'dark' +} diff --git a/web/src/App.tsx b/web/src/App.tsx index df456cd3d..2715e39ab 100644 --- a/web/src/App.tsx +++ b/web/src/App.tsx @@ -1600,15 +1600,18 @@ function GitHubStarButton() { // Theme toggle button component function ThemeToggle() { - const { theme, toggleTheme } = useTheme() + // The icon and tooltip should reflect what the user actually sees, + // not their stored preference — otherwise on 'system' the button + // would display the wrong icon. (SKY-823 bug 13) + const { effectiveTheme, toggleTheme } = useTheme() return (