fix(theme): add 'system' as a first-class theme preference

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'

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)
+  })
+})

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'
+}

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 (
     <button
       onClick={toggleTheme}
       className="p-1.5 rounded-md bg-theme-elevated hover:bg-theme-hover text-theme-text-secondary hover:text-theme-text-primary transition-colors"
-      title={`Switch to ${theme === 'dark' ? 'light' : 'dark'} mode`}
+      title={`Switch to ${effectiveTheme === 'dark' ? 'light' : 'dark'} mode`}
     >
-      {theme === 'dark' ? (
+      {effectiveTheme === 'dark' ? (
         <Sun className="w-4 h-4" />
       ) : (
         <Moon className="w-4 h-4" />

web/src/components/logs/LogsViewer.tsx

@@ -14,7 +14,7 @@ interface LogsViewerProps {
 
 export function LogsViewer({ namespace, podName, containers, initialContainer }: LogsViewerProps) {
   const desktopDownload = useDesktopDownload()
-  const { theme } = useTheme()
+  const { effectiveTheme } = useTheme()
 
   const fetchLogs = useCallback(async (params: LogsFetchParams) => {
     const query = new URLSearchParams()
@@ -41,7 +41,7 @@ export function LogsViewer({ namespace, podName, containers, initialContainer }:
       fetchLogs={fetchLogs}
       createStream={makeStream}
       overrideDownload={desktopDownload}
-      forceDark={theme === 'dark' ? true : undefined}
+      forceDark={effectiveTheme === 'dark' ? true : undefined}
     />
   )
 }

web/src/components/logs/WorkloadLogsViewer.tsx

@@ -13,7 +13,7 @@ interface WorkloadLogsViewerProps {
 
 export function WorkloadLogsViewer({ kind, namespace, name }: WorkloadLogsViewerProps) {
   const desktopDownload = useDesktopDownload()
-  const { theme } = useTheme()
+  const { effectiveTheme } = useTheme()
 
   const fetchAll = useCallback(async (params: WorkloadLogsFetchParams): Promise<WorkloadLogsResult> => {
     const query = new URLSearchParams()
@@ -37,7 +37,7 @@ export function WorkloadLogsViewer({ kind, namespace, name }: WorkloadLogsViewer
       fetchAll={fetchAll}
       createStream={makeStream}
       overrideDownload={desktopDownload}
-      forceDark={theme === 'dark' ? true : undefined}
+      forceDark={effectiveTheme === 'dark' ? true : undefined}
     />
   )
 }

web/src/components/ui/CodeViewer.tsx

@@ -3,6 +3,9 @@ import { CodeViewer as BaseCodeViewer } from '@skyhook-io/k8s-ui'
 import { useTheme } from '../../context/ThemeContext'
 
 export function CodeViewer(props: Omit<ComponentProps<typeof BaseCodeViewer>, 'theme'>) {
-  const { theme } = useTheme()
-  return <BaseCodeViewer {...props} theme={theme} />
+  // BaseCodeViewer wants a concrete dark/light value, not the user's
+  // stored preference (which can now be 'system'). Pass the resolved
+  // effective theme.
+  const { effectiveTheme } = useTheme()
+  return <BaseCodeViewer {...props} theme={effectiveTheme} />
 }

web/src/context/ThemeContext.tsx

@@ -1,35 +1,56 @@
-import { createContext, useContext, useEffect, useState, ReactNode } from 'react'
+import { createContext, useContext, useEffect, useMemo, useState, ReactNode } from 'react'
+import {
+  resolveEffectiveTheme,
+  isTheme,
+  nextThemeForToggle,
+  type Theme,
+  type EffectiveTheme,
+} from '@skyhook-io/k8s-ui/utils/theme'
 import { apiUrl, getAuthHeaders, getCredentialsMode } from '../api/config'
 
-type Theme = 'dark' | 'light'
+// Re-export so existing radar-app imports of `Theme` from this module
+// keep compiling. The canonical types now live in @skyhook-io/k8s-ui
+// so the radar-hub-web Preferences page can bind to the same vocab.
+export type { Theme, EffectiveTheme }
 
 interface ThemeContextType {
+  /** User's stored preference (may be 'system'). */
   theme: Theme
+  /** What's actually applied right now ('dark' or 'light'). */
+  effectiveTheme: EffectiveTheme
   setTheme: (theme: Theme) => void
+  /**
+   * Toggles between explicit dark and light based on what's currently
+   * visible. Always writes an explicit ('dark' or 'light') value so a
+   * subsequent OS theme change doesn't silently fight the user's
+   * deliberate keyboard-shortcut choice. (SKY-823 bug 13)
+   */
   toggleTheme: () => void
 }
 
 const ThemeContext = createContext<ThemeContextType | undefined>(undefined)
 
 const THEME_STORAGE_KEY = 'radar-theme'
 
-function getInitialTheme(): Theme {
-  // Check localStorage first
-  if (typeof window !== 'undefined') {
-    const stored = localStorage.getItem(THEME_STORAGE_KEY)
-    if (stored === 'light' || stored === 'dark') {
-      return stored
-    }
-    // Check system preference
-    if (window.matchMedia('(prefers-color-scheme: light)').matches) {
-      return 'light'
-    }
-  }
-  return 'dark' // Default to dark
+function readStoredTheme(): Theme {
+  if (typeof window === 'undefined') return 'system'
+  const stored = localStorage.getItem(THEME_STORAGE_KEY)
+  return isTheme(stored) ? stored : 'system'
+}
+
+function readPrefersLight(): boolean {
+  if (typeof window === 'undefined') return false
+  return window.matchMedia('(prefers-color-scheme: light)').matches
 }
 
 export function ThemeProvider({ children }: { children: ReactNode }) {
-  const [theme, setThemeState] = useState<Theme>(getInitialTheme)
+  const [theme, setThemeState] = useState<Theme>(readStoredTheme)
+  const [prefersLight, setPrefersLight] = useState<boolean>(readPrefersLight)
+
+  const effectiveTheme = useMemo(
+    () => resolveEffectiveTheme(theme, prefersLight),
+    [theme, prefersLight],
+  )
 
   const setTheme = (newTheme: Theme) => {
     setThemeState(newTheme)
@@ -45,44 +66,43 @@ export function ThemeProvider({ children }: { children: ReactNode }) {
   }
 
   const toggleTheme = () => {
-    setTheme(theme === 'dark' ? 'light' : 'dark')
+    setTheme(nextThemeForToggle(effectiveTheme))
   }
 
-  // Apply theme to document
+  // Apply effective theme to document
   useEffect(() => {
-    document.documentElement.classList.toggle('dark', theme === 'dark')
-    document.documentElement.style.colorScheme = theme
-  }, [theme])
+    document.documentElement.classList.toggle('dark', effectiveTheme === 'dark')
+    document.documentElement.style.colorScheme = effectiveTheme
+  }, [effectiveTheme])
 
   // Sync theme from server (persisted settings survive port changes in desktop app)
   useEffect(() => {
     fetch(apiUrl('/settings'), { credentials: getCredentialsMode(), headers: getAuthHeaders() })
       .then((res) => res.ok ? res.json() : null)
       .then((data) => {
-        if (data?.theme && (data.theme === 'dark' || data.theme === 'light') && data.theme !== theme) {
+        if (isTheme(data?.theme) && data.theme !== theme) {
           setThemeState(data.theme)
           localStorage.setItem(THEME_STORAGE_KEY, data.theme)
         }
       })
       .catch((err) => console.warn('[settings] Failed to load theme from server:', err))
   }, []) // eslint-disable-line react-hooks/exhaustive-deps
 
-  // Listen for system theme changes
+  // Track OS theme. In 'system' mode this updates `effectiveTheme`
+  // immediately; in explicit dark/light mode the value is still
+  // tracked so a future switch back to 'system' picks up the latest
+  // OS state without a page refresh.
   useEffect(() => {
     const mediaQuery = window.matchMedia('(prefers-color-scheme: light)')
     const handleChange = (e: MediaQueryListEvent) => {
-      // Only auto-switch if user hasn't explicitly set a preference
-      const stored = localStorage.getItem(THEME_STORAGE_KEY)
-      if (!stored) {
-        setThemeState(e.matches ? 'light' : 'dark')
-      }
+      setPrefersLight(e.matches)
     }
     mediaQuery.addEventListener('change', handleChange)
     return () => mediaQuery.removeEventListener('change', handleChange)
   }, [])
 
   return (
-    <ThemeContext.Provider value={{ theme, setTheme, toggleTheme }}>
+    <ThemeContext.Provider value={{ theme, effectiveTheme, setTheme, toggleTheme }}>
       {children}
     </ThemeContext.Provider>
   )