-
Notifications
You must be signed in to change notification settings - Fork 102
fix: Dark Mode Toggle Not Functioning on Click (#117) #196
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
- Fix variable shadowing in theme-provider.tsx by renaming state setter to setThemeState - Wrap App component with ThemeProvider in main.tsx to enable theme context - Theme toggle now correctly updates DOM classes and persists to localStorage - Users can now switch between light, dark, and system theme preferences
WalkthroughThe changes refactor the theme provider's internal state management by renaming the setter from Changes
Sequence DiagramsequenceDiagram
participant App as App
participant TP as ThemeProvider
participant LS as localStorage
participant UI as UI Components
App->>TP: Rendered with defaultTheme<br/>(system) & storageKey
TP->>LS: Initialize from localStorage<br/>(or use default)
TP->>UI: Provide theme context value<br/>& setTheme callback
UI->>TP: User clicks theme toggle<br/>calls setTheme(newTheme)
TP->>TP: setThemeState(newTheme) executes
TP->>LS: Persist theme to localStorage
TP->>UI: Context updated, components<br/>re-render with new theme
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related issues
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
Frontend/src/components/theme-provider.tsx (2)
1-1: Remove unused React import.With React 19 and the modern JSX transform enabled, the explicit
import React from "react"is no longer necessary.Apply this diff:
-import React from "react"; - import { createContext, useContext, useEffect, useState } from "react";
10-15: Improve type safety for component props.The
...propsparameter is typed asany, which bypasses TypeScript's type checking. Consider defining a proper interface for the component props.Apply this diff:
+type ThemeProviderProps = { + children: React.ReactNode; + defaultTheme?: string; + storageKey?: string; +} & React.ComponentProps<typeof ThemeProviderContext.Provider>; + export function ThemeProvider({ children, defaultTheme = "system", storageKey = "vite-ui-theme", ...props -}: any) { +}: ThemeProviderProps) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Frontend/src/components/theme-provider.tsx(2 hunks)Frontend/src/main.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
Frontend/src/main.tsx (1)
Frontend/src/components/theme-provider.tsx (1)
ThemeProvider(10-51)
🔇 Additional comments (5)
Frontend/src/components/theme-provider.tsx (2)
16-18: LGTM! Variable shadowing fix is correct.Renaming the state setter from
setThemetosetThemeStateproperly resolves the variable shadowing issue that prevented the theme toggle from functioning. The initialization logic correctly reads from localStorage with fallback to defaultTheme.
40-42: LGTM! The setTheme implementation correctly updates state and persists to localStorage.The context value setter now properly calls
setThemeState(newTheme)to update the state, and persists the selection to localStorage using the providedstorageKey. This addresses the core issue described in the PR objectives.Frontend/src/main.tsx (3)
7-7: LGTM! ThemeProvider import is correct.The import path correctly references the local theme-provider component.
12-14: LGTM! ThemeProvider integration correctly wraps the App.The ThemeProvider is properly integrated with appropriate props (
defaultTheme="system"andstorageKey="vite-ui-theme"), making the theme context available throughout the application. This addresses the missing wrapper issue described in the PR objectives.
10-10: Verify why StrictMode is disabled.StrictMode is commented out, which disables React's additional development-mode checks (double-invocation of effects, deprecated API warnings, etc.). This is unusual and may hide potential issues.
Can you confirm whether StrictMode was intentionally disabled? If this is temporary, consider re-enabling it to benefit from React 19's enhanced strict mode checks.
Closes #117 and #195
📝 Description
This pull request fixes the non-functional dark mode toggle button in the application. The issue was caused by two problems:
theme-provider.tsxwhere the state setter was namedsetTheme, conflicting with the context method name, preventing state updates.main.tsxthat prevented the theme context from being accessible to components using theuseTheme()hook.With these fixes, users can now successfully toggle between light mode, dark mode, and system theme preferences. The selected theme is persisted in localStorage and applies correctly to the entire application.
🔧 Changes Made
Frontend/src/components/theme-provider.tsx
setThemetosetThemeStatesetThememethod to callsetThemeState(newTheme)instead ofsetTheme(theme)Frontend/src/main.tsx
ThemeProviderfrom./components/theme-providerAppcomponent with<ThemeProvider defaultTheme="system" storageKey="vite-ui-theme">🎯 Related Issues
Fixes #117 - Dark Mode Toggle Not Functioning on Click
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.