Skip to content

Add dark mode toggle#3

Merged
robbfo merged 1 commit into
mainfrom
worktree-dark-mode
May 21, 2026
Merged

Add dark mode toggle#3
robbfo merged 1 commit into
mainfrom
worktree-dark-mode

Conversation

@robbfo

@robbfo robbfo commented May 21, 2026

Copy link
Copy Markdown
Owner

Summary

  • useDarkMode.js — singleton composable (same pattern as useFilters/useSidebar); applies html.dark class immediately at module load (no flash), persists preference in localStorage
  • App.vue — CSS custom properties on :root / html.dark drive all light/dark values for page background, cards, borders, text colors, and tables; no new classes needed in templates
  • AppSidebar.vue — moon/sun icon toggle button in .sidebar-bottom; collapses to icon-only with tooltip when sidebar is in 56px mode

Design tokens added:

Token Light Dark
--bg-page #f8fafc #0f172a
--bg-card #ffffff #1e293b
--border-card #e2e8f0 rgba(255,255,255,0.08)
--text-primary #0f172a #f1f5f9
--text-secondary #64748b #94a3b8
--table-row-hover #f8fafc #243349

Known scope: global layout styles are themed; view-specific scoped CSS (chart labels, inline SVG) retains hardcoded colors — those would be addressed in a follow-up.

Test plan

  • Click "Dark mode" button in sidebar — page switches to dark theme
  • Click "Light mode" button — page returns to light theme
  • Reload page in dark mode — preference persists (no flash)
  • Collapse sidebar — toggle button becomes icon-only with tooltip
  • Navigate across all 6 views — cards and tables use dark colors throughout

🤖 Generated with Claude Code

- useDarkMode.js: singleton composable that toggles `html.dark` class and
  persists choice in localStorage; no flash on reload because class is applied
  at module-load time before first render
- App.vue: CSS custom properties on :root (light) and html.dark (dark) cover
  page bg, card bg, borders, text, and table colors; useDarkMode() initialized
  in setup to guarantee early class application
- AppSidebar.vue: moon/sun toggle button in sidebar-bottom; collapses to
  icon-only (32px) when sidebar is collapsed, with tooltip

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
@robbfo

robbfo commented May 21, 2026

Copy link
Copy Markdown
Owner Author

@claude what do you think?

@claude

claude Bot commented May 21, 2026

Copy link
Copy Markdown

Claude finished @robbfo's task in 1m 28s —— View job


Review: dark mode toggle

Overall this is well-scoped and clean. The singleton composable pattern matches useSidebar/useFilters, the module-load class-apply correctly prevents flash, and CSS variables on :root / html.dark is the right primitive to build on. Below are the things I'd address before merging plus a few nits.


One bug to fix before merge

body { color: #1e293b } in client/src/App.vue:184 is hardcoded. This sits in the same <style> block where you just defined --text-primary / --text-body, and it overrides the dark token for any text that inherits from <body> (anything not explicitly using a var(--text-*)). It's the single most visible miss in the global styles — easy fix:

body {
  background: var(--bg-page);
  color: var(--text-body);  /* was: #1e293b */
}

Fix this →


Worth considering (not blockers)

  1. No prefers-color-scheme fallback. First-time visitors on a dark-themed OS get light mode until they manually toggle. One-line addition in useDarkMode.js:

    const stored = localStorage.getItem(STORAGE_KEY);
    const isDark = ref(
      stored === null
        ? window.matchMedia("(prefers-color-scheme: dark)").matches
        : stored === "true"
    );
  2. Badges and the .error block still use hardcoded pastel backgrounds (App.vue:351-399, App.vue:408-416). Pastel-on-dark is readable but visually clashes with the new dark surfaces. Same category as the scoped-view colors you've already flagged as follow-up — just noting they're in the global stylesheet, not just views, so they're worth grouping into the same follow-up PR.

  3. AppSidebar.vue is unchanged structurally in dark mode. That's fine — the sidebar was already dark — but it means the only visual cue that dark mode is "on" outside the main canvas is the toggle icon swap. Consider whether the active nav link's bright #2563eb (line 487) should soften slightly in dark mode for less contrast burn. Stylistic call.


Things I liked

  • classList.toggle("dark", val) in the watch handles both add and remove without branching — nicer than the alternative.
  • useDarkMode() returns the singleton refs without re-initializing — same pattern as the other composables, future calls from other components will Just Work.
  • PR description explicitly scopes the view-level hardcoded colors as a follow-up. That's the right move — bundling them would have doubled the diff and made review harder.
  • Diff is mostly additive CSS variables — low risk of regressing existing behavior.

Test plan note

You haven't checked any boxes in the PR description's test plan yet. Since the visible-correctness story depends on actually toggling and walking the views (especially with the hardcoded body color above), I'd run through that checklist locally before merging — Vue/CSS unit tests won't catch the contrast issues.


  • Read useDarkMode.js
  • Read App.vue changes
  • Read AppSidebar.vue changes
  • Sanity-check against scoped CSS in views
  • Post review
    • Branch: worktree-dark-mode

@robbfo robbfo merged commit 170ea24 into main May 21, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant