Skip to content

feat(theme): dark mode with user preference toggle#124

Merged
steilerDev merged 1 commit into
betafrom
feat/119-dark-mode
Feb 18, 2026
Merged

feat(theme): dark mode with user preference toggle#124
steilerDev merged 1 commit into
betafrom
feat/119-dark-mode

Conversation

@steilerDev
Copy link
Copy Markdown
Owner

Summary

  • Complete [data-theme="dark"] overrides in tokens.css for all semantic token categories: backgrounds, text, borders, primary/danger/success actions, sidebar, focus rings, overlays, status/role/user-status badges, and shadows
  • New ThemeContext (ThemeProvider + useTheme) that reads/writes localStorage, resolves 'system' via window.matchMedia, and reactively updates on OS preference changes — sets document.documentElement.dataset.theme on every change
  • New ThemeToggle component in the sidebar: cycles Light → Dark → System with inline SVG sun/moon/monitor icons (no icon library dependency), keyboard accessible, uses token-only CSS
  • ThemeProvider wraps AuthProvider in App.tsx (inside BrowserRouter, outside AuthProvider) so dark mode applies globally including auth pages
  • Test infrastructure: window.matchMedia polyfill added to setupTests.ts; AppShell overlay gets data-testid="sidebar-overlay" so tests are not confused by SVG aria-hidden="true" icons; Sidebar and AppShell tests updated to mock ThemeContext

Files Changed

  • client/src/styles/tokens.css — Layer 3 dark mode overrides (was a commented stub)
  • client/src/contexts/ThemeContext.tsx — new file
  • client/src/components/ThemeToggle/ThemeToggle.tsx — new file
  • client/src/components/ThemeToggle/ThemeToggle.module.css — new file
  • client/src/App.tsx — wrap with ThemeProvider
  • client/src/components/Sidebar/Sidebar.tsx — add ThemeToggle
  • client/src/components/Sidebar/Sidebar.module.css — add themeSection wrapper style
  • client/src/components/AppShell/AppShell.tsx — add data-testid to overlay
  • client/src/test/setupTests.ts — add window.matchMedia polyfill
  • client/src/components/Sidebar/Sidebar.test.tsx — mock ThemeContext, update button count
  • client/src/components/AppShell/AppShell.test.tsx — mock ThemeContext, update overlay selector

Quality Gates

  • npm run lint — 0 errors
  • npm run format:check — all clean
  • npm run typecheck — all 3 workspaces clean
  • npm test — 1072/1072 tests pass (53 suites)
  • npm audit — 17 pre-existing vulns (semantic-release/eslint chain, not fixable without breaking changes), 0 new

Test Plan

  • Toggle cycles: Light → Dark → System → Light
  • Dark mode visuals: dark backgrounds, light text, readable badges across all pages
  • System mode: respects OS dark/light preference; updates reactively on OS change
  • Preference persists across page reload (stored in localStorage)
  • Login and setup pages also apply the theme (ThemeProvider is outside AuthProvider)
  • Token verification: grep -rn '#[0-9a-fA-F]' client/src --include="*.module.css" returns zero results

Fixes #119

🤖 Generated with Claude Code

Implement ThemeContext with Light/Dark/System preference,
ThemeToggle component in sidebar, and dark mode token overrides.
Preference persisted to localStorage, system preference respected
via window.matchMedia and reactive OS-preference change listener.

- tokens.css: complete [data-theme="dark"] overrides for all
  semantic tokens (backgrounds, text, borders, primary, danger,
  success, sidebar, focus rings, overlays, badges, shadows)
- ThemeContext.tsx: ThemeProvider + useTheme hook; reads/writes
  localStorage; resolves 'system' via matchMedia; sets
  document.documentElement.dataset.theme on every change
- ThemeToggle component: cycles Light → Dark → System with
  inline SVG sun/moon/monitor icons (no icon library dependency)
- Sidebar: ThemeToggle placed between nav separators (before logout)
- App.tsx: ThemeProvider wraps AuthProvider inside BrowserRouter
- test/setupTests.ts: polyfill window.matchMedia for jsdom
- AppShell overlay: add data-testid="sidebar-overlay" so tests
  can distinguish it from SVG aria-hidden="true" icons
- Updated Sidebar and AppShell tests to mock ThemeContext and
  use the new data-testid selector respectively

