Skip to content

Commit 2b4665d

Browse files
committed
[dark-mode] Phase 3.5: Cross-browser compatibility tests and ThemeManager invalid-value bugfix
Add 19 browser-compat tests run across Chromium, Firefox, and WebKit (57 total cross-browser runs). Fix ThemeManager bug where invalid localStorage values caused applyTheme(null). All 139 tests passing. ThemeManager bugfix (themeManager.js): - init() now handles invalid localStorage values: hasSavedPreference() returning true while getSavedPreference() returns null correctly falls back to system preference or light default Playwright config (playwright.config.ts): - Add chromium-compat, firefox-compat, webkit-compat projects - browser-compat.spec.ts runs on all 3 browsers - Existing chromium project excludes compat tests to avoid duplication New test file (e2e/dark-mode/browser-compat.spec.ts — 19 tests x 3 browsers): - CSS custom properties: light/dark resolution, [data-theme] selector override - Theme toggle: click both directions, icon swap, keyboard Enter/Space - localStorage: save, restore after reload, API availability check - System preference: matchMedia API, prefers-color-scheme dark/light emulation - Rendered colors: body bg dark/light, text color, header bg - API support: CustomEvent dispatch, MutationObserver attribute detection Updated tests (theme-persistence.spec.ts): - Invalid value test updated: now asserts fallback to light mode
1 parent 8cc3574 commit 2b4665d

5 files changed

Lines changed: 301 additions & 11 deletions

File tree

.github/specs/dark-mode/documentation/3-progress-report.md

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@
3131
| **3.2** | Accessibility testing (WCAG AA contrast, keyboard nav, focus visibility, ARIA) | ✅ Done |
3232
| **3.3** | Performance testing (< 100ms theme switch, no layout shifts, CSS architecture) | ✅ Done |
3333
| **3.4** | localStorage persistence testing (edge cases, cross-navigation, validation) | ✅ Done |
34-
| **3.5** | Browser compatibility testing | Not Started |
34+
| **3.5** | Browser compatibility testing (Chromium, Firefox, WebKit) | ✅ Done |
3535
| **4.1** | Code review & cleanup | Not Started |
3636
| **4.2** | Unit tests for ThemeManager | Not Started |
3737
| **4.3** | Developer documentation | Not Started |
@@ -126,7 +126,7 @@ Fixes applied to both `[data-theme="dark"]` and `@media (prefers-color-scheme: d
126126
| Cross-page navigation persistence | Theme survives navigating away and back |
127127
| Clearing localStorage resets to light | `localStorage.clear()` → default light |
128128
| Removing only theme key resets | `removeItem()` → default light |
129-
| Invalid localStorage value handling | `'invalid-theme'` doesn't crash; toggle still works |
129+
| Invalid localStorage value handling | `'invalid-theme'` falls back to light mode (ThemeManager bugfix) |
130130
| Rapid toggles persist final state | 7 toggles → final value persisted and survives reload |
131131
| Storage format validation | Value is plain string, not JSON object |
132132
| No key leakage | Only `codex-docs-theme` key exists |
@@ -137,6 +137,33 @@ Fixes applied to both `[data-theme="dark"]` and `@media (prefers-color-scheme: d
137137

138138
---
139139

140+
## Phase 3.5 Completion Details — Browser Compatibility (NFR-3.3)
141+
142+
### ThemeManager Bugfix
143+
144+
Fixed `init()` to handle invalid localStorage values: `hasSavedPreference()` returning `true` while `getSavedPreference()` returns `null` now correctly falls back to system preference / light default instead of calling `applyTheme(null)`.
145+
146+
### Multi-Browser Playwright Config
147+
148+
Added 3 browser projects to `playwright.config.ts`:
149+
- `chromium-compat`, `firefox-compat`, `webkit-compat` — run only `browser-compat.spec.ts`
150+
- Existing `chromium` project excludes compat tests (avoids duplication)
151+
152+
### Cross-Browser Test Suite (19 tests × 3 browsers = 57 runs)
153+
154+
| Test Group | Tests | Coverage |
155+
|------------|-------|----------|
156+
| CSS Custom Properties | 3 | Light/dark variable resolution, `[data-theme]` selector override |
157+
| Theme Toggle | 4 | Click light→dark, dark→light, icon swap, keyboard Enter/Space |
158+
| localStorage Persistence | 3 | Save, restore after reload, API availability |
159+
| System Preference | 3 | `matchMedia` API, `prefers-color-scheme` dark/light emulation |
160+
| Rendered Colors | 4 | Body bg dark/light, text color, header bg |
161+
| CustomEvent & API | 2 | CustomEvent dispatch, MutationObserver attribute detection |
162+
163+
**Total test suite:** 139 tests (82 Chromium-only + 19×3 cross-browser) — all passing.
164+
165+
---
166+
140167
## Architecture Summary
141168

142169
- **Approach:** CSS custom properties + `[data-theme="dark"]` attribute on `<html>`
Lines changed: 248 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,248 @@
1+
import { test, expect, selectors, getCSSVariable, getThemeAttribute, goToThemedPage, STORAGE_KEY, themeColors } from '../fixtures/setup';
2+
3+
/**
4+
* Phase 3.5 — Browser Compatibility Tests (NFR-3.3)
5+
*
6+
* Run across Chromium, Firefox, and WebKit to verify:
7+
* - CSS custom properties work correctly
8+
* - data-theme attribute selector mechanism
9+
* - localStorage persistence
10+
* - matchMedia / prefers-color-scheme
11+
* - Theme toggle interaction
12+
* - No browser-specific rendering issues
13+
*/
14+
15+
test.describe('Cross-Browser: CSS Custom Properties', () => {
16+
test('CSS variables resolve in light mode', async ({ page }) => {
17+
await goToThemedPage(page);
18+
19+
for (const [variable, expected] of Object.entries(themeColors.light)) {
20+
const value = await getCSSVariable(page, variable);
21+
expect(value, `${variable} should be ${expected}`).toBe(expected);
22+
}
23+
});
24+
25+
test('CSS variables resolve in dark mode', async ({ page }) => {
26+
await goToThemedPage(page);
27+
await page.click(selectors.themeToggleButton);
28+
29+
for (const [variable, expected] of Object.entries(themeColors.dark)) {
30+
const value = await getCSSVariable(page, variable);
31+
expect(value, `${variable} should be ${expected}`).toBe(expected);
32+
}
33+
});
34+
35+
test('[data-theme] selector overrides :root values', async ({ page }) => {
36+
await goToThemedPage(page);
37+
38+
const result = await page.evaluate(() => {
39+
const before = getComputedStyle(document.documentElement)
40+
.getPropertyValue('--color-bg-main').trim().toLowerCase();
41+
42+
document.documentElement.setAttribute('data-theme', 'dark');
43+
44+
const after = getComputedStyle(document.documentElement)
45+
.getPropertyValue('--color-bg-main').trim().toLowerCase();
46+
47+
return { before, after };
48+
});
49+
50+
expect(result.before).toBe('#fff');
51+
expect(result.after).toBe('#18181b');
52+
});
53+
});
54+
55+
test.describe('Cross-Browser: Theme Toggle', () => {
56+
test('click toggles light to dark', async ({ page }) => {
57+
await goToThemedPage(page);
58+
expect(await getThemeAttribute(page)).toBe('light');
59+
60+
await page.click(selectors.themeToggleButton);
61+
expect(await getThemeAttribute(page)).toBe('dark');
62+
});
63+
64+
test('click toggles dark to light', async ({ page }) => {
65+
await goToThemedPage(page);
66+
await page.click(selectors.themeToggleButton);
67+
expect(await getThemeAttribute(page)).toBe('dark');
68+
69+
await page.click(selectors.themeToggleButton);
70+
expect(await getThemeAttribute(page)).toBe('light');
71+
});
72+
73+
test('icon visibility updates on toggle', async ({ page }) => {
74+
await goToThemedPage(page);
75+
76+
const sunIcon = page.locator(selectors.sunIcon);
77+
const moonIcon = page.locator(selectors.moonIcon);
78+
79+
await expect(sunIcon).toHaveCSS('display', 'block');
80+
await expect(moonIcon).toHaveCSS('display', 'none');
81+
82+
await page.click(selectors.themeToggleButton);
83+
84+
await expect(sunIcon).toHaveCSS('display', 'none');
85+
await expect(moonIcon).toHaveCSS('display', 'block');
86+
});
87+
88+
test('keyboard activation works', async ({ page }) => {
89+
await goToThemedPage(page);
90+
const button = page.locator(selectors.themeToggleButton);
91+
await button.focus();
92+
93+
await page.keyboard.press('Enter');
94+
expect(await getThemeAttribute(page)).toBe('dark');
95+
96+
await page.keyboard.press('Space');
97+
expect(await getThemeAttribute(page)).toBe('light');
98+
});
99+
});
100+
101+
test.describe('Cross-Browser: localStorage Persistence', () => {
102+
test('theme saves to localStorage', async ({ page }) => {
103+
await goToThemedPage(page);
104+
await page.click(selectors.themeToggleButton);
105+
106+
const stored = await page.evaluate((key) => localStorage.getItem(key), STORAGE_KEY);
107+
expect(stored).toBe('dark');
108+
});
109+
110+
test('theme restores from localStorage after reload', async ({ page }) => {
111+
await goToThemedPage(page);
112+
await page.click(selectors.themeToggleButton);
113+
114+
await page.reload();
115+
await page.waitForSelector(selectors.themeToggleButton, { timeout: 10000 });
116+
117+
expect(await getThemeAttribute(page)).toBe('dark');
118+
});
119+
120+
test('localStorage API is available', async ({ page }) => {
121+
await goToThemedPage(page);
122+
123+
const available = await page.evaluate(() => {
124+
try {
125+
const testKey = '__storage_test__';
126+
localStorage.setItem(testKey, 'test');
127+
localStorage.removeItem(testKey);
128+
return true;
129+
} catch {
130+
return false;
131+
}
132+
});
133+
134+
expect(available).toBe(true);
135+
});
136+
});
137+
138+
test.describe('Cross-Browser: System Preference Detection', () => {
139+
test('matchMedia API is available', async ({ page }) => {
140+
await goToThemedPage(page);
141+
142+
const available = await page.evaluate(() => {
143+
return typeof window.matchMedia === 'function';
144+
});
145+
146+
expect(available).toBe(true);
147+
});
148+
149+
test('prefers-color-scheme: dark is detected when emulated', async ({ page }) => {
150+
await goToThemedPage(page);
151+
await page.evaluate((key) => localStorage.removeItem(key), STORAGE_KEY);
152+
153+
await page.emulateMedia({ colorScheme: 'dark' });
154+
155+
await page.reload();
156+
await page.waitForSelector(selectors.themeToggleButton, { timeout: 10000 });
157+
158+
expect(await getThemeAttribute(page)).toBe('dark');
159+
});
160+
161+
test('prefers-color-scheme: light is detected when emulated', async ({ page }) => {
162+
await goToThemedPage(page);
163+
await page.evaluate((key) => localStorage.removeItem(key), STORAGE_KEY);
164+
165+
await page.emulateMedia({ colorScheme: 'light' });
166+
167+
await page.reload();
168+
await page.waitForSelector(selectors.themeToggleButton, { timeout: 10000 });
169+
170+
expect(await getThemeAttribute(page)).toBe('light');
171+
});
172+
});
173+
174+
test.describe('Cross-Browser: Rendered Colors', () => {
175+
test('body background is correct in dark mode', async ({ page }) => {
176+
await goToThemedPage(page);
177+
await page.click(selectors.themeToggleButton);
178+
179+
const bodyBg = await page.evaluate(() => getComputedStyle(document.body).backgroundColor);
180+
expect(bodyBg).toBe('rgb(24, 24, 27)');
181+
});
182+
183+
test('body background is correct in light mode', async ({ page }) => {
184+
await goToThemedPage(page);
185+
186+
const bodyBg = await page.evaluate(() => getComputedStyle(document.body).backgroundColor);
187+
expect(bodyBg).toBe('rgb(255, 255, 255)');
188+
});
189+
190+
test('text color updates in dark mode', async ({ page }) => {
191+
await goToThemedPage(page);
192+
await page.click(selectors.themeToggleButton);
193+
194+
const bodyColor = await page.evaluate(() => getComputedStyle(document.body).color);
195+
expect(bodyColor).toBe('rgb(228, 228, 231)');
196+
});
197+
198+
test('header background matches theme', async ({ page }) => {
199+
await goToThemedPage(page);
200+
await page.click(selectors.themeToggleButton);
201+
202+
const headerBg = await page.evaluate(() => {
203+
const header = document.querySelector('header.docs-header');
204+
return header ? getComputedStyle(header).backgroundColor : '';
205+
});
206+
expect(headerBg).toBe('rgb(24, 24, 27)');
207+
});
208+
});
209+
210+
test.describe('Cross-Browser: CustomEvent & API Support', () => {
211+
test('CustomEvent dispatches correctly', async ({ page }) => {
212+
await goToThemedPage(page);
213+
214+
const received = await page.evaluate(() => {
215+
return new Promise<boolean>((resolve) => {
216+
document.addEventListener('themeChange', (e: Event) => {
217+
resolve((e as CustomEvent).detail?.theme === 'test');
218+
});
219+
document.dispatchEvent(new CustomEvent('themeChange', {
220+
detail: { theme: 'test' },
221+
}));
222+
});
223+
});
224+
225+
expect(received).toBe(true);
226+
});
227+
228+
test('MutationObserver detects data-theme attribute changes', async ({ page }) => {
229+
await goToThemedPage(page);
230+
231+
const detected = await page.evaluate(() => {
232+
return new Promise<boolean>((resolve) => {
233+
const observer = new MutationObserver((mutations) => {
234+
for (const m of mutations) {
235+
if (m.attributeName === 'data-theme') {
236+
observer.disconnect();
237+
resolve(true);
238+
}
239+
}
240+
});
241+
observer.observe(document.documentElement, { attributes: true });
242+
document.documentElement.setAttribute('data-theme', 'dark');
243+
});
244+
});
245+
246+
expect(detected).toBe(true);
247+
});
248+
});

e2e/dark-mode/theme-persistence.spec.ts

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ test.describe('Theme Persistence — Edge Cases (FR-2.2)', () => {
9696
expect(await getThemeAttribute(page)).toBe('light');
9797
});
9898

99-
test('invalid localStorage value does not crash theme initialization', async ({ page }) => {
99+
test('invalid localStorage value falls back to light mode', async ({ page }) => {
100100
await page.goto('/auth');
101101
await page.waitForSelector(selectors.themeToggleButton, { timeout: 10000 });
102102

@@ -105,10 +105,9 @@ test.describe('Theme Persistence — Edge Cases (FR-2.2)', () => {
105105
await page.reload();
106106
await page.waitForSelector(selectors.themeToggleButton, { timeout: 10000 });
107107

108-
// Page should still work — toggle should still function
109-
await page.click(selectors.themeToggleButton);
110-
const stored = await page.evaluate((key) => localStorage.getItem(key), STORAGE_KEY);
111-
expect(['light', 'dark']).toContain(stored);
108+
// Invalid value should be ignored — falls back to system pref or light
109+
const theme = await getThemeAttribute(page);
110+
expect(theme).toBe('light');
112111
});
113112

114113
test('rapid toggles persist the final state', async ({ page }) => {

playwright.config.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,22 @@ export default defineConfig({
1515
{
1616
name: 'chromium',
1717
use: { ...devices['Desktop Chrome'] },
18+
testIgnore: /browser-compat/,
19+
},
20+
{
21+
name: 'chromium-compat',
22+
use: { ...devices['Desktop Chrome'] },
23+
testMatch: /browser-compat/,
24+
},
25+
{
26+
name: 'firefox-compat',
27+
use: { ...devices['Desktop Firefox'] },
28+
testMatch: /browser-compat/,
29+
},
30+
{
31+
name: 'webkit-compat',
32+
use: { ...devices['Desktop Safari'] },
33+
testMatch: /browser-compat/,
1834
},
1935
],
2036
webServer: {

src/frontend/js/modules/themeManager.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,12 +44,12 @@ class ThemeManager {
4444

4545
try {
4646
// Determine which theme to use based on priority:
47-
// 1. Saved preference in localStorage
47+
// 1. Saved preference in localStorage (if valid)
4848
// 2. System preference (prefers-color-scheme)
4949
// 3. Default to light mode
50-
const theme = this.hasSavedPreference()
51-
? this.getSavedPreference()
52-
: (this.getSystemPreference() ? ThemeManager.THEMES.DARK : ThemeManager.THEMES.LIGHT);
50+
const saved = this.hasSavedPreference() ? this.getSavedPreference() : null;
51+
const theme = saved
52+
|| (this.getSystemPreference() ? ThemeManager.THEMES.DARK : ThemeManager.THEMES.LIGHT);
5353

5454
// Apply theme synchronously to prevent visual flicker
5555
this.applyTheme(theme);

0 commit comments

Comments
 (0)