diff options
| author | Adam Malczewski <[email protected]> | 2026-06-01 09:45:36 +0900 |
|---|---|---|
| committer | Adam Malczewski <[email protected]> | 2026-06-01 09:45:36 +0900 |
| commit | 6c377fba7d516ea89ef2d906a40785a997299b0c (patch) | |
| tree | 32e4dde45fbb4f57680e92050798aa65f15e5a6f /packages/frontend/src/lib | |
| parent | 60999dc48d8c06a10ff8f5b3f6edb1d220fd85ca (diff) | |
| download | dispatch-6c377fba7d516ea89ef2d906a40785a997299b0c.tar.gz dispatch-6c377fba7d516ea89ef2d906a40785a997299b0c.zip | |
fix(theme): consolidate boot apply and Settings picker into shared module
Gemini review surfaced that App.svelte (onMount theme apply) and
SettingsPanel.svelte (theme <select>) hand-rolled their own defaults
and could disagree:
- App.svelte only set data-theme if localStorage had a value, so on a
fresh install daisyUI fell back to the first theme in app.css (light).
- SettingsPanel.svelte hardcoded a UI default of "dark".
Result: a first-time user saw a light app but a Settings panel that
claimed "dark" was selected. Picking *any* value in the dropdown was
the only way to reconcile reality with the UI.
This commit:
- Adds packages/frontend/src/lib/theme.ts as the single source of truth:
THEMES list, Theme type, THEME_STORAGE_KEY, DEFAULT_THEME, plus
loadStoredTheme() and applyTheme() that handle SSR / private-mode /
bad-value cases.
- Rewires App.svelte's onMount to call applyTheme(loadStoredTheme()),
so the boot apply always writes a known good theme to the DOM (even
on fresh installs), matching what Settings will show.
- Rewires SettingsPanel.svelte's picker to use the shared module,
dropping its duplicate THEMES const, duplicate storage key, duplicate
apply/persist logic, and the conflicting "dark" fallback.
- Adds 11 unit tests in tests/theme.test.ts covering the
default-fallback, known/unknown stored values, SecurityError-on-read,
SSR (no localStorage), DOM-attribute write, persistence round-trip,
and the "DOM still updates if storage write throws" contract.
The daisyUI plugin block in app.css still lists themes — that's a
CSS-time concern and can't be imported from TS, so it's kept in sync
by convention (noted in the new module's doc comment).
Diffstat (limited to 'packages/frontend/src/lib')
| -rw-r--r-- | packages/frontend/src/lib/components/SettingsPanel.svelte | 38 | ||||
| -rw-r--r-- | packages/frontend/src/lib/theme.ts | 92 |
2 files changed, 99 insertions, 31 deletions
diff --git a/packages/frontend/src/lib/components/SettingsPanel.svelte b/packages/frontend/src/lib/components/SettingsPanel.svelte index efbaf5f..b6a44bc 100644 --- a/packages/frontend/src/lib/components/SettingsPanel.svelte +++ b/packages/frontend/src/lib/components/SettingsPanel.svelte @@ -1,6 +1,7 @@ <script lang="ts"> import { config } from "../config.js"; import { appSettings } from "../settings.svelte.js"; +import { applyTheme, loadStoredTheme, THEMES, type Theme } from "../theme.js"; import type { KeyInfo } from "../types.js"; const { @@ -13,38 +14,13 @@ const { // Theme picker — was a header-triggered modal (`ThemeSwitcher.svelte`); // inlined here so theme picking lives in Settings alongside other UI -// preferences. The list and localStorage key must stay in sync with the -// boot-time theme apply in `App.svelte`'s `onMount`. -const THEMES = [ - "light", - "dark", - "dracula", - "night", - "nord", - "sunset", - "cyberpunk", - "forest", - "cmyk", - "coffee", - "caramellatte", - "garden", - "luxury", -] as const; +// preferences. Theme constants and apply/persist live in `../theme.ts` +// so the boot-time apply in `App.svelte` and this picker can't drift. +let currentTheme = $state<Theme>(loadStoredTheme()); -const THEME_STORAGE_KEY = "dispatch-theme"; - -let currentTheme = $state( - (typeof localStorage !== "undefined" && localStorage.getItem(THEME_STORAGE_KEY)) || "dark", -); - -function selectTheme(theme: string): void { +function selectTheme(theme: Theme): void { currentTheme = theme; - document.documentElement.setAttribute("data-theme", theme); - try { - localStorage.setItem(THEME_STORAGE_KEY, theme); - } catch { - // Best-effort — private mode / quota. - } + applyTheme(theme); } let titleKeyId = $state<string | null>(null); @@ -178,7 +154,7 @@ $effect(() => { <select class="select select-bordered select-sm w-full capitalize" value={currentTheme} - onchange={(e) => selectTheme(e.currentTarget.value)} + onchange={(e) => selectTheme(e.currentTarget.value as Theme)} > {#each THEMES as theme (theme)} <option value={theme} class="capitalize">{theme}</option> diff --git a/packages/frontend/src/lib/theme.ts b/packages/frontend/src/lib/theme.ts new file mode 100644 index 0000000..2b4bad0 --- /dev/null +++ b/packages/frontend/src/lib/theme.ts @@ -0,0 +1,92 @@ +/** + * Single source of truth for the app's theme picker. + * + * Two callers care about themes: + * - `App.svelte`'s `onMount`, which applies the persisted theme on boot + * so the first paint is the right color. + * - `SettingsPanel.svelte`'s theme `<select>`, which lets the user + * change theme at runtime. + * + * Both used to hand-roll their own `localStorage` key, default value, + * and DOM-attribute write. That drift produced a real bug: `App.svelte` + * left the DOM untouched on first load (so daisyUI fell back to the + * first theme in `app.css`, `light`), while `SettingsPanel` showed + * `"dark"` as the selected value — UI and reality disagreed until the + * user manually picked a theme. Centralizing here closes that gap. + * + * The theme list also intentionally mirrors the `@plugin "daisyui"` + * block in `app.css`. Drift between the two is harmless (a theme name + * present here but not in CSS just falls back to the default daisyUI + * theme at render time), but they should be kept in sync by hand — + * daisyUI's plugin config is a CSS-time concern and can't be imported + * from TS. + */ + +export const THEMES = [ + "light", + "dark", + "dracula", + "night", + "nord", + "sunset", + "cyberpunk", + "forest", + "cmyk", + "coffee", + "caramellatte", + "garden", + "luxury", +] as const; + +export type Theme = (typeof THEMES)[number]; + +export const THEME_STORAGE_KEY = "dispatch-theme"; + +/** + * The fallback theme used both at first-boot apply (when nothing is + * persisted) and as the UI's default-selected value. They MUST match — + * if they ever diverged again, the UI would show one theme and the + * page would render another. + */ +export const DEFAULT_THEME: Theme = "dark"; + +/** + * Read the persisted theme. Returns `DEFAULT_THEME` when nothing is + * stored, when access throws (private mode / SecurityError), or when + * the stored value isn't one of the known themes. Never throws. + * + * SSR-safe: if `localStorage` is undefined (e.g. `vite build` during + * prerender, if that's ever wired up), returns the default. + */ +export function loadStoredTheme(): Theme { + try { + if (typeof localStorage === "undefined") return DEFAULT_THEME; + const raw = localStorage.getItem(THEME_STORAGE_KEY); + if (raw && (THEMES as readonly string[]).includes(raw)) { + return raw as Theme; + } + return DEFAULT_THEME; + } catch { + return DEFAULT_THEME; + } +} + +/** + * Apply a theme to the live document and persist it. The DOM write is + * unconditional (daisyUI keys off `data-theme` on the `<html>` + * element); the storage write is best-effort and swallows quota / + * SecurityError failures because the session continues to work either + * way — only cross-reload persistence degrades. + */ +export function applyTheme(theme: Theme): void { + if (typeof document !== "undefined") { + document.documentElement.setAttribute("data-theme", theme); + } + try { + if (typeof localStorage !== "undefined") { + localStorage.setItem(THEME_STORAGE_KEY, theme); + } + } catch { + // Best-effort. + } +} |