Fixes #119

Co-Authored-By: Claude frontend-developer (Sonnet 4.6) <noreply@anthropic.com>
Copy link
Copy Markdown
Owner Author

@steilerDev steilerDev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[security-engineer] Theme implementation security review completed.

Summary

PR #124 implements dark mode with localStorage persistence and ThemeContext. Security analysis: NO CRITICAL OR HIGH-SEVERITY FINDINGS. All major security concerns (localStorage validation, XSS prevention, DOM manipulation, memory leaks) are handled correctly.

Detailed Findings

1. localStorage Usage & Input Validation (OWASP A02) - SECURE

Status: PASS

The implementation validates localStorage data rigorously before use:

ThemeContext.tsx, lines 29-39 — readStoredPreference():

  • Reads localStorage value with try-catch wrapper (handles unavailability in private browsing)
  • Validates stored value against whitelist:
  • Returns safe default ('system') on any failure
  • Type annotations ensure TypeScript catches out-of-range values

Impact: Even if localStorage is corrupted by a browser extension or attacker script, the app safely resets to 'system' theme. No injection vector.

2. XSS Prevention - SECURE

Status: PASS

ThemeToggle.tsx — Zero XSS vectors identified:

  • No use of dangerouslySetInnerHTML, innerHTML, or eval()

  • Inline SVG icons are hardcoded React elements (not string concatenation)

  • THEME_LABELS is a static constant; theme values are enums

  • aria-label and title attributes use template literals with known enum values (lines 99-100):

    These are safe because nextLabel and THEME_LABELS[theme] are statically known strings ('Light', 'Dark', 'System').

Assessment: No reflected, stored, or DOM-based XSS possible via theme setting.

3. DOM Manipulation Safety (document.documentElement) - SECURE

Status: PASS

ThemeContext.tsx, lines 52-54:

  • Sets data-theme attribute to a type-safe enum: 'light' | 'dark'
  • No string interpolation or unsanitized input
  • CSS is scoped to [data-theme='dark'], not HTML content
  • Attribute assignment via dataset API (not setAttribute with string)
  • Safe: CSS var overrides in tokens.css only change colors and spacing, not executable code

Assessment: No DOM-clobbering, script injection, or attribute-based attack possible.

4. matchMedia Listener Cleanup - SECURE

Status: PASS

ThemeContext.tsx, lines 57-68:

  • Event listener is properly cleaned up in the return function
  • Dependency array [theme] ensures new listener when theme preference changes
  • Listener is removed when component unmounts or theme changes
  • No memory leaks: each listener is 1:1 with effect execution

Assessment: Event listeners properly managed; no resource leaks.

5. Sensitive Data & localStorage - SECURE

Status: PASS

  • Only non-sensitive data stored: theme preference ('light', 'dark', 'system')
  • No auth tokens, session IDs, PII, or credentials stored
  • No personal data exposure
  • Safe to persist across sessions

Assessment: No sensitive data exposure risk.

6. CSS Token Security - SECURE

Status: PASS

tokens.css, lines 281-390:

  • Dark mode overrides contain only CSS property values (hex colors, rgb/rgba, spacing units)
  • No animation delays, calc() expressions, or font-family values that could encode data
  • All values are static, not user-controlled
  • CSS scope prevents bleed to other elements

Assessment: Dark mode tokens are inert stylistic changes only.

7. localStorage Failure Handling - SECURE

Status: PASS

ThemeContext.tsx, lines 70-78:

  • localStorage.setItem() wrapped in try-catch
  • Graceful degradation: theme still updates in memory even if storage fails
  • Silently ignores errors (acceptable for a non-critical feature like theme persistence)

Assessment: Robust error handling prevents app breakage from storage unavailability.

Testing Notes

The PR includes comprehensive test mocks:

  • AppShell.test.tsx, lines 33-40: ThemeContext mocked for sidebar tests
  • Sidebar.test.tsx, lines 34-45: useTheme hook mocked with predictable values
  • setupTests.ts, lines 14-30: matchMedia polyfill for jsdom (window.matchMedia not natively supported)
  • AppShell.test.tsx, lines 161+: Overlay selector updated to use data-testid for reliability

All changes maintain test isolation and prevent localStorage side effects during test runs.

