Skip to content

Commit 2dbc126

Browse files
committed
🤖 fix: prevent nested ThemeProvider from overriding forced theme
ThemeProvider now detects when it's nested under a parent with forcedTheme and defers to that parent's theme value. This ensures Storybook/Chromatic's forced theme decorator isn't overridden by the app's own ThemeProvider. Changes: - ThemeProvider accepts forcedTheme prop and tracks isForced in context - Nested providers inherit parent's theme when parent has forcedTheme - Storybook decorator applies theme synchronously before React renders - Added tests for ThemeContext forced theme behavior _Generated with `mux`_
1 parent db9ecdb commit 2dbc126

File tree

4 files changed

+166
-35
lines changed

4 files changed

+166
-35
lines changed

.storybook/preview.tsx

Lines changed: 17 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,30 +1,13 @@
1-
import React, { useEffect } from "react";
1+
import React from "react";
22
import type { Preview } from "@storybook/react-vite";
3-
import {
4-
ThemeProvider,
5-
useTheme,
6-
type ThemeMode,
7-
} from "../src/browser/contexts/ThemeContext";
3+
import { ThemeProvider, type ThemeMode } from "../src/browser/contexts/ThemeContext";
84
import "../src/browser/styles/globals.css";
95

10-
const ThemeStorySync: React.FC<{ mode: ThemeMode }> = ({ mode }) => {
11-
const { theme, setTheme } = useTheme();
12-
13-
useEffect(() => {
14-
if (theme !== mode) {
15-
setTheme(mode);
16-
}
17-
}, [mode, setTheme, theme]);
18-
19-
return null;
20-
};
21-
226
const preview: Preview = {
237
globalTypes: {
248
theme: {
259
name: "Theme",
2610
description: "Choose between light and dark UI themes",
27-
defaultValue: "dark",
2811
toolbar: {
2912
icon: "mirror",
3013
items: [
@@ -35,12 +18,22 @@ const preview: Preview = {
3518
},
3619
},
3720
},
21+
initialGlobals: {
22+
theme: "dark",
23+
},
3824
decorators: [
3925
(Story, context) => {
40-
const mode = (context.globals.theme ?? "dark") as ThemeMode;
26+
// Default to dark if mode not set (e.g., Chromatic headless browser defaults to light)
27+
const mode = (context.globals.theme as ThemeMode | undefined) ?? "dark";
28+
29+
// Apply theme synchronously before React renders - critical for Chromatic snapshots
30+
if (typeof document !== "undefined") {
31+
document.documentElement.dataset.theme = mode;
32+
document.documentElement.style.colorScheme = mode;
33+
}
34+
4135
return (
42-
<ThemeProvider>
43-
<ThemeStorySync mode={mode} />
36+
<ThemeProvider forcedTheme={mode}>
4437
<Story />
4538
</ThemeProvider>
4639
);
@@ -55,8 +48,8 @@ const preview: Preview = {
5548
},
5649
chromatic: {
5750
modes: {
58-
dark: { globals: { theme: "dark" } },
59-
light: { globals: { theme: "light" } },
51+
dark: { theme: "dark" },
52+
light: { theme: "light" },
6053
},
6154
},
6255
},

bun.lock

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
{
22
"lockfileVersion": 1,
3-
"configVersion": 0,
43
"workspaces": {
54
"": {
65
"name": "@coder/cmux",
Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
import { GlobalWindow } from "happy-dom";
2+
3+
// Setup basic DOM environment for testing-library
4+
const dom = new GlobalWindow();
5+
/* eslint-disable @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access */
6+
(global as any).window = dom.window;
7+
(global as any).document = dom.window.document;
8+
// Polyfill console since happy-dom might interfere or we just want standard console
9+
(global as any).console = console;
10+
/* eslint-enable @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access */
11+
12+
import { afterEach, describe, expect, mock, test, beforeEach } from "bun:test";
13+
14+
import { render, cleanup } from "@testing-library/react";
15+
import React from "react";
16+
import { ThemeProvider, useTheme } from "./ThemeContext";
17+
import { UI_THEME_KEY } from "@/common/constants/storage";
18+
19+
// Helper to access internals
20+
const TestComponent = () => {
21+
const { theme, toggleTheme } = useTheme();
22+
return (
23+
<div>
24+
<span data-testid="theme-value">{theme}</span>
25+
<button onClick={toggleTheme} data-testid="toggle-btn">
26+
Toggle
27+
</button>
28+
</div>
29+
);
30+
};
31+
32+
describe("ThemeContext", () => {
33+
// Mock matchMedia
34+
const mockMatchMedia = mock(() => ({
35+
matches: false,
36+
media: "",
37+
onchange: null,
38+
addListener: () => {
39+
// no-op
40+
},
41+
removeListener: () => {
42+
// no-op
43+
},
44+
addEventListener: () => {
45+
// no-op
46+
},
47+
removeEventListener: () => {
48+
// no-op
49+
},
50+
dispatchEvent: () => true,
51+
}));
52+
53+
beforeEach(() => {
54+
// Ensure window exists (Bun test with happy-dom should provide it)
55+
if (typeof window !== "undefined") {
56+
window.matchMedia = mockMatchMedia;
57+
window.localStorage.clear();
58+
}
59+
});
60+
61+
afterEach(() => {
62+
cleanup();
63+
if (typeof window !== "undefined") {
64+
window.localStorage.clear();
65+
}
66+
});
67+
68+
test("uses persisted state by default", () => {
69+
const { getByTestId } = render(
70+
<ThemeProvider>
71+
<TestComponent />
72+
</ThemeProvider>
73+
);
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");
77+
});
78+
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", () => {
96+
window.localStorage.setItem(UI_THEME_KEY, JSON.stringify("light"));
97+
98+
const { getByTestId } = render(
99+
<ThemeProvider forcedTheme="dark">
100+
<TestComponent />
101+
</ThemeProvider>
102+
);
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");
107+
});
108+
});

src/browser/contexts/ThemeContext.tsx

Lines changed: 41 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ interface ThemeContextValue {
1515
theme: ThemeMode;
1616
setTheme: React.Dispatch<React.SetStateAction<ThemeMode>>;
1717
toggleTheme: () => void;
18+
/** True if this provider has a forcedTheme - nested providers should not override */
19+
isForced: boolean;
1820
}
1921

2022
const ThemeContext = createContext<ThemeContextValue | null>(null);
@@ -51,29 +53,58 @@ function applyThemeToDocument(theme: ThemeMode) {
5153
}
5254
}
5355

54-
export function ThemeProvider(props: { children: ReactNode }) {
55-
const [theme, setTheme] = usePersistedState<ThemeMode>(UI_THEME_KEY, resolveSystemTheme(), {
56-
listener: true,
57-
});
56+
export function ThemeProvider({
57+
children,
58+
forcedTheme,
59+
}: {
60+
children: ReactNode;
61+
forcedTheme?: ThemeMode;
62+
}) {
63+
// Check if we're nested inside a forced theme provider
64+
const parentContext = useContext(ThemeContext);
65+
const isNestedUnderForcedProvider = parentContext?.isForced ?? false;
66+
67+
const [persistedTheme, setTheme] = usePersistedState<ThemeMode>(
68+
UI_THEME_KEY,
69+
resolveSystemTheme(),
70+
{
71+
listener: true,
72+
}
73+
);
74+
75+
// If nested under a forced provider, use parent's theme
76+
// Otherwise, use forcedTheme (if provided) or persistedTheme
77+
const theme =
78+
isNestedUnderForcedProvider && parentContext
79+
? parentContext.theme
80+
: (forcedTheme ?? persistedTheme);
81+
82+
const isForced = forcedTheme !== undefined || isNestedUnderForcedProvider;
5883

84+
// Only apply to document if we're the authoritative provider
5985
useLayoutEffect(() => {
60-
applyThemeToDocument(theme);
61-
}, [theme]);
86+
if (!isNestedUnderForcedProvider) {
87+
applyThemeToDocument(theme);
88+
}
89+
}, [theme, isNestedUnderForcedProvider]);
6290

6391
const toggleTheme = useCallback(() => {
64-
setTheme((current) => (current === "dark" ? "light" : "dark"));
65-
}, [setTheme]);
92+
if (!isNestedUnderForcedProvider) {
93+
setTheme((current) => (current === "dark" ? "light" : "dark"));
94+
}
95+
}, [setTheme, isNestedUnderForcedProvider]);
6696

6797
const value = useMemo<ThemeContextValue>(
6898
() => ({
6999
theme,
70100
setTheme,
71101
toggleTheme,
102+
isForced,
72103
}),
73-
[setTheme, theme, toggleTheme]
104+
[setTheme, theme, toggleTheme, isForced]
74105
);
75106

76-
return <ThemeContext.Provider value={value}>{props.children}</ThemeContext.Provider>;
107+
return <ThemeContext.Provider value={value}>{children}</ThemeContext.Provider>;
77108
}
78109

79110
export function useTheme(): ThemeContextValue {

0 commit comments

Comments
 (0)