feat(web): add light/dark theme toggle#1715
Open
fezzzza wants to merge 2 commits into
Open
Conversation
Adds a theme switcher to the Web UI with light and dark mode support. Defaults to dark (preserving existing appearance). User preference is persisted in localStorage and applied before React hydrates to prevent flash of wrong theme. Changes: - index.css: Add :root:not(.dark) light theme variables, rename :root to :root.dark for dark theme - index.html: Add inline script to read localStorage and set .dark class before first paint (flash prevention) - useTheme.ts: New hook that toggles .dark class on <html> and persists to localStorage - TopNav.tsx: Add Sun/Moon toggle button next to version number
Addresses CodeRabbit review comments on coleam00#1713. - index.html: Wrap pre-mount IIFE in try-catch, fallback to dark mode - useTheme.ts: Wrap both getItem and setItem in try-catch with comments matching the established pattern in ProjectContext/Sidebar/WorkflowBuilder
📝 WalkthroughWalkthroughThe PR introduces a complete light/dark theme system for the web application. A new ChangesTheme Toggle Implementation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/web/index.html`:
- Around line 25-39: The pre-hydration script currently forces dark mode when
localStorage access fails or no saved value exists; change the IIFE so after
reading 'archon-theme' it only applies dark when the value is 'dark', removes
dark when 'light', and if the value is missing or storage throws fall back to
the system preference via window.matchMedia('(prefers-color-scheme: dark)').use
the existing document.documentElement.classList.add('dark') and
.classList.remove('dark') calls and the same 'archon-theme' key so the logic in
the anonymous function matches saved preference first, saved 'light'/'dark'
explicitly, otherwise consult matchMedia.
In `@packages/web/src/hooks/useTheme.ts`:
- Around line 7-16: getInitialTheme currently falls back to 'dark' for
first-time users instead of respecting the system preference; update
getInitialTheme to, after failing to read localStorage (or when no saved value),
check window.matchMedia('(prefers-color-scheme: dark)').matches and return
'dark' if true or 'light' if false, while keeping the existing server-side guard
(typeof window === 'undefined') and the try/catch around localStorage access for
Safari/private-mode safety; reference STORAGE_KEY and the getInitialTheme
function when making the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ddc844fd-b3e4-4743-9bc7-c446591b123f
📒 Files selected for processing (4)
packages/web/index.htmlpackages/web/src/components/layout/TopNav.tsxpackages/web/src/hooks/useTheme.tspackages/web/src/index.css
Comment on lines
+25
to
+39
| <script> | ||
| // Apply saved theme before React renders to prevent flash | ||
| (function () { | ||
| try { | ||
| var theme = localStorage.getItem('archon-theme'); | ||
| if (theme === 'light') { | ||
| document.documentElement.classList.remove('dark'); | ||
| } else { | ||
| document.documentElement.classList.add('dark'); | ||
| } | ||
| } catch (_) { | ||
| document.documentElement.classList.add('dark'); | ||
| } | ||
| })(); | ||
| </script> |
There was a problem hiding this comment.
Pre-hydration fallback should respect system theme, not always dark.
Lines 32-37 force dark when there is no saved value (or storage throws), so first paint can be wrong for users with light OS preference.
Suggested fix
(function () {
+ var isDark;
try {
var theme = localStorage.getItem('archon-theme');
- if (theme === 'light') {
- document.documentElement.classList.remove('dark');
- } else {
- document.documentElement.classList.add('dark');
- }
+ if (theme === 'light') isDark = false;
+ else if (theme === 'dark') isDark = true;
+ else isDark = window.matchMedia && window.matchMedia('(prefers-color-scheme: dark)').matches;
} catch (_) {
- document.documentElement.classList.add('dark');
+ isDark = window.matchMedia && window.matchMedia('(prefers-color-scheme: dark)').matches;
}
+ document.documentElement.classList.toggle('dark', !!isDark);
})();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/web/index.html` around lines 25 - 39, The pre-hydration script
currently forces dark mode when localStorage access fails or no saved value
exists; change the IIFE so after reading 'archon-theme' it only applies dark
when the value is 'dark', removes dark when 'light', and if the value is missing
or storage throws fall back to the system preference via
window.matchMedia('(prefers-color-scheme: dark)').use the existing
document.documentElement.classList.add('dark') and .classList.remove('dark')
calls and the same 'archon-theme' key so the logic in the anonymous function
matches saved preference first, saved 'light'/'dark' explicitly, otherwise
consult matchMedia.
Collaborator
Review SummaryVerdict: minor-fixes-needed Adds a light/dark theme toggle to the web UI with CSS variable-based theming, a Blocking issues
Suggested fixes
Minor / nice-to-have
Compliments
Reviewed via maintainer-review-pr workflow (Pi/Minimax). Aspects run: code-review, error-handling, test-coverage, comment-quality. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
UX Journey
Before
After
Architecture Diagram
Before
After
Label Snapshot
risk: lowsize: XSwebweb:layoutChange Metadata
featurewebLinked Issue
Validation Evidence (required)
All checks pass: type-check, lint, format, tests.
Security Impact (required)
Compatibility / Migration
Human Verification (required)
Side Effects / Blast Radius (required)
Rollback Plan (required)
Risks and Mitigations
Summary by CodeRabbit
Release Notes