Refactor: Replace dropdown theme menu with single-click toggle button…#21
Conversation
WalkthroughThis PR refactors the mode-toggle component, replacing a dropdown-based theme selector with a compact icon button featuring tooltips. The component now includes hydration-safe rendering, cycles through three theme states (light, dark, system), and dynamically displays icons based on the current theme. Changes
Sequence DiagramsequenceDiagram
participant User
participant ModeToggle as ModeToggle<br/>(Component)
participant Effect as useEffect<br/>(Hydration)
participant Theme as next-themes<br/>(Theme Provider)
participant UI as Button/Tooltip<br/>(UI)
User->>ModeToggle: Component renders (SSR)
ModeToggle->>Effect: Register effect (mounted flag: false)
Effect-->>UI: Initially rendered (placeholder)
Note over Effect: Browser hydrates component
Effect->>Effect: Set mounted = true
Effect-->>UI: Re-render with mounted state
User->>UI: Click button
UI->>ModeToggle: Trigger nextTheme()
ModeToggle->>Theme: Call next-themes setTheme()
Theme->>Theme: Cycle theme state
Theme-->>ModeToggle: Update resolvedTheme
ModeToggle->>ModeToggle: Select icon (Moon/Sun/Monitor)
ModeToggle->>ModeToggle: Update tooltip text
ModeToggle-->>UI: Re-render with new icon & tooltip
UI-->>User: Display updated theme indicator
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (1)
components/mode-toggle.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
components/mode-toggle.tsx (2)
components/ui/button.tsx (1)
Button(59-59)components/ui/tooltip.tsx (4)
TooltipProvider(61-61)Tooltip(61-61)TooltipTrigger(61-61)TooltipContent(61-61)
🔇 Additional comments (5)
components/mode-toggle.tsx (5)
1-14: LGTM!Imports are well-organized and all are used in the component. Good choice using the existing project Tooltip components rather than introducing new dependencies.
16-35: LGTM!The hydration-safety pattern using
mountedstate is the correct approach fornext-themes. The placeholder button maintains visual consistency with matching dimensions and a disabled state.
37-46: LGTM!Theme cycling logic is correct. Using
theme(user selection) rather thanresolvedThemeis the right choice for determining the next state in the cycle.
66-78: LGTM!Tooltip text logic correctly uses
themeto determine the current selection and provides helpful context about the current state and next action.
80-102: Good accessibility implementation.The component has solid accessibility with
aria-labeland screen reader text. The Tooltip integration is clean.Minor observation:
TooltipProvideris typically placed at the app root level to share delay settings across all tooltips. However, per-component usage works correctly and keeps this component self-contained.
| // Determine which icon to display based on resolved theme | ||
| const renderIcon = () => { | ||
| switch (resolvedTheme) { | ||
| case "dark": | ||
| return ( | ||
| <Moon className="h-[1.2rem] w-[1.2rem] rotate-0 scale-100 transition-all duration-300" /> | ||
| ); | ||
| case "light": | ||
| return ( | ||
| <Sun className="h-[1.2rem] w-[1.2rem] rotate-0 scale-100 transition-all duration-300" /> | ||
| ); | ||
| default: | ||
| return ( | ||
| <Monitor className="h-[1.2rem] w-[1.2rem] rotate-0 scale-100 transition-all duration-300" /> | ||
| ); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Monitor icon will never display - resolvedTheme is never "system".
The resolvedTheme from next-themes always resolves to the actual applied theme ("light" or "dark"), never "system". When the user selects system mode, resolvedTheme reflects the OS preference. This means the default case never executes and the Monitor icon is unreachable.
To show Monitor when the user has selected system theme, use theme instead:
// Determine which icon to display based on resolved theme
const renderIcon = () => {
- switch (resolvedTheme) {
+ switch (theme) {
+ case "system":
+ return (
+ <Monitor className="h-[1.2rem] w-[1.2rem] rotate-0 scale-100 transition-all duration-300" />
+ );
case "dark":
return (
<Moon className="h-[1.2rem] w-[1.2rem] rotate-0 scale-100 transition-all duration-300" />
);
- case "light":
- return (
- <Sun className="h-[1.2rem] w-[1.2rem] rotate-0 scale-100 transition-all duration-300" />
- );
default:
return (
- <Monitor className="h-[1.2rem] w-[1.2rem] rotate-0 scale-100 transition-all duration-300" />
+ <Sun className="h-[1.2rem] w-[1.2rem] rotate-0 scale-100 transition-all duration-300" />
);
}
};Alternatively, if you want the icon to reflect the actual appearance when in system mode (Sun/Moon based on OS) while showing Monitor only as a deliberate design choice when theme === "system", adjust the logic accordingly.
📝 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.
| // Determine which icon to display based on resolved theme | |
| const renderIcon = () => { | |
| switch (resolvedTheme) { | |
| case "dark": | |
| return ( | |
| <Moon className="h-[1.2rem] w-[1.2rem] rotate-0 scale-100 transition-all duration-300" /> | |
| ); | |
| case "light": | |
| return ( | |
| <Sun className="h-[1.2rem] w-[1.2rem] rotate-0 scale-100 transition-all duration-300" /> | |
| ); | |
| default: | |
| return ( | |
| <Monitor className="h-[1.2rem] w-[1.2rem] rotate-0 scale-100 transition-all duration-300" /> | |
| ); | |
| } | |
| }; | |
| // Determine which icon to display based on resolved theme | |
| const renderIcon = () => { | |
| switch (theme) { | |
| case "system": | |
| return ( | |
| <Monitor className="h-[1.2rem] w-[1.2rem] rotate-0 scale-100 transition-all duration-300" /> | |
| ); | |
| case "dark": | |
| return ( | |
| <Moon className="h-[1.2rem] w-[1.2rem] rotate-0 scale-100 transition-all duration-300" /> | |
| ); | |
| default: | |
| return ( | |
| <Sun className="h-[1.2rem] w-[1.2rem] rotate-0 scale-100 transition-all duration-300" /> | |
| ); | |
| } | |
| }; |
🤖 Prompt for AI Agents
In components/mode-toggle.tsx around lines 48 to 64, the Monitor icon is
unreachable because resolvedTheme never equals "system"; use theme (the user's
selected preference) to detect when they chose "system". Change the render logic
to first check if theme === "system" and return the Monitor icon, otherwise
switch on resolvedTheme to return Sun or Moon (or simply use theme instead of
resolvedTheme if you want Monitor only when theme is "system"); ensure you
import/receive theme and resolvedTheme from next-themes and preserve the
existing classes and transitions.
|
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 @yogeshkumawat2027 are you available to resolve this ? |
3 participants
🎯 Overview
Refactored the theme toggle component to replace the dropdown menu with an intuitive single-click toggle button, significantly improving user experience and accessibility.
🔴 Problem Statement
The current theme switcher required multiple clicks (click icon → select from dropdown) and lacked clear visual feedback about the active theme:
✅ Solution Implemented
Replaced the
DropdownMenuwith a single-click toggle button that cycles through Light → Dark → System → Light with contextual visual feedback.Key Improvements:
🎨 Technical Changes
Added Features:
resolvedTheme(Sun/Moon/Monitor)mountedstatearia-labeland screen reader textDependencies:
next-themeslibraryTooltipcomponent from UI library🧪 Testing Checklist
📱 User Experience Impact
Files Changed
components/mode-toggle.tsx- Complete refactor of theme toggle componentRelated Issues
Closes #[issue-number] (if applicable)
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.