fix: theme toggle now applies theme on change (#2361) - #2434
Conversation
utksh1
left a comment
utksh1
left a comment
There was a problem hiding this comment.
Request changes: replacing the existing theme control removes the system-preference indicator and the reset-to-system behavior. Preserve those existing settings workflows while fixing the theme application, and add a regression test that changing the select updates the document theme and that system-controlled mode still works.
utksh1
added
area:frontend
Fixed: ENGINE_RESET now clears the saved theme override and re-derives from OS preference (prefers-color-scheme) instead of leaving the previously-applied theme stale — this restores reset-to-system behavior. Added regression tests covering: theme applies immediately on select (the original #2361 fix), and ENGINE_RESET correctly restores system-controlled theme. One thing I want to confirm — I didn't find a literal "system-preference indicator" UI element in Settings.tsx before this PR (no badge/label showing "currently following system"). If you meant something specific I might be missing, let me know and I'll add it; otherwise I've interpreted this as ensuring Reset genuinely re-syncs to OS preference rather than just cosmetically defaulting to "dark. |
2 participants
Description
Fixes the non-functional theme toggle in the Settings panel. The "Visual_Spectrum" dropdown was only updating local component state (
config.theme) and never calledsetTheme()fromThemeContext— so selecting Light/Dark had zero effect until (and even after) clicking "Commit_Engine_Changes." The dropdown'sonChangenow callssetTheme()immediately, so the theme context and<html>class update as soon as the user picks a value.Also added a
.theme-lightCSS override block inindex.css(previously the class was toggled on<html>but no light-theme styles existed for the core design tokens like--bg-primary,--text-primary, etc., so even a working toggle had no visible effect on token-driven elements).Known limitation (out of scope for this PR): Several pages, including Settings itself, use hardcoded Tailwind color classes (e.g.
bg-charcoal-dark,text-silver) defined as static hex values intailwind.config.js, rather than the CSS variable tokens. These don't respond to the theme toggle at all. Fully wiring light-mode support across the app would require rewriting these Tailwind classes to reference CSS variables across many files — a much larger change than this bug fix. Flagging this for a separate issue/PR.Related Issues
Fixes #2361
Type of Change
How Has This Been Tested?
Ran
npm run dev, went to Settings → Visual_Spectrum, changed the dropdown from Dark to Light and confirmedsetTheme()fires immediately (verified via the sidebar toggle switching icon/state, andlocalStorage['secuscan-theme']updating). Confirmed the.theme-lightclass is applied to<html>and that pages using CSS-variable-driven styles respond correctly. Tested in Chrome.Checklist