fix(theme): resolve ThemeToggle hydration mismatch#688
fix(theme): resolve ThemeToggle hydration mismatch#688nino-chavez wants to merge 1 commit intoOpenCut-app:mainfrom
Conversation
👷 Deploy request for appcut pending review.Visit the deploys page to approve it
|
|
@nino-chavez is attempting to deploy a commit to the OpenCut OSS Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a client-mounted guard to ThemeToggle so theme-dependent UI is rendered only after hydration, preventing SSR/client mismatches; renders a neutral placeholder until mounted and retains existing toggle behavior and API. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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
🤖 Fix all issues with AI agents
In `@apps/web/src/components/theme-toggle.tsx`:
- Around line 32-48: Add an explicit type attribute to the Button used in
ThemeToggle to avoid defaulting to type="submit": update the JSX for the Button
component (the one rendering Sun/Moon and calling setTheme based on isDark) to
include type="button" alongside existing props (size, variant, className,
onClick) so it won't submit forms when nested inside a form.
🧹 Nitpick comments (2)
apps/web/src/components/theme-toggle.tsx (2)
13-13: Consider usingresolvedThemeinstead ofthemeto handle system preference correctly.When a user has
themeset to"system", checkingtheme === "dark"will be false even if the system is in dark mode. TheuseThemehook providesresolvedThemewhich reflects the actual computed theme (accounting for system preference).♻️ Proposed fix
export function ThemeToggle({ className }: ThemeToggleProps) { - const { theme, setTheme } = useTheme(); + const { resolvedTheme, setTheme } = useTheme(); const [mounted, setMounted] = useState(false);- const isDark = theme === "dark"; + const isDark = resolvedTheme === "dark";Also applies to: 30-30
23-23: TheclassNamefallback logic may not behave as expected.Using
className ?? "h-7"means if anyclassNameis passed, the default"h-7"is completely replaced rather than merged. If callers expect to add classes while keeping the default height, this could cause layout inconsistencies.If the intent is to allow full override, this is fine. Otherwise, consider merging classes.
♻️ Example using cn utility (if available)
- className={className ?? "h-7"} + className={cn("h-7", className)}Also applies to: 36-36
| return ( | ||
| <Button | ||
| size="icon" | ||
| variant="text" | ||
| className={className ?? "h-7"} | ||
| onClick={() => setTheme(isDark ? "light" : "dark")} | ||
| > | ||
| {isDark ? ( | ||
| <Sun className="!size-[1.1rem]" /> | ||
| ) : ( | ||
| <Moon className="!size-[1.1rem]" /> | ||
| )} | ||
| <span className="sr-only"> | ||
| Switch to {isDark ? "light" : "dark"} mode | ||
| </span> | ||
| </Button> | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find apps/web/src -name "*.tsx" -o -name "*.ts" | xargs grep -l "export.*Button" | head -20Repository: OpenCut-app/OpenCut
Length of output: 239
🏁 Script executed:
fd -t f -e tsx -e ts apps/web/src | xargs grep -l "const Button\|export.*Button\|function Button" | head -20Repository: OpenCut-app/OpenCut
Length of output: 416
🏁 Script executed:
cat -n apps/web/src/components/ui/button.tsxRepository: OpenCut-app/OpenCut
Length of output: 2837
🏁 Script executed:
grep -r "theme-toggle" apps/web/src --include="*.tsx" --include="*.ts" -B 2 -A 2 | head -50Repository: OpenCut-app/OpenCut
Length of output: 827
🏁 Script executed:
grep -r "ThemeToggle\|theme-toggle" apps/web/src --include="*.tsx" --include="*.ts" | grep -E "import|from" | head -10Repository: OpenCut-app/OpenCut
Length of output: 239
🏁 Script executed:
cat -n apps/web/src/components/header.tsxRepository: OpenCut-app/OpenCut
Length of output: 1993
🏁 Script executed:
cat -n apps/web/src/components/editor/editor-header.tsxRepository: OpenCut-app/OpenCut
Length of output: 5222
Add type="button" to the Button component in ThemeToggle.
Per coding guidelines, button elements must always include a type attribute. The Button component doesn't set type="button" by default, and without it, the browser defaults to type="submit", which could cause unintended behavior if this component is used in a form context in the future.
<Button
type="button"
size="icon"
variant="text"
className={className ?? "h-7"}
onClick={() => setTheme(isDark ? "light" : "dark")}
>🤖 Prompt for AI Agents
In `@apps/web/src/components/theme-toggle.tsx` around lines 32 - 48, Add an
explicit type attribute to the Button used in ThemeToggle to avoid defaulting to
type="submit": update the JSX for the Button component (the one rendering
Sun/Moon and calling setTheme based on isDark) to include type="button"
alongside existing props (size, variant, className, onClick) so it won't submit
forms when nested inside a form.
- Added mounted state to prevent SSR/client hydration mismatch - Show Sun icon in dark mode, Moon icon in light mode - Render placeholder during SSR to maintain layout stability - Fixed className prop not being applied Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
nino-chavez
force-pushed
the
fix/theme-toggle-hydration-648
branch
from
cf0f9a6 to
55a29be
Compare
1 participant
Summary
mountedstate pattern to prevent SSR/client icon mismatchRelated Issue
Fixes #648
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Improvements