fix(mobileMenuOpen): make exit animation work #248
Conversation
|
@smmitc is attempting to deploy a commit to the tweakcn OSS program Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughBoth header components now wrap their mobile menu rendering with 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: 2
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/figma-header.tsx(2 hunks)components/home/header.tsx(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
components/figma-header.tsx (1)
utils/format.ts (1)
formatCompactNumber(4-9)
🔇 Additional comments (4)
components/home/header.tsx (2)
10-10: LGTM! Import correctly adds AnimatePresence.The import change properly adds
AnimatePresencewhich is required for exit animations in the Motion library.
140-147: LGTM! AnimatePresence wrapper correctly enables exit animations.The mobile menu is now properly wrapped in
AnimatePresencewith amotion.divthat has matchinginitialandexitstates. This will enable the exit animation to work as intended per the Motion library requirements.components/figma-header.tsx (2)
10-10: LGTM! Import correctly adds AnimatePresence.The import change properly adds
AnimatePresenceto enable exit animations for the mobile menu, consistent with the changes incomponents/home/header.tsx.
80-110: LGTM! AnimatePresence wrapper correctly enables exit animations.The mobile menu implementation properly wraps the conditional rendering with
AnimatePresenceand uses amotion.divwith matchinginitialandexitstates. This enables smooth exit animations as required by the Motion library. The simplified mobile menu with the GitHub link works correctly.
| onClick={(e) => { | ||
| handleScrollToSection(e); | ||
| setMobileMenuOpen(false); | ||
| }} |
There was a problem hiding this comment.
Fix navigation bug: Conditionally call handleScrollToSection.
The onClick handler unconditionally calls handleScrollToSection for all menu items, but this breaks the "/pricing" link because handleScrollToSection calls e.preventDefault(). The desktop version (line 79) correctly calls handleScrollToSection only for hash links.
🔎 Apply this diff to fix the navigation bug:
onClick={(e) => {
- handleScrollToSection(e);
+ if (item.href.startsWith("#")) {
+ handleScrollToSection(e);
+ }
setMobileMenuOpen(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.
| onClick={(e) => { | |
| handleScrollToSection(e); | |
| setMobileMenuOpen(false); | |
| }} | |
| onClick={(e) => { | |
| if (item.href.startsWith("#")) { | |
| handleScrollToSection(e); | |
| } | |
| setMobileMenuOpen(false); | |
| }} |
🤖 Prompt for AI Agents
In components/home/header.tsx around lines 156 to 159, the onClick handler
currently always calls handleScrollToSection which prevents navigation for
normal links (e.g. /pricing); change it to first read the clicked link's href
(e.g. const href = (e.currentTarget as HTMLAnchorElement).getAttribute('href'))
and only call handleScrollToSection(e) when href is a hash link
(startsWith('#')); always call setMobileMenuOpen(false) afterwards so the menu
still closes for all links.
| }} | ||
| className="group relative overflow-hidden py-2 text-sm font-medium" | ||
| > | ||
| <span className="relative z-10">{item.href}</span> |
There was a problem hiding this comment.
Fix critical bug: Display label instead of href.
Line 162 displays item.href instead of item.label, which would show technical values like "#examples" or "/pricing" instead of user-friendly labels like "Examples" or "Pricing".
🔎 Apply this diff to fix the display bug:
- <span className="relative z-10">{item.href}</span>
+ <span className="relative z-10">{item.label}</span>📝 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.
| <span className="relative z-10">{item.href}</span> | |
| <span className="relative z-10">{item.label}</span> |
🤖 Prompt for AI Agents
In components/home/header.tsx around line 162 the span currently renders
item.href which shows raw route values; change it to render the human-facing
item.label (e.g., replace item.href with item.label), optionally using a safe
fallback like item.label || item.href to avoid breaking if label is missing.
1 participant
The mobile menu contains an exit animation. But it is not working because it is not wrapped in an AnimatePresence component as described here. This is fixed by this PR.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.