Recommendations (Low Priority)

  1. CSP Header (Informational): If Content Security Policy is implemented in future PRs, ensure no 'unsafe-inline' is required for the theme system (currently none). The implementation uses only runtime state and CSS vars.

  2. Security Audit Page Update: Document dark mode as a non-sensitive UX preference that persists in localStorage and uses client-side detection of OS preferences via matchMedia API.

Conclusion

PR #124 is APPROVED from a security standpoint. The implementation follows OWASP guidelines for client-side storage, XSS prevention, and DOM manipulation. No security blockers identified.

Copy link
Copy Markdown
Owner Author

@steilerDev steilerDev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[security-engineer] Theme implementation security review completed.

Summary

PR #124 implements dark mode with localStorage persistence and ThemeContext. Security analysis: NO CRITICAL OR HIGH-SEVERITY FINDINGS. All major security concerns (localStorage validation, XSS prevention, DOM manipulation, memory leaks) are handled correctly.

Detailed Findings

1. localStorage Usage & Input Validation (OWASP A02) - SECURE

Status: PASS

The implementation validates localStorage data rigorously before use:

ThemeContext.tsx, lines 29-39 — readStoredPreference():

  • Reads localStorage value with try-catch wrapper (handles unavailability in private browsing)
  • Validates stored value against whitelist: if (stored === 'light' || stored === 'dark' || stored === 'system')
  • Returns safe default ('system') on any failure
  • Type annotations ensure TypeScript catches out-of-range values

Impact: Even if localStorage is corrupted by a browser extension or attacker script, the app safely resets to 'system' theme. No injection vector.

2. XSS Prevention - SECURE

Status: PASS

ThemeToggle.tsx — Zero XSS vectors identified:

  • No use of dangerouslySetInnerHTML, innerHTML, or eval()
  • Inline SVG icons are hardcoded React elements (not string concatenation)
  • THEME_LABELS is a static constant; theme values are enums
  • aria-label and title attributes use template literals with known enum values
  • These are safe because label values are statically known strings ('Light', 'Dark', 'System')

Assessment: No reflected, stored, or DOM-based XSS possible via theme setting.

3. DOM Manipulation Safety (document.documentElement) - SECURE

Status: PASS

ThemeContext.tsx, lines 52-54:

  • Sets data-theme attribute to a type-safe enum: 'light' | 'dark'
  • No string interpolation or unsanitized input
  • CSS is scoped to [data-theme='dark'], not HTML content
  • Attribute assignment via dataset API (not setAttribute with string)
  • Safe: CSS var overrides in tokens.css only change colors and spacing, not executable code

Assessment: No DOM-clobbering, script injection, or attribute-based attack possible.

4. matchMedia Listener Cleanup - SECURE

Status: PASS

ThemeContext.tsx, lines 57-68:

  • Event listener is properly cleaned up in the return function
  • Dependency array [theme] ensures new listener when theme preference changes
  • Listener is removed when component unmounts or theme changes
  • No memory leaks: each listener is 1:1 with effect execution

Assessment: Event listeners properly managed; no resource leaks.

5. Sensitive Data & localStorage - SECURE

Status: PASS

  • Only non-sensitive data stored: theme preference ('light', 'dark', 'system')
  • No auth tokens, session IDs, PII, or credentials stored
  • No personal data exposure
  • Safe to persist across sessions

Assessment: No sensitive data exposure risk.

6. CSS Token Security - SECURE

Status: PASS

tokens.css, lines 281-390:

  • Dark mode overrides contain only CSS property values (hex colors, rgb/rgba, spacing units)
  • No animation delays, calc() expressions, or font-family values that could encode data
  • All values are static, not user-controlled
  • CSS scope prevents bleed to other elements

Assessment: Dark mode tokens are inert stylistic changes only.

7. localStorage Failure Handling - SECURE

Status: PASS

ThemeContext.tsx, lines 70-78:

  • localStorage.setItem() wrapped in try-catch
  • Graceful degradation: theme still updates in memory even if storage fails
  • Silently ignores errors (acceptable for a non-critical feature like theme persistence)

Assessment: Robust error handling prevents app breakage from storage unavailability.

Testing Coverage

The PR includes comprehensive test mocks:

  • AppShell.test.tsx: ThemeContext mocked for sidebar tests
  • Sidebar.test.tsx: useTheme hook mocked with predictable values
  • setupTests.ts: matchMedia polyfill for jsdom (window.matchMedia not natively supported)
  • Overlay selector updated to use data-testid for reliability

