Skip to content

Commit e7243fb

Browse files
Harden early theme init against storage access failures
1 parent 974f03f commit e7243fb

6 files changed

Lines changed: 198 additions & 22 deletions

File tree

frontend/charts.js

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,11 @@ let radarChartInstance = null;
1212
let barChartInstance = null;
1313
let chartJsLoadPromise = null;
1414
let latestRenderToken = 0;
15+
let cachedChartPalette = null;
1516

1617
if (typeof document !== 'undefined') {
1718
document.addEventListener('themechange', () => {
19+
cachedChartPalette = null;
1820
applyThemeToExistingCharts();
1921
});
2022
}
@@ -30,8 +32,10 @@ export function generateCharts(subjects) {
3032
ensureChartJsLoaded()
3133
.then((ChartCtor) => {
3234
if (renderToken !== latestRenderToken) return;
33-
updateRadarChart(ChartCtor, labels, myScores, avgScores);
34-
updateBarChart(ChartCtor, labels, myScores, avgScores);
35+
36+
const palette = getCachedChartThemePalette();
37+
updateRadarChart(ChartCtor, labels, myScores, avgScores, palette);
38+
updateBarChart(ChartCtor, labels, myScores, avgScores, palette);
3539
})
3640
.catch((error) => {
3741
console.warn('Failed to load Chart.js', error);
@@ -104,13 +108,11 @@ function ensureChartJsLoaded() {
104108
return chartJsLoadPromise;
105109
}
106110

107-
function updateRadarChart(ChartCtor, labels, myScores, avgScores) {
111+
function updateRadarChart(ChartCtor, labels, myScores, avgScores, palette) {
108112
const radarCanvas = document.getElementById('radarChart');
109113
const radarCtx = radarCanvas?.getContext('2d');
110114
if (!radarCtx) return;
111115

112-
const palette = getChartThemePalette();
113-
114116
if (!radarChartInstance) {
115117
radarChartInstance = new ChartCtor(radarCtx, {
116118
type: 'radar',
@@ -190,13 +192,11 @@ function updateRadarChart(ChartCtor, labels, myScores, avgScores) {
190192
radarChartInstance.update('none');
191193
}
192194

193-
function updateBarChart(ChartCtor, labels, myScores, avgScores) {
195+
function updateBarChart(ChartCtor, labels, myScores, avgScores, palette) {
194196
const barCanvas = document.getElementById('barChart');
195197
const barCtx = barCanvas?.getContext('2d');
196198
if (!barCtx) return;
197199

198-
const palette = getChartThemePalette();
199-
200200
if (!barChartInstance) {
201201
barChartInstance = new ChartCtor(barCtx, {
202202
type: 'bar',
@@ -280,12 +280,19 @@ function getChartThemePalette() {
280280
textMuted: readVar('--color-text-muted', '#64748b'),
281281
surface: readVar('--color-surface-elevated', '#ffffff'),
282282
grid: readVar('--color-border-subtle', 'rgba(148, 163, 184, 0.22)'),
283-
gridSubtle: readVar('--color-border-subtle', 'rgba(148, 163, 184, 0.16)')
283+
gridSubtle: readVar('--color-border-extra-subtle', 'rgba(148, 163, 184, 0.16)')
284284
};
285285
}
286286

287+
function getCachedChartThemePalette() {
288+
if (!cachedChartPalette) {
289+
cachedChartPalette = getChartThemePalette();
290+
}
291+
return cachedChartPalette;
292+
}
293+
287294
function applyThemeToExistingCharts() {
288-
const palette = getChartThemePalette();
295+
const palette = getCachedChartThemePalette();
289296

290297
if (radarChartInstance) {
291298
const radarOptions = radarChartInstance.options;
@@ -330,6 +337,11 @@ function applyThemeToExistingCharts() {
330337
}
331338
}
332339

340+
export function __setChartInstancesForTest(instances = {}) {
341+
if (Object.hasOwn(instances, 'radar')) radarChartInstance = instances.radar;
342+
if (Object.hasOwn(instances, 'bar')) barChartInstance = instances.bar;
343+
}
344+
333345
function clearCanvas(canvasId) {
334346
const canvas = document.getElementById(canvasId);
335347
const context = canvas?.getContext?.('2d');

frontend/styles/tokens.css

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@
5454
/* Separator */
5555
--color-border: var(--md-sys-color-outline-variant);
5656
--color-border-subtle: rgba(142, 144, 153, 0.2);
57+
--color-border-extra-subtle: rgba(142, 144, 153, 0.14);
5758

5859
/* Shadows */
5960
--shadow-soft: 0 1px 2px 0 rgba(0, 0, 0, 0.3), 0 1px 3px 1px rgba(0, 0, 0, 0.15);
@@ -122,6 +123,7 @@
122123
--color-surface-elevated: #ffffff;
123124
--color-surface-hover: #eef1fa;
124125
--color-border-subtle: rgba(116, 119, 127, 0.25);
126+
--color-border-extra-subtle: rgba(116, 119, 127, 0.16);
125127

126128
--shadow-soft: 0 1px 2px 0 rgba(24, 27, 33, 0.09), 0 1px 4px 1px rgba(24, 27, 33, 0.08);
127129
--shadow-hover: 0 1px 3px 0 rgba(24, 27, 33, 0.12), 0 4px 12px 2px rgba(24, 27, 33, 0.12);

frontend/theme.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
const STORAGE_KEY = 'grades-theme';
22

3-
function getInitialTheme() {
3+
export function getInitialTheme() {
44
const storedTheme = localStorage.getItem(STORAGE_KEY);
55
if (storedTheme === 'light' || storedTheme === 'dark') return storedTheme;
66
return 'dark';
77
}
88

9-
function applyTheme(theme) {
9+
export function applyTheme(theme) {
1010
const root = document.documentElement;
1111
if (theme === 'light') {
1212
root.setAttribute('data-theme', 'light');

public/index.html

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -39,15 +39,8 @@
3939
href="https://fonts.googleapis.com/css2?family=Roboto:wght@400;500;600;700&family=Noto+Sans+TC:wght@400;500;600;700&display=swap">
4040
</noscript>
4141
<script src="https://challenges.cloudflare.com/turnstile/v0/api.js?render=explicit" async defer></script>
42-
<script>
43-
(() => {
44-
const storedTheme = localStorage.getItem('grades-theme');
45-
if (storedTheme === 'light') {
46-
document.documentElement.setAttribute('data-theme', 'light');
47-
}
48-
})();
49-
</script>
50-
<link rel="stylesheet" href="/dist/main-DRLHkMzl.css" id="vite-css">
42+
<script src="/theme-init.js"></script>
43+
<link rel="stylesheet" href="/dist/main-CoyldwFZ.css" id="vite-css">
5144
<link rel="icon" type="image/x-icon" href="/favicon.ico">
5245
</head>
5346

@@ -530,7 +523,7 @@ <h3 class="section-title">
530523
</footer>
531524
</div>
532525

533-
<script type="module" src="/dist/main-BwIAXOgl.js"></script>
526+
<script type="module" src="/dist/main-1Jkr9rBG.js"></script>
534527
</body>
535528

536529
</html>

public/theme-init.js

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
(() => {
2+
try {
3+
const storedTheme = localStorage.getItem('grades-theme');
4+
if (storedTheme === 'light') {
5+
document.documentElement.setAttribute('data-theme', 'light');
6+
}
7+
} catch {
8+
// Ignore storage access failures (privacy mode / blocked storage)
9+
// and keep default theme.
10+
}
11+
})();

tests/frontend/theme.test.js

Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,158 @@
1+
import { beforeEach, afterEach, describe, it } from 'node:test';
2+
import assert from 'node:assert/strict';
3+
import { JSDOM } from 'jsdom';
4+
import { getInitialTheme, applyTheme, setupThemeToggle } from '../../frontend/theme.js';
5+
6+
let dom;
7+
let cleanupGlobals = [];
8+
9+
function installDom(html = '<!doctype html><html><body></body></html>') {
10+
dom = new JSDOM(html, { url: 'https://example.test' });
11+
const map = {
12+
window: dom.window,
13+
document: dom.window.document,
14+
localStorage: dom.window.localStorage,
15+
CustomEvent: dom.window.CustomEvent,
16+
getComputedStyle: dom.window.getComputedStyle
17+
};
18+
19+
cleanupGlobals = Object.entries(map).map(([key, value]) => {
20+
const previous = globalThis[key];
21+
globalThis[key] = value;
22+
return [key, previous];
23+
});
24+
}
25+
26+
function uninstallDom() {
27+
for (const [key, previous] of cleanupGlobals) {
28+
if (previous === undefined) {
29+
delete globalThis[key];
30+
} else {
31+
globalThis[key] = previous;
32+
}
33+
}
34+
cleanupGlobals = [];
35+
dom?.window?.close();
36+
}
37+
38+
describe('theme.js', () => {
39+
beforeEach(() => {
40+
installDom(`<!doctype html><html><body>
41+
<button id="themeToggleBtn" aria-label="" title="">
42+
<span class="theme-icon theme-icon-moon"></span>
43+
<span class="theme-icon theme-icon-sun hidden"></span>
44+
</button>
45+
</body></html>`);
46+
});
47+
48+
afterEach(() => {
49+
uninstallDom();
50+
});
51+
52+
it('getInitialTheme should handle stored light/dark/invalid/missing values', async () => {
53+
localStorage.setItem('grades-theme', 'light');
54+
assert.equal(getInitialTheme(), 'light');
55+
56+
localStorage.setItem('grades-theme', 'dark');
57+
assert.equal(getInitialTheme(), 'dark');
58+
59+
localStorage.setItem('grades-theme', 'unexpected');
60+
assert.equal(getInitialTheme(), 'dark');
61+
62+
localStorage.removeItem('grades-theme');
63+
assert.equal(getInitialTheme(), 'dark');
64+
});
65+
66+
it('applyTheme should update data-theme, icon states, aria-label and title', async () => {
67+
const root = document.documentElement;
68+
const btn = document.getElementById('themeToggleBtn');
69+
const moon = btn.querySelector('.theme-icon-moon');
70+
const sun = btn.querySelector('.theme-icon-sun');
71+
72+
applyTheme('light');
73+
assert.equal(root.getAttribute('data-theme'), 'light');
74+
assert.equal(sun.classList.contains('hidden'), false);
75+
assert.equal(moon.classList.contains('hidden'), true);
76+
assert.equal(btn.getAttribute('aria-label'), '切換至深色模式');
77+
assert.equal(btn.getAttribute('title'), '切換至深色模式');
78+
79+
applyTheme('dark');
80+
assert.equal(root.hasAttribute('data-theme'), false);
81+
assert.equal(sun.classList.contains('hidden'), true);
82+
assert.equal(moon.classList.contains('hidden'), false);
83+
assert.equal(btn.getAttribute('aria-label'), '切換至淺色模式');
84+
assert.equal(btn.getAttribute('title'), '切換至淺色模式');
85+
});
86+
87+
it('setupThemeToggle should toggle theme, persist localStorage and dispatch themechange', async () => {
88+
localStorage.setItem('grades-theme', 'dark');
89+
90+
const events = [];
91+
document.addEventListener('themechange', (event) => events.push(event.detail.theme));
92+
93+
setupThemeToggle();
94+
const btn = document.getElementById('themeToggleBtn');
95+
96+
btn.click();
97+
assert.equal(document.documentElement.getAttribute('data-theme'), 'light');
98+
assert.equal(localStorage.getItem('grades-theme'), 'light');
99+
100+
btn.click();
101+
assert.equal(document.documentElement.hasAttribute('data-theme'), false);
102+
assert.equal(localStorage.getItem('grades-theme'), 'dark');
103+
104+
assert.deepEqual(events, ['dark', 'light', 'dark']);
105+
});
106+
107+
it('chart integration should re-apply chart palette on themechange', async () => {
108+
document.documentElement.style.setProperty('--color-text-main', 'rgb(1, 2, 3)');
109+
document.documentElement.style.setProperty('--color-text-secondary', 'rgb(10, 20, 30)');
110+
document.documentElement.style.setProperty('--color-text-muted', 'rgb(40, 50, 60)');
111+
document.documentElement.style.setProperty('--color-surface-elevated', 'rgb(70, 80, 90)');
112+
document.documentElement.style.setProperty('--color-border-subtle', 'rgba(1,1,1,0.3)');
113+
document.documentElement.style.setProperty('--color-border-extra-subtle', 'rgba(2,2,2,0.2)');
114+
115+
const charts = await import('../../frontend/charts.js');
116+
117+
const radar = {
118+
options: {
119+
scales: { r: { ticks: {}, grid: {}, angleLines: {}, pointLabels: {} } },
120+
plugins: { legend: { labels: {} } }
121+
},
122+
data: { datasets: [{}, {}] },
123+
updateCalls: 0,
124+
update() { this.updateCalls += 1; }
125+
};
126+
127+
const bar = {
128+
options: {
129+
scales: { x: { ticks: {}, grid: {} }, y: { ticks: {}, grid: {} } },
130+
plugins: { legend: { labels: {} } }
131+
},
132+
data: { datasets: [] },
133+
updateCalls: 0,
134+
update() { this.updateCalls += 1; }
135+
};
136+
137+
charts.__setChartInstancesForTest({ radar, bar });
138+
applyTheme('light');
139+
140+
assert.equal(radar.options.scales.r.grid.color, 'rgba(1,1,1,0.3)');
141+
assert.equal(bar.options.scales.x.grid.color, 'rgba(2,2,2,0.2)');
142+
assert.equal(radar.data.datasets[0].pointBorderColor, 'rgb(70, 80, 90)');
143+
assert.equal(radar.updateCalls, 1);
144+
assert.equal(bar.updateCalls, 1);
145+
146+
document.documentElement.style.setProperty('--color-border-subtle', 'rgba(9,9,9,0.9)');
147+
document.documentElement.style.setProperty('--color-border-extra-subtle', 'rgba(8,8,8,0.8)');
148+
document.documentElement.style.setProperty('--color-surface-elevated', 'rgb(7, 8, 9)');
149+
150+
applyTheme('dark');
151+
152+
assert.equal(radar.options.scales.r.grid.color, 'rgba(9,9,9,0.9)');
153+
assert.equal(bar.options.scales.x.grid.color, 'rgba(8,8,8,0.8)');
154+
assert.equal(radar.data.datasets[0].pointBorderColor, 'rgb(7, 8, 9)');
155+
assert.equal(radar.updateCalls, 2);
156+
assert.equal(bar.updateCalls, 2);
157+
});
158+
});

0 commit comments

Comments
 (0)