mode toggle#22
Conversation
WalkthroughThe theme toggle component is refactored from a dropdown menu to a direct-click button with framer-motion animations and tooltip enhancements. The component maintains its public API while replacing the UI pattern and adding visual feedback on theme switches. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
components/mode-toggle.tsx (1)
22-47: Consider making aria-label dynamic to match tooltip.The
aria-labelis currently static ("Toggle theme"), while the tooltip dynamically shows the specific action ("Switch to Light/Dark mode"). For improved accessibility, the aria-label could match the tooltip text.Apply this diff:
<Button variant="ghost" size="icon" className="h-9 w-9" - aria-label="Toggle theme" + aria-label={`Switch to ${isDark ? "Light" : "Dark"} mode`} onClick={() => setTheme(isDark ? "light" : "dark")} >
📜 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 (2)
components/ui/tooltip.tsx (4)
TooltipProvider(61-61)Tooltip(61-61)TooltipTrigger(61-61)TooltipContent(61-61)components/ui/button.tsx (1)
Button(59-59)
🔇 Additional comments (2)
components/mode-toggle.tsx (2)
35-39: Icon logic is correct.The implementation correctly shows the Sun icon in dark mode and Moon icon in light mode, representing the theme the user will switch to. This pattern pairs well with the tooltip that clarifies the action.
27-27: This is intentional design, not a removed feature.The "system" theme is supported and configured at the application level (
app/layout.tsxusesdefaultTheme="system"withenableSystem={true}). The mode-toggle component is intentionally designed as a binary light/dark toggle to let users override the system preference, not as a tri-state selector. Users who prefer system theme can do so through the application's default theme configuration.
| import { Moon, Sun } from "lucide-react"; | ||
| import { useTheme } from "next-themes"; | ||
|
|
||
| import { Button } from "@/components/ui/button"; | ||
| import { | ||
| DropdownMenu, | ||
| DropdownMenuContent, | ||
| DropdownMenuItem, | ||
| DropdownMenuTrigger, | ||
| } from "@/components/ui/dropdown-menu"; | ||
| Tooltip, | ||
| TooltipContent, | ||
| TooltipProvider, | ||
| TooltipTrigger, | ||
| } from "@/components/ui/tooltip"; | ||
| import { motion } from "framer-motion"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify library versions in npm registry
echo "Checking lucide-react version 0.511.0..."
npm view lucide-react@0.511.0 version 2>&1
echo -e "\nChecking next-themes version 0.4.6..."
npm view next-themes@0.4.6 version 2>&1
echo -e "\nChecking framer-motion version 12.15.0..."
npm view framer-motion@12.15.0 version 2>&1
echo -e "\nChecking available versions of lucide-react (recent)..."
npm view lucide-react versions --json 2>&1 | tail -20
echo -e "\nChecking available versions of next-themes (recent)..."
npm view next-themes versions --json 2>&1 | tail -20
echo -e "\nChecking available versions of framer-motion (recent)..."
npm view framer-motion versions --json 2>&1 | tail -20Repository: StabilityNexus/HammerAuctionHouse-WebUI
Length of output: 1195
🏁 Script executed:
#!/bin/bash
# Check for breaking changes and compatibility notes in newer versions
echo "=== Lucide-react changelog summary ==="
npm view lucide-react@latest description 2>&1 | head -5
echo -e "\n=== Next-themes changelog summary ==="
npm view next-themes@latest description 2>&1 | head -5
echo -e "\n=== Framer-motion changelog summary ==="
npm view framer-motion@latest description 2>&1 | head -5
echo -e "\n=== Checking if lucide-react has major version differences ==="
npm view lucide-react dist-tags 2>&1
echo -e "\n=== Checking if framer-motion has major version differences ==="
npm view framer-motion dist-tags 2>&1Repository: StabilityNexus/HammerAuctionHouse-WebUI
Length of output: 671
Consider updating lucide-react and framer-motion to newer versions.
lucide-react@0.511.0 is 50 versions behind the latest (0.561.0), and framer-motion@12.15.0 is 8 patch versions behind the latest (12.23.26). Both updates are within the same major version and should be backward compatible. next-themes@0.4.6 is already at the latest stable version.
🤖 Prompt for AI Agents
In components/mode-toggle.tsx lines 3 to 12: the imports reference lucide-react
and framer-motion versions that are behind; update package.json to bump
lucide-react to 0.561.0 and framer-motion to 12.23.26 (or run npm/yarn upgrade
for those packages), then run install to update the lockfile, rebuild and run
tests to ensure no breakage; if any icons or motion APIs changed, adjust
imports/usages in this file (and project-wide) to match the new versions and
commit the updated package.json and lockfile.
| export function ModeToggle() { | ||
| const { setTheme } = useTheme(); | ||
| const { resolvedTheme, setTheme } = useTheme(); | ||
| const isDark = resolvedTheme === "dark"; |
There was a problem hiding this comment.
Handle undefined resolvedTheme during hydration.
The resolvedTheme from next-themes is undefined during SSR and initial client render until hydration completes. This causes isDark to incorrectly evaluate to false, leading to hydration mismatches, console errors, and a flash of the wrong icon.
Apply this diff to handle the undefined state:
export function ModeToggle() {
const { resolvedTheme, setTheme } = useTheme();
- const isDark = resolvedTheme === "dark";
+ const isDark = resolvedTheme === "dark";
+
+ // Prevent hydration mismatch by not rendering until theme is resolved
+ if (!resolvedTheme) {
+ return (
+ <Button variant="ghost" size="icon" className="h-9 w-9" disabled>
+ <Sun className="h-[1.2rem] w-[1.2rem] opacity-0" />
+ </Button>
+ );
+ }📝 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.
| export function ModeToggle() { | |
| const { setTheme } = useTheme(); | |
| const { resolvedTheme, setTheme } = useTheme(); | |
| const isDark = resolvedTheme === "dark"; | |
| export function ModeToggle() { | |
| const { resolvedTheme, setTheme } = useTheme(); | |
| const isDark = resolvedTheme === "dark"; | |
| // Prevent hydration mismatch by not rendering until theme is resolved | |
| if (!resolvedTheme) { | |
| return ( | |
| <Button variant="ghost" size="icon" className="h-9 w-9" disabled> | |
| <Sun className="h-[1.2rem] w-[1.2rem] opacity-0" /> | |
| </Button> | |
| ); | |
| } |
🤖 Prompt for AI Agents
In components/mode-toggle.tsx around lines 14 to 16, resolvedTheme can be
undefined during SSR/initial render which causes isDark to wrongly evaluate
false and produces hydration mismatches; update the component to detect the
undefined state (e.g., track a mounted/hydrated flag with useState/useEffect or
guard with typeof resolvedTheme !== "undefined") and avoid rendering the
theme-dependent icon until hydration completes (return null or a placeholder) or
compute isDark only when resolvedTheme is defined (const isDark = resolvedTheme
=== "dark" && typeof resolvedTheme !== "undefined"); ensure setTheme usage
remains unchanged and the component resumes normal rendering after hydration.
| <motion.div | ||
| key={resolvedTheme} | ||
| initial={{ rotate: -90, scale: 0.8, opacity: 0 }} | ||
| animate={{ rotate: 0, scale: 1, opacity: 1 }} | ||
| transition={{ duration: 0.25 }} | ||
| > |
There was a problem hiding this comment.
Animation will trigger unnecessarily during hydration.
Using resolvedTheme as the key causes the icon to remount and animate when the theme hydrates (transitions from undefined to the actual theme value). This results in an unwanted animation on initial page load.
This issue is tied to the hydration concern raised earlier. Once resolvedTheme is guarded against undefined, this animation behavior will be corrected.
🤖 Prompt for AI Agents
In components/mode-toggle.tsx around lines 29 to 34, the motion.div uses
resolvedTheme as the React key which causes a remount and animation when
resolvedTheme changes from undefined to a value during hydration; stop using a
fluctuating value as the key — either ensure resolvedTheme is never undefined
before rendering (provide a stable fallback) or remove the key and use a
constant key (e.g., "mode-toggle-icon") and drive animation state via
props/variants based on the guarded theme value so the icon does not remount and
animate on initial hydration.
|
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: |
The dropdown-based theme switcher was replaced with a single-click, icon-based toggle that provides immediate visual feedback, improves discoverability, and enhances accessibility.
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.