All changes maintain test isolation and prevent localStorage side effects during test runs.

Recommendations

  1. CSP Header (Informational): If Content Security Policy is implemented in future, ensure no 'unsafe-inline' is required for theme system. The implementation uses only runtime state and CSS vars — compliant with strict CSP.

  2. Security Audit Page: Document dark mode as a non-sensitive UX preference persisted in localStorage using client-side OS preference detection via matchMedia API.

Approval

PR #124 is APPROVED from a security standpoint. The implementation follows OWASP guidelines for client-side storage, XSS prevention, and DOM manipulation. No security blockers identified.

@steilerDev
Copy link
Copy Markdown
Owner Author

[security-engineer] Theme implementation security review completed.

Summary

PR #124 implements dark mode with localStorage persistence and ThemeContext. Security analysis: NO CRITICAL OR HIGH-SEVERITY FINDINGS. All major security concerns (localStorage validation, XSS prevention, DOM manipulation, memory leaks) are handled correctly.

Detailed Findings

1. localStorage Usage & Input Validation (OWASP A02) - SECURE

Status: PASS

The implementation validates localStorage data rigorously before use:

ThemeContext.tsx, lines 29-39 — readStoredPreference():

  • Reads localStorage value with try-catch wrapper (handles unavailability in private browsing)
  • Validates stored value against whitelist: if (stored === 'light' || stored === 'dark' || stored === 'system')
  • Returns safe default ('system') on any failure
  • Type annotations ensure TypeScript catches out-of-range values

Impact: Even if localStorage is corrupted by a browser extension or attacker script, the app safely resets to 'system' theme. No injection vector.

2. XSS Prevention - SECURE

Status: PASS

ThemeToggle.tsx — Zero XSS vectors identified:

  • No use of dangerouslySetInnerHTML, innerHTML, or eval()
  • Inline SVG icons are hardcoded React elements (not string concatenation)
  • THEME_LABELS is a static constant; theme values are enums
  • aria-label and title attributes use template literals with known enum values
  • These are safe because label values are statically known strings ('Light', 'Dark', 'System')

Assessment: No reflected, stored, or DOM-based XSS possible via theme setting.

3. DOM Manipulation Safety - SECURE

Status: PASS

ThemeContext.tsx, lines 52-54:

  • Sets data-theme attribute to a type-safe enum: 'light' | 'dark'
  • No string interpolation or unsanitized input
  • CSS is scoped to [data-theme='dark'], not HTML content
  • Attribute assignment via dataset API (not setAttribute with string)

Assessment: No DOM-clobbering, script injection, or attribute-based attack possible.

4. matchMedia Listener Cleanup - SECURE

Status: PASS

ThemeContext.tsx, lines 57-68:

  • Event listener is properly cleaned up in the return function
  • Dependency array [theme] ensures cleanup when preference changes
  • No memory leaks or orphaned listeners

Assessment: Event listeners properly managed.

5. Sensitive Data & localStorage - SECURE

Status: PASS

  • Only non-sensitive data stored: theme preference ('light', 'dark', 'system')
  • No auth tokens, session IDs, PII, or credentials stored
  • Safe to persist across sessions

Assessment: No sensitive data exposure risk.

6. CSS Token Security - SECURE

Status: PASS

tokens.css, lines 281-390:

  • Dark mode overrides contain only CSS property values (hex colors, rgb/rgba)
  • All values are static, not user-controlled
  • CSS scope prevents bleed to other elements

Assessment: Dark mode tokens are inert stylistic changes only.

7. localStorage Failure Handling - SECURE

Status: PASS

ThemeContext.tsx, lines 70-78:

  • localStorage.setItem() wrapped in try-catch
  • Graceful degradation: theme still updates in memory even if storage fails
  • Robust error handling prevents app breakage

Assessment: Proper error recovery.

Recommendations

  1. CSP Header (Informational): If Content Security Policy is implemented in future, the implementation is compliant with strict CSP (uses only runtime state and CSS vars).

  2. Security Audit Page: Document dark mode as a non-sensitive UX preference persisted in localStorage using matchMedia API for OS preference detection.

Approval

