feat(dashboard): add light/dark theme toggle#616
feat(dashboard): add light/dark theme toggle#616scotthamilton77 wants to merge 3 commits intogastownhall:mainfrom
Conversation
…tence Adds a theme toggle button to the dashboard header. Clicking switches between dark (default) and light mode; the preference is saved to localStorage and restored on page load. Changes: - dashboard.css: add [data-theme="light"] CSS variable overrides - convoy.html: add toggle button alongside Commands button in header - dashboard.js: theme init on load (reads localStorage), click handler, localStorage persistence with graceful fallback Co-Authored-By: Claude Sonnet 4.6 (1M context) <[email protected]>
github-actions
bot
added
the
status/needs-triage
There was a problem hiding this comment.
Pull request overview
Adds a light/dark theme toggle to the GC dashboard UI, persisting the user’s preference in localStorage and applying a light color palette via CSS variable overrides.
Changes:
- Add a header toggle button for switching between dark (default) and light themes.
- Add JS to apply/persist theme selection and update the toggle icon.
- Add CSS custom property overrides under
[data-theme="light"]for the light palette.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
cmd/gc/dashboard/templates/convoy.html |
Adds the theme toggle button/icon to the dashboard header. |
cmd/gc/dashboard/static/dashboard.js |
Implements theme apply/toggle logic and persistence. |
cmd/gc/dashboard/static/dashboard.css |
Adds light-theme CSS variable overrides. |
.gitignore |
Adds ignore rules for .sidekick/* and .worktrees/ (unrelated to theme toggle). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| var saved = localStorage.getItem(THEME_STORAGE_KEY); | ||
| if (saved === 'light') { | ||
| applyTheme('light'); |
There was a problem hiding this comment.
localStorage.getItem(...) is called without a try/catch. In environments where storage access is blocked (e.g., privacy mode / disabled storage), this can throw and abort the rest of dashboard.js execution (breaking SSE + UI handlers). Wrap the read in the same try/catch pattern used by loadRecentCommands() and fall back to default theme when unavailable.
| var saved = localStorage.getItem(THEME_STORAGE_KEY); | |
| if (saved === 'light') { | |
| applyTheme('light'); | |
| try { | |
| var saved = localStorage.getItem(THEME_STORAGE_KEY); | |
| if (saved === 'light') { | |
| applyTheme('light'); | |
| } | |
| } catch (_) { | |
| // localStorage unavailable, fall back to default theme |
There was a problem hiding this comment.
Fixed in fb7ec9c — the IIFE now wraps localStorage.getItem in a try/catch, matching the pattern used by loadRecentCommands. Falls back silently to default (dark) theme.
| // Apply saved theme immediately on load (before first paint if possible). | ||
| (function() { | ||
| var saved = localStorage.getItem(THEME_STORAGE_KEY); | ||
| if (saved === 'light') { | ||
| applyTheme('light'); | ||
| } | ||
| })(); |
There was a problem hiding this comment.
This init runs from /static/dashboard.js which is loaded at the end of <body> in convoy.html, so it will generally execute after the first paint. If the intent is to avoid a dark→light flash for users with a saved light preference, consider moving a minimal theme bootstrap (read storage + set data-theme) into an inline <script> in the document <head> before rendering, and keep this block for icon wiring/click handling.
There was a problem hiding this comment.
Fixed in fb7ec9c — added an inline <script> in <head> (before the stylesheet) that reads localStorage and sets data-theme before first paint. The JS block in dashboard.js is now responsible only for icon wiring and click handling, not initial theme application.
| function applyTheme(theme) { | ||
| document.documentElement.setAttribute('data-theme', theme); | ||
| var icon = document.getElementById('theme-toggle-icon'); | ||
| if (icon) { | ||
| icon.textContent = theme === 'light' ? '🌙' : '☀'; | ||
| } |
There was a problem hiding this comment.
applyTheme() updates the toggle icon, but the header (including #theme-toggle-icon) lives inside #dashboard-main which is periodically replaced via HTMX morph. After a swap, the server-rendered icon will revert to the template default (☀) even if data-theme is still light. To keep UI consistent, re-apply the current theme (or at least re-sync the icon) in the existing htmx:afterSwap handler when target.id === 'dashboard-main'.
There was a problem hiding this comment.
Fixed in fb7ec9c — added a call to applyTheme() at the top of the existing htmx:afterSwap handler that's already scoped to #dashboard-main. This re-syncs the icon (and data-theme) after each morph without touching partial-swap paths.
- Wrap localStorage.getItem in try/catch to handle privacy mode/blocked storage without aborting JS execution - Add inline <script> in <head> to apply saved theme before first paint, preventing dark->light flash for returning light-mode users - Re-sync theme icon in htmx:afterSwap handler for #dashboard-main so morph replacements don't revert the icon to the template default Co-Authored-By: Claude Sonnet 4.6 (1M context) <[email protected]>
julianknutsen
added
priority/p3
Summary
localStorageunder keygc-dashboard-themeand restored on page loadChanges
dashboard.css— adds[data-theme="light"]CSS custom property overrides (19 lines); dark palette unchanged as defaultconvoy.html— adds toggle button witharia-labelandtitleto the header flex rowdashboard.js— theme init on load (reads localStorage before first paint), delegated click handler, localStorage persistence with silent fallbackTesting
go test ./cmd/gc/dashboard/...— all 18 tests passgo build ./cmd/gc/...— clean🤖 Generated with Claude Code