feat: implement dark/light mode toggle (issue #170) #199
Conversation
- Add ThemeContext with localStorage persistence and system preference detection - Create ThemeToggle component with sun/moon icons - Add ClientLayout wrapper for client-side components - Update Header with dark mode styles and toggle button placement - Configure Tailwind with class-based dark mode strategy - Add dark mode styles for body, scrollbar, and navigation - Ensure smooth transitions between themes
WalkthroughThis PR introduces a comprehensive dark mode theming system for the client application. A new ThemeContext provider manages theme state with localStorage persistence and system preference detection. New components (ThemeToggle, ClientLayout) are added, and existing components are updated with dark mode styling and Tailwind class-based dark mode configuration. Changes
Sequence DiagramsequenceDiagram
participant User
participant Browser
participant ThemeProvider
participant useTheme
participant DOM as Document.documentElement
participant Component
rect rgb(200, 220, 255)
Note over ThemeProvider,DOM: Theme Initialization
Browser->>ThemeProvider: Mount ThemeProvider
ThemeProvider->>ThemeProvider: Check localStorage
alt localStorage has saved theme
ThemeProvider->>ThemeProvider: Use saved theme
else localStorage empty
ThemeProvider->>Browser: Query system preference
ThemeProvider->>ThemeProvider: Use system or default
end
ThemeProvider->>DOM: Apply "dark" class if needed
ThemeProvider->>Browser: Store in localStorage
end
rect rgb(220, 255, 220)
Note over User,Component: Theme Toggle Action
User->>Component: Click ThemeToggle button
Component->>useTheme: Call toggleTheme()
useTheme->>ThemeProvider: Update theme state
ThemeProvider->>ThemeProvider: Flip theme value
ThemeProvider->>DOM: Toggle "dark" class
ThemeProvider->>Browser: Persist to localStorage
ThemeProvider->>Component: Trigger re-render via Context
Component->>Component: Update icon (Sun ↔ Moon)
Component->>Browser: Reflect new theme
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Areas requiring extra attention:
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
client/app/components/Header/Header.tsx (1)
56-70: Avoid<button>inside<Link>(nested interactive elements).
Linkrenders an anchor; placing abuttoninside is invalid HTML and hurts a11y. Prefer styling theLinkitself (or usemotion(Link)/motion.a) and drop the inner button.- <Link key={item.name} href={item.href} className="relative"> - <motion.button className={`...`} ...> - <item.icon ... /> - <span>{item.name}</span> - </motion.button> - </Link> + <motion.div key={item.name} className="relative" whileHover={{ scale: 1.05 }} whileTap={{ scale: 0.95 }}> + <Link href={item.href} className={`inline-flex items-center px-3 py-2 ...`}> + <item.icon className="h-5 w-5 mr-2" aria-hidden="true" /> + <span>{item.name}</span> + </Link> + </motion.div>
🧹 Nitpick comments (4)
client/tailwind.config.ts (1)
3-22: Consider daisyUI theme compatibility with dark mode.
darkMode: "class"is aligned with the implementation, butdaisyui.themes: ["light"]may keep daisyUI components visually “light” in dark mode. If daisyUI components are used, consider adding a dark theme (or custom theme mapping) to avoid mixed styling.client/app/components/ClientLayout.tsx (1)
1-17: Clean layout wrapper; composition is straightforward.
Optional: if you anticipate routes that shouldn’t showChatBot, consider a prop/slot (or route-group layout) rather than hard-wiring it here.client/app/components/ThemeToggle.tsx (1)
6-34: Add pressed-state + focus styles for a11y.
Consider addingaria-pressed={theme === "dark"}and afocus-visible:ringtreatment so keyboard users can see focus on the toggle.client/app/globals.css (1)
21-24: Scrollbar at1pxmay be an accessibility/UX regression.
If this isn’t strictly intentional, consider a larger width (or media-query-based sizing) so it remains usable with a mouse/trackpad.Also applies to: 31-39
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
client/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
client/app/components/ClientLayout.tsx(1 hunks)client/app/components/Header/Header.tsx(8 hunks)client/app/components/Pages/LoginPage.tsx(1 hunks)client/app/components/ThemeToggle.tsx(1 hunks)client/app/context/ThemeContext.tsx(1 hunks)client/app/globals.css(1 hunks)client/app/layout.tsx(2 hunks)client/tailwind.config.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
client/app/layout.tsx (3)
client/app/context/ThemeContext.tsx (1)
ThemeProvider(13-45)client/app/helpers/client.ts (2)
config(7-17)queryClient(19-19)client/app/components/ClientLayout.tsx (1)
ClientLayout(6-18)
client/app/components/ThemeToggle.tsx (1)
client/app/context/ThemeContext.tsx (1)
useTheme(47-53)
client/app/components/Header/Header.tsx (1)
client/app/components/ThemeToggle.tsx (1)
ThemeToggle(6-35)
🔇 Additional comments (4)
client/app/components/Pages/LoginPage.tsx (1)
5-12: Dark-mode container styling looks correct.
Thebg-white dark:bg-gray-900+transition-colorsaddition is consistent with class-based theming.client/app/globals.css (1)
10-14: Global body theming via@layer baseis consistent.
This should play nicely with thedarkclass onhtml.client/app/components/Header/Header.tsx (1)
33-140: Dark-mode styling + ThemeToggle placement are coherent.
Transitions and dark variants look consistently applied across header, sidebar, and menu item states.client/app/layout.tsx (1)
12-40: Confirm that Header + ChatBot should appear with unauthenticated users.LoginPage is currently rendered conditionally within HomePage (when wallet is not connected), not as a separate route. Since HomePage is wrapped by ClientLayout, the Header and ChatBot appear above LoginPage content (indicated by the
pt-20padding). Verify this is the intended UX for unauthenticated users. If future auth flows require standalone pages without Header/ChatBot, consider using route groups to opt-out selectively.
| export function ThemeProvider({ children }: { children: React.ReactNode }) { | ||
| const [theme, setTheme] = useState<Theme>("light"); | ||
| const [mounted, setMounted] = useState(false); | ||
|
|
||
| // Load theme from localStorage on mount | ||
| useEffect(() => { | ||
| setMounted(true); | ||
| const savedTheme = localStorage.getItem("theme") as Theme; | ||
| if (savedTheme) { | ||
| setTheme(savedTheme); | ||
| document.documentElement.classList.toggle("dark", savedTheme === "dark"); | ||
| } else { | ||
| // Check system preference | ||
| const prefersDark = window.matchMedia("(prefers-color-scheme: dark)").matches; | ||
| const initialTheme = prefersDark ? "dark" : "light"; | ||
| setTheme(initialTheme); | ||
| document.documentElement.classList.toggle("dark", prefersDark); | ||
| } | ||
| }, []); | ||
|
|
||
| const toggleTheme = () => { | ||
| const newTheme = theme === "light" ? "dark" : "light"; | ||
| setTheme(newTheme); | ||
| localStorage.setItem("theme", newTheme); | ||
| document.documentElement.classList.toggle("dark", newTheme === "dark"); | ||
| }; |
There was a problem hiding this comment.
Validate localStorage theme + drop unused mounted.
Right now any truthy localStorage.getItem("theme") is trusted via a cast, and mounted is unused. Safer to validate and simplify.
export function ThemeProvider({ children }: { children: React.ReactNode }) {
const [theme, setTheme] = useState<Theme>("light");
- const [mounted, setMounted] = useState(false);
// Load theme from localStorage on mount
useEffect(() => {
- setMounted(true);
- const savedTheme = localStorage.getItem("theme") as Theme;
- if (savedTheme) {
- setTheme(savedTheme);
- document.documentElement.classList.toggle("dark", savedTheme === "dark");
+ const savedTheme = localStorage.getItem("theme");
+ if (savedTheme === "light" || savedTheme === "dark") {
+ setTheme(savedTheme);
+ document.documentElement.classList.toggle("dark", savedTheme === "dark");
} else {
// Check system preference
const prefersDark = window.matchMedia("(prefers-color-scheme: dark)").matches;
const initialTheme = prefersDark ? "dark" : "light";
setTheme(initialTheme);
document.documentElement.classList.toggle("dark", prefersDark);
}
}, []);🤖 Prompt for AI Agents
In client/app/context/ThemeContext.tsx around lines 13 to 38, the code trusts
any truthy localStorage value cast to Theme and keeps an unused mounted state;
validate the saved value against the allowed Theme union ("light" | "dark")
before applying it (fallback to system preference if invalid or missing), remove
the mounted state entirely, and simplify the effect to read localStorage, check
if value === "light" or "dark", apply document.documentElement.classList.toggle
appropriately, set the theme state, and keep the toggleTheme logic the same
(ensure it also writes only "light" or "dark" to localStorage).
1 participant
Description
Include a summary of the change and which issue is fixed. List any dependencies that are required for this change.
Fixes # (issue)
Type of change
Please mark the options that are relevant.
Checklist:
Summary by CodeRabbit
Release Notes
New Features
Styling
✏️ Tip: You can customize this high-level summary in your review settings.