-
Notifications
You must be signed in to change notification settings - Fork 144
SideBar enhancement for Dashboard #191
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?
SideBar enhancement for Dashboard #191
Conversation
|
@abhijit-pr 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. |
WalkthroughThe dashboard layout component was refactored to conditionally render an overlay panel or inline header based on a Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Layout as Dashboard Layout
participant Overlay as Overlay Container
participant Header as Header Component
rect rgb(220, 240, 255)
Note over User,Header: showSidebar = true
User->>Header: View dashboard (sidebar visible)
Header->>Overlay: Render fixed overlay (xl:hidden)
Overlay->>Overlay: Mount header + navigation
Header->>User: Display Bars3Icon (read-only)
User->>Overlay: Click overlay
Overlay->>Layout: setShowSidebar(false)
end
rect rgb(240, 220, 255)
Note over User,Header: showSidebar = false
Layout->>Header: Render inline header
Header->>User: Display interactive Bars3Icon
User->>Header: Click Bars3Icon
Header->>Layout: setShowSidebar(true)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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 |
|
|
1 similar 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 (2)
apps/web/src/app/(main)/dashboard/layout.tsx (2)
31-53: Overlay click handler closes sidebar on any click inside mobile contentBecause
onClickis attached to the full‑screen wrapper, any click inside the header or{children}(not just the backdrop area around the sidebar) will also callsetShowSidebar(false). If the intent is to close only when the backdrop outside the sidebar is clicked, consider guarding on the event target:- {showSidebar && ( - <div - onClick={() => setShowSidebar(false)} - className="fixed inset-0 xl:hidden" - > + {showSidebar && ( + <div + onClick={(e) => { + if (e.target === e.currentTarget) { + setShowSidebar(false); + } + }} + className="fixed inset-0 xl:hidden" + >Also note that
IconWrapperhere still hascursor-pointerbut no explicitonClick, so the burger visually looks clickable yet relies on the parent’s handler to close the sidebar. If you want the icon to behave as an explicit close affordance, wiringonClick={ () => setShowSidebar(false) }directly on theIconWrapperin this branch (and optionally removing the parent handler) would make the UX clearer.
55-69: Duplicate mobile header/content layout between sidebar-open and -closed statesThe header +
<main>structure for the mobile layout is effectively duplicated in both{showSidebar && ...}and{!showSidebar && ...}branches, differing mainly by theIconWrapper’sonClick. This makes future changes to the header (labels, styles, etc.) easy to forget in one branch.Consider extracting a small mobile header component that takes an optional
onMenuClick:const MobileHeader = ({ onMenuClick }: { onMenuClick?: () => void }) => ( <div className="xl:hidden flex items-center h-16 px-4 border-b border-ox-header bg-ox-content"> <IconWrapper onClick={onMenuClick}> <Bars3Icon className="size-5 text-ox-purple" /> </IconWrapper> <Link href="/" className="ml-4 text-lg font-semibold text-ox-white hover:text-ox-purple transition-colors" > Opensox </Link> </div> );Then reuse it in both branches, passing
onMenuClick={() => setShowSidebar(true)}only in the!showSidebarcase. This keeps behavior identical while reducing duplication.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/src/app/(main)/dashboard/layout.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/web/src/app/(main)/dashboard/layout.tsx (1)
apps/web/src/components/ui/IconWrapper.tsx (1)
IconWrapper(10-22)
Optimized the Dashboard sidebar so it closes when clicking anywhere outside, instead of relying solely on the close button.
Opensox.SideBar.mp4
Summary by CodeRabbit