add faq in the navbar #129
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThe changes add an id attribute to the FAQ section component and implement responsive mobile navigation with a toggle menu. The mobile menu includes all navigation links, closes on item click, and responds to the Escape key. Changes
Sequence DiagramsequenceDiagram
actor User
participant Mobile as Mobile Screen
participant Navbar as Navbar Component
participant Menu as Mobile Menu Panel
User->>Mobile: Tap Menu Icon
Mobile->>Navbar: Toggle isOpen state
Navbar->>Menu: Render dropdown panel
Menu->>User: Show navigation links
User->>Menu: Click navigation link
Menu->>Navbar: Set isOpen to false
Navbar->>Mobile: Hide menu panel
User->>Mobile: Press Escape key
Mobile->>Navbar: Handle Escape event
Navbar->>Navbar: Set isOpen to false & blur focus
Navbar->>Menu: Close menu panel
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 0
🧹 Nitpick comments (1)
apps/web/src/components/landing-sections/navbar.tsx (1)
111-149: Mobile menu implementation is solid.The dropdown menu is well-implemented with proper animation, styling, and click handlers that close the menu on navigation. All interactive elements correctly update the menu state.
Consider these optional UX enhancements:
- Add click-outside-to-close functionality for better mobile UX
- Implement body scroll lock when the menu is open to prevent background scrolling
Example for click-outside-to-close:
const menuRef = React.useRef<HTMLDivElement>(null); React.useEffect(() => { const handleClickOutside = (event: MouseEvent) => { if (menuRef.current && !menuRef.current.contains(event.target as Node)) { setIsOpen(false); } }; if (isOpen) { document.addEventListener("mousedown", handleClickOutside); return () => document.removeEventListener("mousedown", handleClickOutside); } }, [isOpen]);Then add
ref={menuRef}to the motion.div at Line 112.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/src/components/faq/FaqSection.tsx(1 hunks)apps/web/src/components/landing-sections/navbar.tsx(6 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/web/src/components/landing-sections/navbar.tsx (1)
apps/web/src/components/icons/icons.tsx (2)
Github(195-197)Terminal(1-5)
🔇 Additional comments (6)
apps/web/src/components/faq/FaqSection.tsx (1)
12-12: LGTM! Anchor ID added for navigation.The
id="faq"attribute correctly enables anchor navigation from the navbar FAQ link.apps/web/src/components/landing-sections/navbar.tsx (5)
6-6: LGTM! Mobile menu state and icons added.The imports and state setup for the mobile navigation are correct.
Also applies to: 16-16
18-28: LGTM! Escape key handler is well-implemented.The keyboard accessibility enhancement properly closes the mobile menu on Escape key press and blurs the active element to reset focus.
43-43: LGTM! FAQ navigation link added.The FAQ link correctly references the
#faqanchor added to the FaqSection component.
58-65: Mobile toggle button looks good, but verify layout intent.The toggle button implementation is solid with proper accessibility attributes (
aria-labelandaria-expanded). However, note that with the current flex layout (justify-between), the toggle button appears on the left and the logo on the right on mobile devices. Verify this is intentional, as typically the logo remains on the left for brand consistency.
94-94: LGTM! Desktop buttons properly hidden on mobile.
corrects #124
contributed by @ssscharan149
Summary by CodeRabbit