PR #124 is APPROVED from a security standpoint. No security blockers identified. Implementation follows OWASP guidelines for client-side storage, XSS prevention, and DOM manipulation.

Copy link
Copy Markdown
Owner Author

@steilerDev steilerDev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[product-architect]

Architecture Review — PR #124: Dark Mode with User Preference Toggle

Overall this is a clean, well-structured implementation. The core patterns are correct and match the project's established conventions. I have a few observations — one substantive concern and several minor notes — but nothing that blocks merge.


ThemeContext Pattern

Passes. The implementation follows the AuthContext pattern precisely:

  • createContext<ThemeContextValue | undefined>(undefined) matches the AuthContext pattern exactly.
  • The useTheme() hook throws 'useTheme must be used within a ThemeProvider' on missing provider, identical in structure to useAuth().
  • Named exports for both the provider and the hook are consistent with how AuthContext exports are structured.
  • Interface ThemeContextValue is explicit and clearly typed; ThemeProviderProps is defined as a separate interface rather than inlined.

localStorage Read/Write

Passes. readStoredPreference() validates the stored value against the union literal set before accepting it — prevents stale or injected invalid values from being consumed. The try/catch around both read and write is correct (private browsing throws on localStorage access). Default fallback to 'system' is the right choice.


System Preference Listener

Passes with a note. The matchMedia listener lifecycle is correct:

  • Effect is conditional on theme === 'system', so it only attaches when needed.
  • Cleanup runs removeEventListener via the return function.
  • The effect re-runs when theme changes, so transitioning away from 'system' cleanly detaches the listener.

One minor observation: resolvedTheme state is initialized by calling readStoredPreference() a second time in the useState initializer, rather than reusing the value already read for theme. This means localStorage is read twice on mount. It does not cause a bug, but a small refactor — reading the preference once and passing it to both initializers — would eliminate the double read. This is a non-blocking observation.


Dark Token Overrides in tokens.css

Passes. The dark block is comprehensive and correct in structure:

  • All semantic token categories from Layer 2 that require dark overrides are covered: backgrounds, text, borders, primary, danger, success, sidebar, focus rings, overlays, status badges, role badges, user-status badges, and shadows.
  • The selector is [data-theme='dark'] which matches the attribute set by document.documentElement.dataset.theme.
  • Palette tokens (Layer 1) are correctly left untouched.
  • The comment at the top of Layer 3 accurately describes the scoping strategy.

