-
-
Notifications
You must be signed in to change notification settings - Fork 42
fix(docs): sidebar doesnt close on navigation #87
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
Conversation
|
@Kenzo-Wada is attempting to deploy a commit to the Retro UI Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughSideNav now accepts an optional setIsOpen prop. HamburgerMenu passes its isOpen state setter to SideNav. SideNav invokes setIsOpen(false) on link clicks to close the menu. Minor JSX restructuring and formatting were applied without other behavioral changes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant HM as HamburgerMenu
participant SN as SideNav
participant State as isOpen State
Note over HM: Renders SideNav with prop setIsOpen
HM->>SN: render SideNav({ setIsOpen })
User->>SN: Click nav Link
alt setIsOpen provided
SN->>State: setIsOpen(false)
State-->>HM: isOpen = false
HM-->>User: Menu closes
else no setIsOpen
SN-->>User: Navigate (menu state unchanged)
end
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
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: 0
🧹 Nitpick comments (1)
components/SideNav.tsx (1)
3-13: Type setIsOpen as React’s Dispatch for future-proofing functional updates.Wiring the sidebar’s close handler through a prop looks good, but the current
(isOpen: boolean) => voidsignature is narrower than theuseStatesetter you’re passing fromHamburgerMenu. Typing it asDispatch<SetStateAction<boolean>>keeps it compatible with functional updates (e.g.,setIsOpen?.((prev) => !prev)), which React may need down the line.+import type { Dispatch, SetStateAction } from "react"; import { navConfig } from "@/config/navigation"; import { Badge, Text } from "@/components/retroui"; import Link from "next/link"; import { usePathname } from "next/navigation"; interface SideNavProps { - setIsOpen?: (isOpen: boolean) => void; + setIsOpen?: Dispatch<SetStateAction<boolean>>; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/HamburgerMenu.tsx(1 hunks)components/SideNav.tsx(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
components/HamburgerMenu.tsx (1)
components/SideNav.tsx (1)
SideNav(12-53)
components/SideNav.tsx (1)
components/retroui/Badge.tsx (1)
Badge(29-44)
🔇 Additional comments (1)
components/HamburgerMenu.tsx (1)
33-36: LGTM: Propagating the setter keeps the menu state in one place.Passing the
setIsOpendispatch straight through lets SideNav close the drawer without duplicating state. Nicely scoped change.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Thank you for the contribution 🙏 |
sidebar should close on navigation when using smartphones
Summary by CodeRabbit