Cache chart theme palette, add theme-init, expose chart test hooks, and add theme tests

[alvin000009238]

Motivation

• Reduce redundant CSS variable reads and ensure charts update correctly when the UI theme changes by caching the chart color palette and invalidating it on themechange.
• Improve startup reliability in environments where localStorage access may throw by moving theme initialization into a guarded script.
• Make chart instances testable and verify theme behavior with automated tests.

Description

• Added a cached palette in frontend/charts.js with getCachedChartThemePalette() and invalidate it on themechange, and updated updateRadarChart and updateBarChart to accept a palette parameter so charts use the cached theme values; also replaced internal calls to getChartThemePalette() accordingly.
• Exported a test helper __setChartInstancesForTest from frontend/charts.js to inject fake chart instances in tests and avoid UI dependency; kept existing chart destroy/reset logic intact.
• Added --color-border-extra-subtle token to frontend/styles/tokens.css for both themes to provide a separate CSS variable used by charts.
• Made getInitialTheme and applyTheme exported in frontend/theme.js (so tests and other modules can call them) and preserved existing behavior of toggling icons and dispatching the themechange event.
• Replaced inline theme initialization in public/index.html with a new, safe public/theme-init.js that wraps localStorage access in a try/catch to avoid errors in restricted/privacy modes, and updated references to built assets (main-*.css and main-*.js).
• Added tests/frontend/theme.test.js which exercises getInitialTheme, applyTheme, theme toggle interaction, and chart palette re-application by importing the charts module and using the __setChartInstancesForTest helper.

Testing

• Ran the new unit test file tests/frontend/theme.test.js with node:test, which verifies theme initialization, applyTheme behavior, toggle persistence, dispatch of themechange, and that charts receive updated palette values; the test suite passed.
• Verified the theme init script public/theme-init.js handles localStorage errors in a JSDOM-like environment via the added tests, which succeeded.

---

Codex Task

[gemini-code-assist]

Code Review

This pull request introduces theme palette caching in charts.js and a dedicated theme-init.js script to handle theme initialization more robustly. It also improves testability by exporting core theme functions and adding a comprehensive test suite. The review feedback highlights opportunities to further improve robustness by wrapping localStorage access in theme.js with error handling, mitigating side effects from top-level event listeners in charts.js, and refining the test helper to prevent state pollution.

I am having trouble creating individual review comments. Click here to see my feedback.

frontend/theme.js (3)

While exporting getInitialTheme is helpful for testing, the function should be made more robust by wrapping the localStorage.getItem call in a try/catch block. This ensures that the application doesn't crash in environments where localStorage access is restricted (e.g., privacy modes or blocked cookies), maintaining consistency with the improvements made in public/theme-init.js.

frontend/charts.js (17-22)

Attaching the themechange listener to document at the top level of the module introduces a side effect that can make unit testing fragile. In test environments like JSDOM where the global document is often swapped or cleared between tests, this listener remains bound to the document instance that existed when the module was first loaded. Consider moving this logic into an initialization function or ensuring the listener is attached to a persistent object like window (if the event bubbles).

frontend/charts.js (340-343)

The __setChartInstancesForTest helper should also provide a mechanism to reset the cachedChartPalette. Since cachedChartPalette is a module-level variable, its state persists across tests, which can lead to test pollution if different test cases expect different theme palettes or mock CSS variables differently.

export function __setChartInstancesForTest(instances = {}) {
    if (Object.hasOwn(instances, 'radar')) radarChartInstance = instances.radar;
    if (Object.hasOwn(instances, 'bar')) barChartInstance = instances.bar;
    if (Object.hasOwn(instances, 'palette')) cachedChartPalette = instances.palette;
}

[Copilot]

Pull request overview

This PR improves theme correctness and testability for the frontend by (1) caching Chart.js theme palette values and reapplying them on theme changes, (2) moving early theme initialization into a safe standalone script, and (3) adding unit tests to validate theme behavior and chart theme re-application.

Changes:

• Cache chart theme palette in frontend/charts.js, invalidate it on themechange, and add a test hook for injecting chart instances.
• Add public/theme-init.js and update public/index.html to use it instead of an inline theme init snippet.
• Add a new CSS token (--color-border-extra-subtle) and introduce automated tests for theme + chart integration.

Reviewed changes

Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
tests/frontend/theme.test.js	Adds Node node:test coverage for theme init/apply/toggle behavior and chart palette re-application.
public/theme-init.js	New guarded early theme initializer to avoid localStorage access crashes.
public/index.html	Switches to external theme init script and updates hashed Vite asset references.
frontend/theme.js	Exports getInitialTheme / applyTheme for testing and reuse.
frontend/styles/tokens.css	Adds --color-border-extra-subtle for chart grid styling.
frontend/charts.js	Adds cached palette + invalidation on themechange and exposes a test-only chart instance injector.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.