One substantive concern: the dark overrides use raw hex literals (e.g., #1a1a2e, #f1f5f9, #334155) rather than referencing Layer 1 palette tokens via var(). The light-mode Layer 2 tokens consistently reference palette tokens (e.g., var(--color-gray-900), var(--color-blue-500)). The dark overrides break this discipline — many values are duplicates of palette tokens that already exist (e.g., #f1f5f9 is Tailwind slate-100, but not in our palette; #334155 = Tailwind slate-700, also absent).

Two options for a follow-up: (a) extend the Layer 1 palette to include the dark-appropriate slate scale and reference those, or (b) add a comment in the dark block explicitly noting that the values are intentional raw values because a dark-tuned palette is not yet defined. For now the behavior is correct and the UX designer produced these values, so this is an observation for the ux-designer to address when the Style Guide story ships rather than a blocker for this PR.


ThemeToggle Component

Passes.

  • Cycles correctly: NEXT_THEME maps light → dark → system → light, which is the Light → Dark → System order specified in the acceptance criteria.
  • aria-label describes the next action ('Switch to Dark mode'), which is correct for a toggle button — it tells the user what will happen, not the current state. The title attribute additionally surfaces the current state.
  • SVG icons carry aria-hidden="true" and the visible label text provides the accessible name through the button's aria-label.
  • All CSS in ThemeToggle.module.css uses var() tokens exclusively — zero hardcoded colors. The min-height: 44px touch-target rule at the max-width: 1024px breakpoint matches the project's mobile accessibility requirement.
  • Inline SVG approach with currentColor is appropriate — no icon library dependency added.

Minor observation: THEME_CYCLE is exported from ThemeToggle.tsx with the comment 'for use in tests', but no test file currently imports it. The export comment suggests it was pre-emptively exported for the QA agent's ThemeToggle unit tests. This is fine, but if those tests don't materialize, this becomes dead export surface. Non-blocking.


Integration in App.tsx

Passes. ThemeProvider wraps AuthProvider, placing it outside the auth boundary so dark mode applies to login and setup pages as well. The nesting order BrowserRouter > ThemeProvider > AuthProvider > Routes is correct — theme is infrastructure-level, not auth-gated.

ThemeToggle Placement in Sidebar

Passes. The component is placed between the nav separator and the logout button, which is the expected location from the UX spec. The themeSection wrapper div in Sidebar.module.css uses var(--spacing-1) 0 padding — token-only, no hardcoded values.


Test Infrastructure

Passes.

  • window.matchMedia polyfill in setupTests.ts is the correct location for a global polyfill — it runs before every test suite.
  • The polyfill returns matches: false (light mode default), which is the safe, predictable default for all tests.
  • Both AppShell.test.tsx and Sidebar.test.tsx mock ThemeContext using jest.unstable_mockModule, consistent with how AuthContext is mocked throughout the test suite.

The change from [aria-hidden="true"] to [data-testid="sidebar-overlay"] as the overlay selector in AppShell.test.tsx is a good fix — the SVG icons on ThemeToggle also carry aria-hidden="true", which would have caused the prior selector to become ambiguous. Adding data-testid="sidebar-overlay" to the production AppShell.tsx overlay div is the right resolution.


No ThemeContext Unit Test File

One gap: AuthContext.tsx has a co-located AuthContext.test.tsx that tests the provider lifecycle (loading state, localStorage effects, event listener attach/detach, error on missing provider). ThemeContext.tsx has no corresponding ThemeContext.test.tsx. The QA agent should cover: localStorage read on mount, setTheme writing to localStorage and updating resolvedTheme, system preference listener attach/detach cycle, useTheme throwing outside provider, and getSystemTheme SSR guard (typeof window === 'undefined'). This is a QA agent deliverable, not a reason to block this PR — flagging for the QA cycle.


Summary

The architecture is sound. The context pattern, persistence strategy, listener lifecycle, token coverage, and test infrastructure are all correctly implemented and consistent with the project's established conventions. The two items worth tracking as follow-up tasks are:

  1. Token discipline in dark overrides (raw hex vs. palette vars) — ux-designer follow-up in the Style Guide story.
  2. ThemeContext unit test — qa-integration-tester deliverable for this epic's QA cycle.

No blocking issues.

Copy link
Copy Markdown
Owner Author

@steilerDev steilerDev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[product-architect]

Architecture Review — PR #124: Dark Mode with User Preference Toggle

Overall this is a clean, well-structured implementation. The core patterns are correct and match the project's established conventions. I have a few observations — one substantive concern and several minor notes — but nothing that blocks merge.


ThemeContext Pattern

Passes. The implementation follows the AuthContext pattern precisely:

  • createContext<ThemeContextValue | undefined>(undefined) matches the AuthContext pattern exactly.
  • The useTheme() hook throws 'useTheme must be used within a ThemeProvider' on missing provider, identical in structure to useAuth().
  • Named exports for both the provider and the hook are consistent with how AuthContext exports are structured.
  • Interface ThemeContextValue is explicit and clearly typed; ThemeProviderProps is defined as a separate interface rather than inlined.

localStorage Read/Write

Passes. readStoredPreference() validates the stored value against the union literal set before accepting it — prevents stale or injected invalid values from being consumed. The try/catch around both read and write is correct (private browsing throws on localStorage access). Default fallback to 'system' is the right choice.


System Preference Listener

Passes with a note. The matchMedia listener lifecycle is correct:

  • Effect is conditional on theme === 'system', so it only attaches when needed.
  • Cleanup runs removeEventListener via the return function.
  • The effect re-runs when theme changes, so transitioning away from 'system' cleanly detaches the listener.

One minor observation: resolvedTheme state is initialized by calling readStoredPreference() a second time in the useState initializer, rather than reusing the value already read for theme. This means localStorage is read twice on mount. It does not cause a bug, but a small refactor — reading the preference once and passing it to both initializers — would eliminate the double read. Non-blocking.


Dark Token Overrides in tokens.css

Passes. The dark block is comprehensive and correct in structure:

  • All semantic token categories from Layer 2 that require dark overrides are covered: backgrounds, text, borders, primary, danger, success, sidebar, focus rings, overlays, status badges, role badges, user-status badges, and shadows.
  • The selector [data-theme='dark'] matches the attribute set by document.documentElement.dataset.theme.
  • Palette tokens (Layer 1) are correctly left untouched.

One substantive observation: the dark overrides use raw hex literals (e.g., #1a1a2e, #f1f5f9, #334155) rather than referencing Layer 1 palette tokens via var(). The light-mode Layer 2 tokens consistently reference palette tokens (e.g., var(--color-gray-900), var(--color-blue-500)). The dark overrides break this discipline — a dark-tuned color scale (e.g., Tailwind slate) is not yet in the palette. This is a ux-designer follow-up for the Style Guide story (either extend Layer 1 to include a dark palette scale, or add a comment in the dark block explicitly noting the values are intentional until then). Behavior is correct; not a blocker for this PR.


ThemeToggle Component

Passes.

  • Cycles correctly: NEXT_THEME maps light -> dark -> system -> light, matching the acceptance criteria.
  • aria-label describes the next action ('Switch to Dark mode'), which is correct for a toggle button. The title attribute additionally surfaces the current state for pointer users.
  • SVG icons carry aria-hidden='true' and the visible label provides the accessible name via aria-label.
  • All CSS in ThemeToggle.module.css uses var() tokens exclusively — zero hardcoded colors. The min-height: 44px touch-target rule at max-width: 1024px matches the project's mobile accessibility requirement.
  • Inline SVG with currentColor is appropriate — no icon library dependency added.

Minor observation: THEME_CYCLE is exported from ThemeToggle.tsx with the comment 'for use in tests', but no test file currently imports it. If the QA agent's ThemeToggle unit tests do not import it, this becomes dead export surface. Non-blocking.


Integration in App.tsx

Passes. ThemeProvider wraps AuthProvider, placing it outside the auth boundary so dark mode applies to login and setup pages. The nesting order BrowserRouter > ThemeProvider > AuthProvider > Routes is correct — theme is infrastructure-level, not auth-gated.

ThemeToggle Placement in Sidebar

Passes. Placed between the nav separator and logout button. The themeSection wrapper in Sidebar.module.css uses var(--spacing-1) 0 — token-only, no hardcoded values.


Test Infrastructure

Passes.

  • window.matchMedia polyfill in setupTests.ts is the correct location. Returns matches: false (light mode default), which is the safe default for all tests.
  • Both AppShell.test.tsx and Sidebar.test.tsx mock ThemeContext using jest.unstable_mockModule, consistent with how AuthContext is mocked throughout the suite.

The change from [aria-hidden='true'] to [data-testid='sidebar-overlay'] as the overlay selector in AppShell.test.tsx is a good fix — the ThemeToggle SVG icons also carry aria-hidden='true', which would have made the prior selector ambiguous. Adding data-testid='sidebar-overlay' to the production overlay div is the correct resolution.


Missing ThemeContext Unit Test File

One gap worth flagging: AuthContext.tsx has a co-located AuthContext.test.tsx that covers provider lifecycle, state transitions, and the error-on-missing-provider path. ThemeContext.tsx has no corresponding ThemeContext.test.tsx. The QA agent should cover: localStorage read on mount, setTheme writing to localStorage and updating resolvedTheme, system preference listener attach/detach, useTheme throwing outside provider, and the SSR guard (typeof window === 'undefined' in getSystemTheme). This is a QA agent deliverable, not a reason to block this PR — flagging for the QA cycle.


Summary

The architecture is sound. The context pattern, persistence strategy, listener lifecycle, token coverage, and test infrastructure are all correctly implemented and consistent with the project's established conventions.

Follow-up items (non-blocking, tracked for post-merge QA and refinement):

  1. Token discipline in dark overrides (raw hex vs. palette vars) — ux-designer follow-up in the Style Guide story.
  2. ThemeContext unit test file — qa-integration-tester deliverable.
  3. Double localStorage read on mount — minor refactor opportunity.

No blocking issues from an architecture standpoint.

@steilerDev steilerDev merged commit 9b3b62a into beta Feb 18, 2026
4 checks passed
@steilerDev steilerDev deleted the feat/119-dark-mode branch February 18, 2026 23:00
@github-actions
Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 1.8.0-beta.19 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@github-actions
Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 1.8.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants