Skip to content

Commit b20bf1a

Browse files
Updated per CodeRabbit review
1 parent cbfab5b commit b20bf1a

2 files changed

Lines changed: 57 additions & 2 deletions

File tree

‎lib/report-view.js‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -451,6 +451,10 @@ export function destroyBubbleCharts() {
451451
bubbleCharts.clear();
452452
}
453453

454+
// The MediaQueryList for prefers-color-scheme, retained so the listener
455+
// (and the redraw callback it holds) stays reachable for the page lifetime.
456+
let systemThemeMediaQuery = null;
457+
454458
/**
455459
* Binds one delegated listener redrawing charts on theme changes: the
456460
* rf-theme radios (components/theme-toggle.jsx) for explicit switches and
@@ -468,8 +472,15 @@ export function bindThemeChartRedraw() {
468472
});
469473
if (typeof window !== 'undefined' && typeof window.matchMedia === 'function') {
470474
try {
471-
window.matchMedia('(prefers-color-scheme: dark)')
472-
.addEventListener('change', redrawChartsForTheme);
475+
// Retain the MediaQueryList and subscribe with the best listener API
476+
// it exposes: addEventListener, or the deprecated addListener on
477+
// engines that never grew the modern one.
478+
systemThemeMediaQuery = window.matchMedia('(prefers-color-scheme: dark)');
479+
if (systemThemeMediaQuery && typeof systemThemeMediaQuery.addEventListener === 'function') {
480+
systemThemeMediaQuery.addEventListener('change', redrawChartsForTheme);
481+
} else if (systemThemeMediaQuery && typeof systemThemeMediaQuery.addListener === 'function') {
482+
systemThemeMediaQuery.addListener(redrawChartsForTheme);
483+
}
473484
} catch {
474485
// Older engines without MQL listeners: explicit radios still redraw.
475486
}

‎tests/unit/report-view.test.js‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,50 @@ describe('theme-change chart redraw', () => {
197197
expect(updates.length).toBeGreaterThan(0);
198198
});
199199

200+
it('prefers addEventListener when the MediaQueryList supports both listener APIs', () => {
201+
installThemeRadios('system');
202+
const bound = [];
203+
window.matchMedia = () => ({
204+
matches: false,
205+
addEventListener(type, listener) { bound.push(['modern', type, listener]); },
206+
addListener(listener) { bound.push(['legacy', listener]); }
207+
});
208+
document.documentElement.removeAttribute('data-rf-theme-redraw-bound');
209+
bindThemeChartRedraw();
210+
211+
expect(bound).toHaveLength(1);
212+
expect(bound[0][0]).toBe('modern');
213+
expect(bound[0][1]).toBe('change');
214+
expect(bound[0][2]).toBeTypeOf('function');
215+
});
216+
217+
it('falls back to addListener when the MediaQueryList lacks addEventListener', () => {
218+
installThemeRadios('system');
219+
let legacyListener = null;
220+
window.matchMedia = () => ({
221+
matches: false,
222+
addListener(listener) { legacyListener = listener; }
223+
});
224+
document.documentElement.removeAttribute('data-rf-theme-redraw-bound');
225+
bindThemeChartRedraw();
226+
expect(legacyListener).not.toBeNull();
227+
228+
// The legacy subscription must drive the same redraw callback.
229+
const updates = [];
230+
window.Chart = function () { return { update: () => updates.push('redraw') }; };
231+
initBubbleChart(document.getElementById('chart_GOD'), 'God Classes', { bubbles: [] });
232+
expect(updates.length).toBe(0);
233+
legacyListener();
234+
expect(updates.length).toBeGreaterThan(0);
235+
});
236+
237+
it('keeps working when the MediaQueryList exposes neither listener API', () => {
238+
installThemeRadios('system');
239+
window.matchMedia = () => ({ matches: false });
240+
document.documentElement.removeAttribute('data-rf-theme-redraw-bound');
241+
expect(() => bindThemeChartRedraw()).not.toThrow();
242+
});
243+
200244
it('redraws live charts when a theme radio changes', () => {
201245
installThemeRadios('system');
202246
bindThemeChartRedraw(); // no-op: already bound above (and possibly earlier files)

0 commit comments

Comments
 (0)