Skip to content

Commit a3e1d4f

Browse files
committed
refactor: simplify ThemeContext API - revert setThemePreference to setTheme
- ThemeMode now includes 'system' (user's selection) - ResolvedThemeMode for the actual applied theme (never 'system') - setTheme/theme for the user's preference - resolvedTheme for places that need the actual applied theme - Net LoC reduction (-36 lines)
1 parent f3807fd commit a3e1d4f

File tree

9 files changed

+83
-119
lines changed

9 files changed

+83
-119
lines changed

.storybook/preview.tsx

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import React from "react";
22
import type { Preview } from "@storybook/react-vite";
3-
import { ThemeProvider, type ThemeMode } from "../src/browser/contexts/ThemeContext";
3+
import { ThemeProvider, type ResolvedThemeMode } from "../src/browser/contexts/ThemeContext";
44
import "../src/browser/styles/globals.css";
55
import {
66
TUTORIAL_STATE_KEY,
@@ -9,7 +9,6 @@ import {
99
} from "../src/common/constants/storage";
1010
import { NOW } from "../src/browser/stories/mockFactory";
1111

12-
1312
const STORYBOOK_FONTS_READY_TIMEOUT_MS = 2500;
1413

1514
let fontsReadyPromise: Promise<void> | null = null;
@@ -92,7 +91,7 @@ const preview: Preview = {
9291
// Theme provider
9392
(Story, context) => {
9493
// Default to dark if mode not set (e.g., Chromatic headless browser defaults to light)
95-
const mode = (context.globals.theme as ThemeMode | undefined) ?? "dark";
94+
const mode = (context.globals.theme as ResolvedThemeMode | undefined) ?? "dark";
9695

9796
// Apply theme synchronously before React renders - critical for Chromatic snapshots
9897
if (typeof document !== "undefined") {

src/browser/App.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ function AppInner() {
7676
pendingNewWorkspaceSectionId,
7777
beginWorkspaceCreation,
7878
} = useWorkspaceContext();
79-
const { themePreference, setThemePreference, toggleTheme } = useTheme();
79+
const { theme, setTheme, toggleTheme } = useTheme();
8080
const { open: openSettings } = useSettings();
8181
const { api, status, error, authenticate } = useAPI();
8282

@@ -432,7 +432,7 @@ function AppInner() {
432432
projects,
433433
workspaceMetadata,
434434
selectedWorkspace,
435-
themePreference,
435+
theme,
436436
getThinkingLevel: getThinkingLevelForWorkspace,
437437
onSetThinkingLevel: setThinkingLevelFromPalette,
438438
onStartWorkspaceCreation: openNewWorkspaceFromPalette,
@@ -451,7 +451,7 @@ function AppInner() {
451451
});
452452
},
453453
onToggleTheme: toggleTheme,
454-
onSetThemePreference: setThemePreference,
454+
onSetTheme: setTheme,
455455
onOpenSettings: openSettings,
456456
onClearTimingStats: (workspaceId: string) => workspaceStore.clearTimingStats(workspaceId),
457457
api,

src/browser/components/Settings/sections/GeneralSection.tsx

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,5 @@
11
import React, { useEffect, useState, useCallback } from "react";
2-
import {
3-
useTheme,
4-
THEME_PREFERENCE_OPTIONS,
5-
type ThemePreference,
6-
} from "@/browser/contexts/ThemeContext";
2+
import { useTheme, THEME_OPTIONS, type ThemeMode } from "@/browser/contexts/ThemeContext";
73
import {
84
Select,
95
SelectContent,
@@ -32,7 +28,7 @@ const EDITOR_OPTIONS: Array<{ value: EditorType; label: string }> = [
3228
const isBrowserMode = typeof window !== "undefined" && !window.api;
3329

3430
export function GeneralSection() {
35-
const { themePreference, setThemePreference } = useTheme();
31+
const { theme, setTheme } = useTheme();
3632
const { api } = useAPI();
3733
const [editorConfig, setEditorConfig] = usePersistedState<EditorConfig>(
3834
EDITOR_CONFIG_KEY,
@@ -77,15 +73,12 @@ export function GeneralSection() {
7773
<div className="text-foreground text-sm">Theme</div>
7874
<div className="text-muted text-xs">Choose your preferred theme</div>
7975
</div>
80-
<Select
81-
value={themePreference}
82-
onValueChange={(value) => setThemePreference(value as ThemePreference)}
83-
>
76+
<Select value={theme} onValueChange={(value) => setTheme(value as ThemeMode)}>
8477
<SelectTrigger className="border-border-medium bg-background-secondary hover:bg-hover h-9 w-auto cursor-pointer rounded-md border px-3 text-sm transition-colors">
8578
<SelectValue />
8679
</SelectTrigger>
8780
<SelectContent>
88-
{THEME_PREFERENCE_OPTIONS.map((option) => (
81+
{THEME_OPTIONS.map((option) => (
8982
<SelectItem key={option.value} value={option.value}>
9083
{option.label}
9184
</SelectItem>

src/browser/components/ThemeSelector.tsx

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,4 @@
1-
import {
2-
useTheme,
3-
THEME_PREFERENCE_OPTIONS,
4-
type ThemePreference,
5-
} from "@/browser/contexts/ThemeContext";
1+
import { useTheme, THEME_OPTIONS, type ThemeMode } from "@/browser/contexts/ThemeContext";
62
import { Tooltip, TooltipTrigger, TooltipContent } from "./ui/tooltip";
73
import {
84
Select,
@@ -13,17 +9,13 @@ import {
139
} from "@/browser/components/ui/select";
1410

1511
export function ThemeSelector() {
16-
const { themePreference, setThemePreference } = useTheme();
17-
const currentLabel =
18-
THEME_PREFERENCE_OPTIONS.find((t) => t.value === themePreference)?.label ?? themePreference;
12+
const { theme, setTheme } = useTheme();
13+
const currentLabel = THEME_OPTIONS.find((t) => t.value === theme)?.label ?? theme;
1914

2015
return (
2116
<Tooltip>
2217
<TooltipTrigger asChild>
23-
<Select
24-
value={themePreference}
25-
onValueChange={(value) => setThemePreference(value as ThemePreference)}
26-
>
18+
<Select value={theme} onValueChange={(value) => setTheme(value as ThemeMode)}>
2719
<SelectTrigger
2820
className="border-border-light text-muted-foreground hover:border-border-medium/80 hover:bg-toggle-bg/70 h-5 w-auto cursor-pointer border bg-transparent px-1.5 text-[11px] transition-colors duration-150"
2921
aria-label="Select theme"
@@ -32,7 +24,7 @@ export function ThemeSelector() {
3224
<SelectValue />
3325
</SelectTrigger>
3426
<SelectContent>
35-
{THEME_PREFERENCE_OPTIONS.map((option) => (
27+
{THEME_OPTIONS.map((option) => (
3628
<SelectItem key={option.value} value={option.value}>
3729
{option.label}
3830
</SelectItem>

src/browser/components/ThemeToggleButton.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@ import { useTheme } from "@/browser/contexts/ThemeContext";
33
import { Tooltip, TooltipTrigger, TooltipContent } from "./ui/tooltip";
44

55
export function ThemeToggleButton() {
6-
const { theme, toggleTheme } = useTheme();
7-
const isLightTheme = theme === "light" || theme === "flexoki-light";
6+
const { resolvedTheme, toggleTheme } = useTheme();
7+
const isLightTheme = resolvedTheme === "light" || resolvedTheme === "flexoki-light";
88
const label = isLightTheme ? "Switch to dark theme" : "Switch to light theme";
99
const Icon = isLightTheme ? MoonStar : SunMedium;
1010

src/browser/contexts/ThemeContext.test.tsx

Lines changed: 12 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,12 @@ import { UI_THEME_KEY } from "@/common/constants/storage";
1818

1919
// Helper to access internals
2020
const TestComponent = () => {
21-
const { theme, toggleTheme } = useTheme();
21+
const { theme, resolvedTheme, toggleTheme } = useTheme();
2222
return (
2323
<div>
24-
<span data-testid="theme-value">{theme}</span>
25-
<button onClick={toggleTheme} data-testid="toggle-btn">
26-
Toggle
27-
</button>
24+
<span data-testid="theme">{theme}</span>
25+
<span data-testid="resolved">{resolvedTheme}</span>
26+
<button onClick={toggleTheme}>Toggle</button>
2827
</div>
2928
);
3029
};
@@ -65,44 +64,26 @@ describe("ThemeContext", () => {
6564
}
6665
});
6766

68-
test("uses persisted state by default", () => {
67+
test("defaults to system theme", () => {
6968
const { getByTestId } = render(
7069
<ThemeProvider>
7170
<TestComponent />
7271
</ThemeProvider>
7372
);
74-
// If matchMedia matches is false (default mock), resolveSystemTheme returns 'dark' (since it checks prefers-color-scheme: light)
75-
// resolveSystemTheme logic: window.matchMedia("(prefers-color-scheme: light)").matches ? "light" : "dark"
76-
expect(getByTestId("theme-value").textContent).toBe("dark");
73+
// Default is "system", which resolves to "dark" when matchMedia returns false for prefers-color-scheme: light
74+
expect(getByTestId("theme").textContent).toBe("system");
75+
expect(getByTestId("resolved").textContent).toBe("dark");
7776
});
7877

79-
test("respects forcedTheme prop", () => {
80-
const { getByTestId, rerender } = render(
81-
<ThemeProvider forcedTheme="light">
82-
<TestComponent />
83-
</ThemeProvider>
84-
);
85-
expect(getByTestId("theme-value").textContent).toBe("light");
86-
87-
rerender(
88-
<ThemeProvider forcedTheme="dark">
89-
<TestComponent />
90-
</ThemeProvider>
91-
);
92-
expect(getByTestId("theme-value").textContent).toBe("dark");
93-
});
94-
95-
test("forcedTheme overrides persisted state", () => {
78+
test("forcedTheme overrides resolved theme without changing preference", () => {
9679
window.localStorage.setItem(UI_THEME_KEY, JSON.stringify("light"));
97-
9880
const { getByTestId } = render(
9981
<ThemeProvider forcedTheme="dark">
10082
<TestComponent />
10183
</ThemeProvider>
10284
);
103-
expect(getByTestId("theme-value").textContent).toBe("dark");
104-
105-
// Check that localStorage is still light (since forcedTheme doesn't write to storage by itself)
106-
expect(JSON.parse(window.localStorage.getItem(UI_THEME_KEY)!)).toBe("light");
85+
// Preference unchanged, but resolved theme is forced
86+
expect(getByTestId("theme").textContent).toBe("light");
87+
expect(getByTestId("resolved").textContent).toBe("dark");
10788
});
10889
});

0 commit comments

Comments
 (0)