-
Notifications
You must be signed in to change notification settings - Fork 155
fix: prevent hidden navbar clicks using pointer-events #186
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
@purubhoite is attempting to deploy a commit to the AJEET PRATAP SINGH's projects Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughNavbar animation states were extended to include pointerEvents and aria-hidden handling: hidden state sets Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Navbar
Note over Navbar: Animation states include opacity + pointerEvents
User->>Navbar: trigger show
activate Navbar
Navbar-->>User: animate to opacity:1, pointerEvents:auto (aria-hidden=false)
deactivate Navbar
User->>Navbar: trigger hide
activate Navbar
Navbar-->>User: animate to opacity:0, pointerEvents:none (aria-hidden=true)
deactivate Navbar
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/web/src/components/landing-sections/navbar.tsx (1)
48-50: Excellent implementation addressing both pointer interaction and accessibility!The addition of
pointerEventscontrol andaria-hidden={!showNavbar}correctly addresses the original issue and the keyboard accessibility concern raised in the previous review. When the navbar is hidden:
pointerEvents: "none"prevents mouse/touch clicksaria-hidden={true}hides it from screen readersThe implementation is syntactically correct and represents a clear improvement.
Optional enhancement: For even more robust keyboard navigation blocking, consider adding the
inertattribute alongsidearia-hidden:<motion.nav initial={{ opacity: 0, pointerEvents: "none" }} animate={showNavbar ? { opacity: 1, pointerEvents: "auto" } : { opacity: 0, pointerEvents: "none" }} aria-hidden={!showNavbar} + inert={!showNavbar ? "" : undefined} transition={{ duration: 0.3 }}The
inertattribute comprehensively removes the element from user interaction (including Tab navigation) in modern browsers, thougharia-hiddenis sufficient for most cases.
Fixed issue #152 by adding pointerEvents: "none" to the navbar when it is hidden.
Summary by CodeRabbit