Skip to content

Commit 8caaf30

Browse files
kantordkantord
authored
fix: app forgets light/dark mode selection (#484)
* fix: app forgets light/dark mode selection * . * . * . * cleanup * cleanup * cleanup
1 parent 93c19bb commit 8caaf30

File tree

2 files changed

+179
-16
lines changed

2 files changed

+179
-16
lines changed

‎renderer/src/common/components/theme/__tests__/theme-provider.test.tsx‎

Lines changed: 173 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,173 @@
1+
import { render, screen, waitFor, act, cleanup } from '@testing-library/react'
2+
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'
3+
import { ThemeProvider } from '../theme-provider'
4+
import { useTheme } from '../../../hooks/use-theme'
5+
6+
const mockElectronAPI = {
7+
darkMode: {
8+
get: vi.fn().mockResolvedValue({
9+
shouldUseDarkColors: false,
10+
themeSource: 'system',
11+
}),
12+
set: vi.fn().mockResolvedValue(true),
13+
},
14+
}
15+
16+
Object.defineProperty(window, 'electronAPI', {
17+
value: mockElectronAPI,
18+
writable: true,
19+
})
20+
21+
const matchMediaListeners: Record<string, Set<(e: Event) => void>> = {}
22+
const matchMediaMatches: Record<string, boolean> = {}
23+
Object.defineProperty(window, 'matchMedia', {
24+
writable: true,
25+
value: vi.fn().mockImplementation((query: string) => {
26+
matchMediaListeners[query] = matchMediaListeners[query] || new Set()
27+
matchMediaMatches[query] = matchMediaMatches[query] ?? false
28+
return {
29+
media: query,
30+
onchange: null,
31+
addListener: vi.fn(), // deprecated
32+
removeListener: vi.fn(), // deprecated
33+
addEventListener: (event: string, cb: (e: Event) => void) => {
34+
if (event === 'change') matchMediaListeners[query]?.add(cb)
35+
},
36+
removeEventListener: (event: string, cb: (e: Event) => void) => {
37+
if (event === 'change') matchMediaListeners[query]?.delete(cb)
38+
},
39+
dispatchEvent: (event: Event) => {
40+
if (event.type === 'change') {
41+
matchMediaListeners[query]?.forEach((cb) => cb(event))
42+
}
43+
},
44+
get matches() {
45+
return matchMediaMatches[query]
46+
},
47+
}
48+
}),
49+
})
50+
function setSystemTheme(isDark: boolean) {
51+
const query = '(prefers-color-scheme: dark)'
52+
matchMediaMatches[query] = isDark
53+
const event = new Event('change')
54+
Object.defineProperty(event, 'matches', { value: isDark })
55+
window.matchMedia(query).dispatchEvent(event)
56+
}
57+
58+
function TestComponent({ id = 'theme' }: { id?: string }) {
59+
const { theme, setTheme } = useTheme()
60+
return (
61+
<div>
62+
<span data-testid={id}>{theme}</span>
63+
<button onClick={() => setTheme('light')}>Light</button>
64+
<button onClick={() => setTheme('dark')}>Dark</button>
65+
<button onClick={() => setTheme('system')}>System</button>
66+
</div>
67+
)
68+
}
69+
70+
describe('<ThemeProvider />', () => {
71+
beforeEach(() => {
72+
localStorage.clear()
73+
vi.clearAllMocks()
74+
document.documentElement.className = ''
75+
mockElectronAPI.darkMode.get.mockResolvedValue({
76+
shouldUseDarkColors: false,
77+
themeSource: 'system',
78+
})
79+
})
80+
81+
afterEach(() => {
82+
cleanup()
83+
localStorage.clear()
84+
document.documentElement.className = ''
85+
})
86+
87+
it('keeps stored theme even if Electron reports something else', async () => {
88+
localStorage.setItem('toolhive-ui-theme', 'dark')
89+
mockElectronAPI.darkMode.get.mockResolvedValue({
90+
shouldUseDarkColors: false,
91+
themeSource: 'system',
92+
})
93+
94+
render(
95+
<ThemeProvider>
96+
<TestComponent id="stored" />
97+
</ThemeProvider>
98+
)
99+
100+
expect(screen.getByTestId('stored')).toHaveTextContent('dark')
101+
expect(document.documentElement).toHaveClass('dark')
102+
103+
await waitFor(() =>
104+
expect(screen.getByTestId('stored')).toHaveTextContent('dark')
105+
)
106+
107+
expect(localStorage.getItem('toolhive-ui-theme')).toBe('dark')
108+
})
109+
110+
it('seeds localStorage from Electron when nothing stored', async () => {
111+
mockElectronAPI.darkMode.get.mockResolvedValue({
112+
shouldUseDarkColors: true,
113+
themeSource: 'dark',
114+
})
115+
116+
render(
117+
<ThemeProvider>
118+
<TestComponent id="seed" />
119+
</ThemeProvider>
120+
)
121+
122+
await waitFor(() =>
123+
expect(screen.getByTestId('seed')).toHaveTextContent('dark')
124+
)
125+
126+
expect(localStorage.getItem('toolhive-ui-theme')).toBe('dark')
127+
expect(document.documentElement).toHaveClass('dark')
128+
})
129+
130+
it('setTheme(light) updates everything consistently', async () => {
131+
localStorage.setItem('toolhive-ui-theme', 'dark')
132+
render(
133+
<ThemeProvider>
134+
<TestComponent id="update" />
135+
</ThemeProvider>
136+
)
137+
138+
await act(async () => {
139+
screen.getByText('Light').click()
140+
})
141+
142+
await waitFor(() =>
143+
expect(screen.getByTestId('update')).toHaveTextContent('light')
144+
)
145+
146+
expect(document.documentElement).toHaveClass('light')
147+
expect(localStorage.getItem('toolhive-ui-theme')).toBe('light')
148+
expect(mockElectronAPI.darkMode.set).toHaveBeenCalledWith('light')
149+
})
150+
151+
it('listens to system theme changes when theme is system', async () => {
152+
render(
153+
<ThemeProvider>
154+
<TestComponent id="system" />
155+
</ThemeProvider>
156+
)
157+
await act(async () => {
158+
screen.getByText('System').click()
159+
})
160+
await waitFor(() =>
161+
expect(screen.getByTestId('system')).toHaveTextContent('system')
162+
)
163+
expect(document.documentElement).toHaveClass('light')
164+
165+
// Simulate system theme change to dark
166+
setSystemTheme(true)
167+
await waitFor(() => expect(document.documentElement).toHaveClass('dark'))
168+
169+
// Simulate system theme change back to light
170+
setSystemTheme(false)
171+
await waitFor(() => expect(document.documentElement).toHaveClass('light'))
172+
})
173+
})

‎renderer/src/common/components/theme/theme-provider.tsx‎

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -24,19 +24,16 @@ export function ThemeProvider({
2424
return isValidTheme(storedTheme) ? storedTheme : defaultTheme
2525
})
2626

27-
// Sync with Electron's native theme on mount
2827
useEffect(() => {
2928
const syncWithNativeTheme = async () => {
3029
if (window.electronAPI?.darkMode) {
3130
try {
32-
const nativeThemeState = await window.electronAPI.darkMode.get()
33-
const nativeThemeSource = nativeThemeState.themeSource
34-
35-
// Only sync if the stored theme doesn't match the native theme
3631
const storedTheme = localStorage.getItem(storageKey)
37-
if (!isValidTheme(storedTheme) || storedTheme !== nativeThemeSource) {
38-
setTheme(nativeThemeSource)
39-
localStorage.setItem(storageKey, nativeThemeSource)
32+
33+
if (!isValidTheme(storedTheme)) {
34+
const nativeThemeState = await window.electronAPI.darkMode.get()
35+
setTheme(nativeThemeState.themeSource)
36+
localStorage.setItem(storageKey, nativeThemeState.themeSource)
4037
}
4138
} catch (error) {
4239
console.warn('Failed to sync with native theme:', error)
@@ -49,23 +46,20 @@ export function ThemeProvider({
4946

5047
useEffect(() => {
5148
const root = window.document.documentElement
52-
5349
root.classList.remove('light', 'dark')
5450

5551
if (theme === 'system') {
5652
const systemTheme = window.matchMedia('(prefers-color-scheme: dark)')
5753
.matches
5854
? 'dark'
5955
: 'light'
60-
6156
root.classList.add(systemTheme)
6257
return
6358
}
6459

6560
root.classList.add(theme)
6661
}, [theme])
6762

68-
// Listen for system theme changes when theme is set to "system"
6963
useEffect(() => {
7064
if (theme !== 'system') return
7165

@@ -87,15 +81,11 @@ export function ThemeProvider({
8781
localStorage.setItem(storageKey, newTheme)
8882
setTheme(newTheme)
8983

90-
// Sync with Electron's native theme
9184
if (window.electronAPI?.darkMode) {
9285
try {
9386
await window.electronAPI.darkMode.set(newTheme)
9487
} catch (error) {
95-
console.warn(
96-
'Failed to sync theme with native Electron theme:',
97-
error
98-
)
88+
console.warn('Failed to sync theme with Electron:', error)
9989
}
10090
}
10191
},

0 commit comments

Comments
 (0)