From 0736aa2cb89f93e065e69480bead297f6b9175fd Mon Sep 17 00:00:00 2001 From: Taylor Ho Date: Thu, 23 Apr 2026 10:27:18 -0700 Subject: [PATCH] fix(desktop): make theme selection explicit Remove the auto-detected light/dark promise from Appearance and treat the saved theme as the source of truth during startup so the shell comes up with the selected palette instead of a system-driven fallback. Keep user-selected accent colors layered on top of the chosen theme and cover both theme persistence and accent persistence in the Playwright settings flow. --- .../features/settings/ui/SettingsPanels.tsx | 32 +++-- desktop/src/main.tsx | 2 +- desktop/src/shared/theme/ThemeProvider.tsx | 125 ++++++++++++------ desktop/src/shared/theme/theme-loader.ts | 45 ++++--- desktop/tests/e2e/profile.spec.ts | 57 +++++--- 5 files changed, 162 insertions(+), 99 deletions(-) diff --git a/desktop/src/features/settings/ui/SettingsPanels.tsx b/desktop/src/features/settings/ui/SettingsPanels.tsx index 05e683363..92aa0b9fe 100644 --- a/desktop/src/features/settings/ui/SettingsPanels.tsx +++ b/desktop/src/features/settings/ui/SettingsPanels.tsx @@ -1,4 +1,4 @@ -import { useState, useMemo, useRef } from "react"; +import { useMemo, useRef, useState } from "react"; import { BellRing, Check, @@ -98,7 +98,7 @@ export const settingsSections: SettingsSectionDescriptor[] = [ function formatThemeLabel(name: string): string { return name .split("-") - .map((w) => w.charAt(0).toUpperCase() + w.slice(1)) + .map((word) => word.charAt(0).toUpperCase() + word.slice(1)) .join(" "); } @@ -113,10 +113,13 @@ function ThemeSettingsCard() { } }; - const filtered = useMemo(() => { - const q = search.toLowerCase().trim(); - if (!q) return SYNTAX_THEMES; - return SYNTAX_THEMES.filter((name) => name.includes(q)); + const filteredThemes = useMemo(() => { + const query = search.toLowerCase().trim(); + if (!query) { + return SYNTAX_THEMES; + } + + return SYNTAX_THEMES.filter((name) => name.includes(query)); }, [search]); return ( @@ -124,7 +127,8 @@ function ThemeSettingsCard() {

Appearance

- Choose a theme for Sprout. Light and dark mode is auto-detected. + Pick the theme Sprout should use. Your selection stays active until + you choose another one.

@@ -132,7 +136,7 @@ function ThemeSettingsCard() { setSearch(e.target.value)} + onChange={(event) => setSearch(event.target.value)} placeholder="Search themes..." type="text" value={search} @@ -140,12 +144,12 @@ function ThemeSettingsCard() {
- {filtered.length === 0 ? ( + {filteredThemes.length === 0 ? (

No themes match your search.

) : ( - filtered.map((name) => { + filteredThemes.map((name) => { const isActive = themeName === name; const light = isLightTheme(name); @@ -172,9 +176,9 @@ function ThemeSettingsCard() { {formatThemeLabel(name)} - {isActive && ( + {isActive ? ( - )} + ) : null} ); }) @@ -198,9 +202,9 @@ function ThemeSettingsCard() { title={color.name} type="button" > - {accentColor === color.value && ( + {accentColor === color.value ? ( - )} + ) : null} ))}
diff --git a/desktop/src/main.tsx b/desktop/src/main.tsx index c80449f6c..4b7f272c5 100644 --- a/desktop/src/main.tsx +++ b/desktop/src/main.tsx @@ -29,7 +29,7 @@ function renderApp() { ReactDOM.createRoot(document.getElementById("root") as HTMLElement).render( - + diff --git a/desktop/src/shared/theme/ThemeProvider.tsx b/desktop/src/shared/theme/ThemeProvider.tsx index 36d5f55c3..df40afdb6 100644 --- a/desktop/src/shared/theme/ThemeProvider.tsx +++ b/desktop/src/shared/theme/ThemeProvider.tsx @@ -10,6 +10,7 @@ import { import { createThemeVars, hexToHsl } from "./adaptive-theme"; import { SYNTAX_THEMES, + isLightTheme, type SyntaxThemeName, extractThemeInfo, loadThemeData, @@ -18,6 +19,9 @@ import { const STORAGE_KEY = "sprout-theme"; const CACHE_KEY = "sprout-theme-cache"; const ACCENT_KEY = "sprout-accent-color"; +const CACHE_VERSION = 2; +const DEFAULT_THEME: SyntaxThemeName = "houston"; +const DEFAULT_ACCENT = "#3b82f6"; export const ACCENT_COLORS = [ { name: "Blue", value: "#3b82f6" }, @@ -31,10 +35,8 @@ export const ACCENT_COLORS = [ { name: "Indigo", value: "#6366f1" }, ] as const; -const DEFAULT_ACCENT = "#3b82f6"; - type ThemeContextValue = { - themeName: string; + themeName: SyntaxThemeName; isDark: boolean; isLoading: boolean; accentColor: string; @@ -44,7 +46,6 @@ type ThemeContextValue = { type ThemeProviderProps = { children: ReactNode; - defaultTheme?: SyntaxThemeName; }; const ThemeContext = createContext(undefined); @@ -53,16 +54,22 @@ function isValidThemeName(name: string): name is SyntaxThemeName { return (SYNTAX_THEMES as readonly string[]).includes(name); } -/** Read stored theme, migrating legacy "light"/"dark"/"system" values. */ -function readStoredTheme(fallback: SyntaxThemeName): SyntaxThemeName { +/** Read the stored explicit theme, migrating legacy appearance values. */ +function readStoredTheme(): SyntaxThemeName { const stored = window.localStorage.getItem(STORAGE_KEY); - if (!stored) return fallback; + if (!stored) { + return DEFAULT_THEME; + } - // Migrate legacy values - if (stored === "light") return "catppuccin-latte"; - if (stored === "dark" || stored === "system") return "houston"; + if (stored === "light") { + return "catppuccin-latte"; + } - return isValidThemeName(stored) ? stored : fallback; + if (stored === "dark" || stored === "system") { + return DEFAULT_THEME; + } + + return isValidThemeName(stored) ? stored : DEFAULT_THEME; } function getContrastColor(hex: string): string { @@ -85,23 +92,47 @@ function applyAccentColor(hex: string) { root.style.setProperty("--sidebar-primary-foreground", fgHsl); } +function applyThemeClass(themeName: SyntaxThemeName) { + const root = document.documentElement; + root.classList.remove("light", "dark"); + root.classList.add(isLightTheme(themeName) ? "light" : "dark"); +} + /** Apply cached CSS vars synchronously to prevent FOUC. */ -function applyCachedVars(): string | null { +function applyCachedVars(): SyntaxThemeName | null { try { const cached = window.localStorage.getItem(CACHE_KEY); if (!cached) return null; - const { themeName, vars, isDark } = JSON.parse(cached); + + const { + version, + themeName, + vars, + isDark, + }: { + version?: number; + themeName?: string; + vars?: Record; + isDark?: boolean; + } = JSON.parse(cached); + + if ( + version !== CACHE_VERSION || + !themeName || + !isValidThemeName(themeName) || + !vars || + typeof isDark !== "boolean" + ) { + return null; + } + const root = document.documentElement; for (const [key, value] of Object.entries(vars)) { - root.style.setProperty(key, value as string); + root.style.setProperty(key, value); } root.classList.remove("light", "dark"); root.classList.add(isDark ? "dark" : "light"); - - // Also apply cached accent - const accent = window.localStorage.getItem(ACCENT_KEY) ?? DEFAULT_ACCENT; - applyAccentColor(accent); - + applyAccentColor(window.localStorage.getItem(ACCENT_KEY) ?? DEFAULT_ACCENT); return themeName; } catch { return null; @@ -130,7 +161,7 @@ async function applyTheme(name: SyntaxThemeName): Promise<{ isDark: boolean }> { try { window.localStorage.setItem( CACHE_KEY, - JSON.stringify({ themeName: name, vars, isDark }), + JSON.stringify({ version: CACHE_VERSION, themeName: name, vars, isDark }), ); } catch { // Storage full — non-critical @@ -139,53 +170,59 @@ async function applyTheme(name: SyntaxThemeName): Promise<{ isDark: boolean }> { return { isDark }; } -export function ThemeProvider({ - children, - defaultTheme = "houston", -}: ThemeProviderProps) { - // Apply cached vars synchronously before first render - const [themeName, setThemeName] = useState(() => { - const cached = applyCachedVars(); - return cached ?? readStoredTheme(defaultTheme); - }); - const [isDark, setIsDark] = useState(() => { - return document.documentElement.classList.contains("dark"); +export function ThemeProvider({ children }: ThemeProviderProps) { + // Apply cached vars synchronously before first render when available. + const [themeName, setThemeName] = useState(() => { + const cachedTheme = applyCachedVars(); + if (cachedTheme) { + return cachedTheme; + } + + const storedTheme = readStoredTheme(); + applyThemeClass(storedTheme); + return storedTheme; }); + const [isDark, setIsDark] = useState(() => + document.documentElement.classList.contains("dark"), + ); const [isLoading, setIsLoading] = useState(true); - const loadingRef = useRef(null); const [accentColor, setAccentColorState] = useState(() => { return window.localStorage.getItem(ACCENT_KEY) ?? DEFAULT_ACCENT; }); + const loadingRef = useRef(null); - // Load and apply theme + // Load and apply the selected theme. useEffect(() => { - if (!isValidThemeName(themeName)) return; - - // Track which theme we're loading to avoid race conditions const thisTheme = themeName; loadingRef.current = thisTheme; setIsLoading(true); - applyTheme(themeName).then(({ isDark: dark }) => { - // Only update if this is still the theme we want - if (loadingRef.current === thisTheme) { + applyTheme(themeName) + .then(({ isDark: dark }) => { + if (loadingRef.current !== thisTheme) return; setIsDark(dark); setIsLoading(false); - // Re-apply accent after theme load (theme vars don't include primary) applyAccentColor( window.localStorage.getItem(ACCENT_KEY) ?? DEFAULT_ACCENT, ); - } - }); + window.localStorage.setItem(STORAGE_KEY, thisTheme); + }) + .catch(() => { + if (loadingRef.current !== thisTheme) return; + setIsLoading(false); + }); }, [themeName]); - // Apply accent color changes + // Apply accent color changes on top of the selected theme. useEffect(() => { applyAccentColor(accentColor); }, [accentColor]); const setTheme = useCallback((name: string) => { - if (!isValidThemeName(name)) return; + if (!isValidThemeName(name)) { + return; + } + setThemeName(name); window.localStorage.setItem(STORAGE_KEY, name); }, []); diff --git a/desktop/src/shared/theme/theme-loader.ts b/desktop/src/shared/theme/theme-loader.ts index 31e7567c6..a79c3bc44 100644 --- a/desktop/src/shared/theme/theme-loader.ts +++ b/desktop/src/shared/theme/theme-loader.ts @@ -73,8 +73,8 @@ export const SYNTAX_THEMES = [ export type SyntaxThemeName = (typeof SYNTAX_THEMES)[number]; -// Known light themes — used by the theme picker to show sun/moon icons -// for themes that haven't been loaded yet. +// Known light themes — used by the theme picker icons and to seed the root +// class before the full theme variables finish loading. export const LIGHT_THEMES: ReadonlySet = new Set([ "catppuccin-latte", "everforest-light", @@ -204,15 +204,29 @@ function stripAlpha(color: string): string { return color; } +function findColor( + colors: Record | undefined, + keys: readonly string[], +): string | null { + if (!colors) { + return null; + } + + for (const key of keys) { + const value = colors[key]; + if (value) { + return stripAlpha(value); + } + } + + return null; +} + function extractGitColors(colors: Record | undefined): { added: string | null; deleted: string | null; modified: string | null; } { - if (!colors) { - return { added: null, deleted: null, modified: null }; - } - const addedKeys = [ "gitDecoration.addedResourceForeground", "editorGutter.addedBackground", @@ -228,18 +242,10 @@ function extractGitColors(colors: Record | undefined): { "editorGutter.modifiedBackground", ]; - const findColor = (keys: string[]): string | null => { - for (const key of keys) { - const value = colors[key]; - if (value) return stripAlpha(value); - } - return null; - }; - return { - added: findColor(addedKeys), - deleted: findColor(deletedKeys), - modified: findColor(modifiedKeys), + added: findColor(colors, addedKeys), + deleted: findColor(colors, deletedKeys), + modified: findColor(colors, modifiedKeys), }; } @@ -261,9 +267,8 @@ export function extractThemeInfo( (theme.colors?.["editor.background"] as string | undefined) || "#1e1e1e"; const fg = (theme.colors?.["editor.foreground"] as string | undefined) || "#d4d4d4"; - const gitColors = extractGitColors( - theme.colors as Record | undefined, - ); + const colors = theme.colors as Record | undefined; + const gitColors = extractGitColors(colors); return { name: themeName, bg, diff --git a/desktop/tests/e2e/profile.spec.ts b/desktop/tests/e2e/profile.spec.ts index 921e712fe..9f9c0860a 100644 --- a/desktop/tests/e2e/profile.spec.ts +++ b/desktop/tests/e2e/profile.spec.ts @@ -276,7 +276,7 @@ test("desktop notification clicks open the matching forum thread", async ({ ).toBeVisible(); }); -test("opens settings with the keyboard shortcut and updates theme", async ({ +test("opens settings with the keyboard shortcut, applies the selected theme, and preserves the user accent", async ({ page, }) => { await page.goto("/"); @@ -290,14 +290,14 @@ test("opens settings with the keyboard shortcut and updates theme", async ({ await expect(page.getByTestId("settings-nav-appearance")).toBeVisible(); await page.getByTestId("settings-nav-appearance").click(); - // Default theme is catppuccin-macchiato (dark) + // The default theme is applied directly on load. await expect .poll(() => page.evaluate(() => document.documentElement.classList.contains("dark")), ) .toBe(true); - // Switch to a light theme — verifies dark→light transition + // Selecting a theme applies it directly and persists the choice. await page.getByTestId("theme-option-github-light").click(); await expect @@ -306,27 +306,36 @@ test("opens settings with the keyboard shortcut and updates theme", async ({ ) .toBe(true); - await expect - .poll(() => - page.evaluate(() => document.documentElement.classList.contains("dark")), - ) - .toBe(false); - - // CSS variables are set on the root element (the real theming mechanism) - await expect - .poll(() => - page.evaluate(() => - document.documentElement.style.getPropertyValue("--background").trim(), - ), - ) - .toBeTruthy(); - - // Theme name persists in localStorage await expect .poll(() => page.evaluate(() => localStorage.getItem("sprout-theme"))) .toBe("github-light"); - // Switch back to a dark theme — verifies light→dark transition + const primaryBeforeAccent = await page.evaluate(() => + document.documentElement.style.getPropertyValue("--primary").trim(), + ); + expect(primaryBeforeAccent).toBeTruthy(); + + await page.getByTestId("accent-color-red").click(); + + await expect + .poll(() => + page.evaluate(() => localStorage.getItem("sprout-accent-color")), + ) + .toBe("#ef4444"); + + await expect + .poll(() => + page.evaluate(() => + document.documentElement.style.getPropertyValue("--primary").trim(), + ), + ) + .not.toBe(primaryBeforeAccent); + + const redAccent = await page.evaluate(() => + document.documentElement.style.getPropertyValue("--primary").trim(), + ); + expect(redAccent).toBeTruthy(); + await page.getByTestId("theme-option-dracula").click(); await expect @@ -339,6 +348,14 @@ test("opens settings with the keyboard shortcut and updates theme", async ({ .poll(() => page.evaluate(() => localStorage.getItem("sprout-theme"))) .toBe("dracula"); + await expect + .poll(() => + page.evaluate(() => + document.documentElement.style.getPropertyValue("--primary").trim(), + ), + ) + .toBe(redAccent); + // Close settings with keyboard shortcut await page.keyboard.press( process.platform === "darwin" ? "Meta+," : "Control+,",