feat: add SSR-safe dark/light theme toggle in navbar#20
Conversation
WalkthroughThe ModeToggle component is refactored to add a client-only rendering guard and simplify the UI from a multi-option dropdown menu to a single toggle button. Theme switching logic is internalized, and visual icons change between Moon and Sun based on the current theme state. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
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
🧹 Nitpick comments (1)
components/mode-toggle.tsx (1)
28-39: LGTM! Good accessibility implementation.The button includes proper ARIA labels and screen reader text. The implementation is clean and accessible.
Optional: Consider whether the icon should represent the current state (Moon when dark) or the action that will be taken (Sun to switch to light). Current implementation shows the current state, which is a valid pattern. Some UIs show the opposite (the state you'll switch to). Either is fine—just ensure it's intuitive for your users.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/mode-toggle.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
components/mode-toggle.tsx (1)
components/ui/button.tsx (1)
Button(59-59)
🔇 Additional comments (2)
components/mode-toggle.tsx (2)
10-17: LGTM! SSR safety correctly implemented.The client-only mounting guard properly prevents server-side rendering and avoids hydration mismatches. This is the recommended pattern for next-themes.
24-26: Verify: Users lose system theme preference after toggling.The toggle function always sets the theme to either "light" or "dark", never returning to "system". This means users who initially had the system preference will permanently lose it after the first toggle, with no way to restore it through this UI.
Is this behavior intentional? If users should be able to return to system preference, consider either:
- Adding a three-way cycle: light → dark → system → light
- Providing a separate control to reset to system preference
| const isDark = | ||
| theme === "dark" || | ||
| (theme === "system" && | ||
| window.matchMedia?.("(prefers-color-scheme: dark)").matches); |
There was a problem hiding this comment.
Fix potential runtime error with matchMedia.
If window.matchMedia is undefined or returns a falsy value, the optional chaining on line 22 returns undefined, and then accessing .matches on undefined would throw a TypeError.
Apply this diff to safely handle the case:
- const isDark =
- theme === "dark" ||
- (theme === "system" &&
- window.matchMedia?.("(prefers-color-scheme: dark)").matches);
+ const isDark =
+ theme === "dark" ||
+ (theme === "system" &&
+ window.matchMedia?.("(prefers-color-scheme: dark)")?.matches);Or use a more explicit fallback:
- const isDark =
- theme === "dark" ||
- (theme === "system" &&
- window.matchMedia?.("(prefers-color-scheme: dark)").matches);
+ const isDark =
+ theme === "dark" ||
+ (theme === "system" &&
+ (window.matchMedia?.("(prefers-color-scheme: dark)")?.matches ?? false));📝 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 isDark = | |
| theme === "dark" || | |
| (theme === "system" && | |
| window.matchMedia?.("(prefers-color-scheme: dark)").matches); | |
| const isDark = | |
| theme === "dark" || | |
| (theme === "system" && | |
| window.matchMedia?.("(prefers-color-scheme: dark)")?.matches); |
| const isDark = | |
| theme === "dark" || | |
| (theme === "system" && | |
| window.matchMedia?.("(prefers-color-scheme: dark)").matches); | |
| const isDark = | |
| theme === "dark" || | |
| (theme === "system" && | |
| (window.matchMedia?.("(prefers-color-scheme: dark)")?.matches ?? false)); |
🤖 Prompt for AI Agents
In components/mode-toggle.tsx around lines 19 to 22, the current expression may
call .matches on undefined when window.matchMedia is absent; update the check to
safely access matchMedia and provide a boolean fallback. Specifically, verify
window exists and matchMedia is a function (or guard for its presence), call
window.matchMedia("(prefers-color-scheme: dark)") into a variable, then read its
.matches only if the result is truthy, otherwise fall back to false; ensure the
overall expression returns a boolean so isDark never attempts to access .matches
on undefined.
|
Please resolve the merge conflicts before review. Your PR will only be reviewed by a maintainer after all conflicts have been resolved. 📺 Watch this video to understand why conflicts occur and how to resolve them: |
|
hi @sachingwala are you available to resolve this ? |
3 participants
PR Description:
Overview
This PR replaces the existing dropdown-based theme selector in the navigation bar with a single, intuitive toggle button. The goal is to improve user experience by making it easier and faster to switch between Light and Dark themes, while ensuring SSR safety and accessibility.
Key Features
Single toggle button for light/dark theme switching
Immediate visual feedback using Sun 🌞 and Moon 🌙 icons
SSR-safe implementation to prevent hydration warnings
Accessible with aria-label and sr-only for screen readers
Supports system theme detection (optional via next-themes)
Motivation
Dropdown required multiple clicks and was not easily discoverable
Users had no immediate visual indication of the current theme
This improvement makes theme switching more intuitive and visually clear, especially for first-time users
Technical Details
useTheme from next-themes handles theme state
Hydration mismatches avoided by rendering the toggle only on the client using useEffect
Icons dynamically update based on the active theme
Toggle persists user preference using next-themes default behavior
Testing
Verified theme toggle works on desktop and mobile
Checked that SSR hydration warnings are resolved
Ensured accessibility: screen readers announce toggle, keyboard navigation works
Verified layout remains consistent with no impact on navbar functionality
Screenshots / GIFs (optional)
Sun icon for Light theme, Moon icon for Dark theme
Toggle button in the navigation bar
Impact
Improves user experience and discoverability
Eliminates extra clicks required for theme switching
Prevents hydration errors during server-side rendering
Related Issue
Closes / addresses: #15
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.