-
Notifications
You must be signed in to change notification settings - Fork 62
Fix: Persist theme selection using localStorage #161
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
Fix: Persist theme selection using localStorage #161
Conversation
✅ Deploy Preview for github-spy ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
WalkthroughThe theme context logic was updated to initialize the theme mode state from localStorage, defaulting to 'light' if no valid value is found. Additionally, the effect that applies the theme mode to the document root now also persists any changes to localStorage, ensuring the user's theme preference is saved and restored across sessions. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Browser
participant ThemeContext
User->>Browser: Loads page
Browser->>ThemeContext: Initialize theme mode
ThemeContext->>Browser: Read 'theme' from localStorage
alt Value is 'dark'
ThemeContext->>Browser: Set mode to 'dark'
else Value is not 'dark'
ThemeContext->>Browser: Set mode to 'light'
end
User->>ThemeContext: Changes theme mode
ThemeContext->>Browser: Update mode and save to localStorage
ThemeContext->>Browser: Apply mode to document root
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Assessment against linked issues
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
🎉 Thank you @sharanya2504 for your contribution. Please make sure your PR follows https://github.com/GitMetricsLab/github_tracker/blob/main/CONTRIBUTING.md#-pull-request-guidelines
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.
| const [mode, setMode] = useState<'light' | 'dark'>(() => { | ||
| const savedMode = localStorage.getItem('theme'); | ||
| return savedMode === 'dark' ? 'dark' : 'light'; | ||
| }); |
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.
🛠️ Refactor suggestion
Add error handling for localStorage access.
The lazy initializer correctly reads from localStorage and defaults appropriately, but it lacks error handling for cases where localStorage might not be available (e.g., SSR, private browsing, or storage quota exceeded).
const [mode, setMode] = useState<'light' | 'dark'>(() => {
- const savedMode = localStorage.getItem('theme');
- return savedMode === 'dark' ? 'dark' : 'light';
+ try {
+ const savedMode = localStorage.getItem('theme');
+ return savedMode === 'dark' ? 'dark' : 'light';
+ } catch (error) {
+ return 'light';
+ }
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const [mode, setMode] = useState<'light' | 'dark'>(() => { | |
| const savedMode = localStorage.getItem('theme'); | |
| return savedMode === 'dark' ? 'dark' : 'light'; | |
| }); | |
| const [mode, setMode] = useState<'light' | 'dark'>(() => { | |
| try { | |
| const savedMode = localStorage.getItem('theme'); | |
| return savedMode === 'dark' ? 'dark' : 'light'; | |
| } catch (error) { | |
| return 'light'; | |
| } | |
| }); |
🤖 Prompt for AI Agents
In src/context/ThemeContext.tsx around lines 13 to 16, the useState lazy
initializer accesses localStorage without error handling, which can cause issues
in environments where localStorage is unavailable or restricted. Wrap the
localStorage.getItem call in a try-catch block to safely handle any errors and
default to 'light' mode if an error occurs, ensuring the app does not crash in
such scenarios.
| } else { | ||
| document.documentElement.classList.remove('dark'); | ||
| } | ||
| localStorage.setItem('theme', mode); |
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.
🛠️ Refactor suggestion
Add error handling for localStorage persistence.
The localStorage persistence works correctly but lacks error handling. If localStorage is unavailable or full, it could throw an exception and potentially break the theme functionality.
useEffect(() => {
if (mode === 'dark') {
document.documentElement.classList.add('dark');
} else {
document.documentElement.classList.remove('dark');
}
- localStorage.setItem('theme', mode);
+ try {
+ localStorage.setItem('theme', mode);
+ } catch (error) {
+ console.warn('Failed to save theme preference:', error);
+ }
}, [mode]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| localStorage.setItem('theme', mode); | |
| useEffect(() => { | |
| if (mode === 'dark') { | |
| document.documentElement.classList.add('dark'); | |
| } else { | |
| document.documentElement.classList.remove('dark'); | |
| } | |
| try { | |
| localStorage.setItem('theme', mode); | |
| } catch (error) { | |
| console.warn('Failed to save theme preference:', error); | |
| } | |
| }, [mode]); |
🤖 Prompt for AI Agents
In src/context/ThemeContext.tsx at line 25, the call to
localStorage.setItem('theme', mode) lacks error handling. Wrap this call in a
try-catch block to catch any exceptions thrown if localStorage is unavailable or
full, and handle the error gracefully, such as by logging it or silently failing
without breaking the theme functionality.
|
🎉🎉 Thank you for your contribution! Your PR #161 has been merged! 🎉🎉 |
Related Issue
Description
This PR adds persistence to the selected theme (light/dark) using
localStorage.When the user selects a theme, it will now be saved and retained even after refreshing or reopening the application.
How Has This Been Tested?
Screenshots (if applicable)
N/A – no visual change, just persistence across reloads.
Type of Change
Summary by CodeRabbit
New Features
Bug Fixes