feat(web): dismiss mobile drawer when clicking outside #2133
Conversation
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
✅ Deploy Preview for hyprnote-storybook ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for hyprnote ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughReplaced the outer drawer container with a fragment and added a conditionally rendered backdrop behind MobileDocsDrawer and MobileHandbookDrawer that calls onClose when clicked; moved drawer inner content into a dedicated scroll wrapper while preserving SidebarNavigation rendering and animations. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Possibly related PRs
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: 0
🧹 Nitpick comments (2)
apps/web/src/routes/_view/route.tsx (2)
92-120: Backdrop and scroll container wiring look solid; consider minor cleanup.The backdrop and drawer container structure achieve the desired behavior: click-outside closes the drawer, the z-index layering is correct (backdrop under drawer), and the scroll container +
scrollContainerRefintegration withSidebarNavigationpreserves the existing auto-scroll behavior.Two small, optional refinements you might consider:
- Use
cnfor the long conditional className
To align with the codebase guideline and improve readability of the conditionaltranslate-xclasses:+import { cn } from "@hypr/utils"; ... - <div - className={`fixed top-[69px] left-0 h-[calc(100vh-69px)] w-72 bg-white/80 backdrop-blur-sm border-r border-neutral-100 shadow-2xl shadow-neutral-900/20 z-50 md:hidden transition-transform duration-300 ease-in-out ${ - isOpen ? "translate-x-0" : "-translate-x-full" - }`} + <div + className={cn( + "fixed top-[69px] left-0 h-[calc(100vh-69px)] w-72 bg-white/80 backdrop-blur-sm border-r border-neutral-100 shadow-2xl shadow-neutral-900/20 z-50 md:hidden transition-transform duration-300 ease-in-out", + isOpen ? "translate-x-0" : "-translate-x-full", + )}
- Optional a11y nit for the backdrop
If you want to be explicit for assistive tech, you could mark the backdrop as presentational:- {isOpen && ( - <div - className="fixed inset-0 top-[69px] bg-black/20 z-40 md:hidden" - onClick={onClose} - /> - )} + {isOpen && ( + <div + className="fixed inset-0 top-[69px] bg-black/20 z-40 md:hidden" + onClick={onClose} + aria-hidden="true" + /> + )}These are non-blocking; behavior is already correct.
179-207: Handbook drawer implementation mirrors docs drawer; consider deduping the shared shell.The handbook drawer’s backdrop + container + scroll wrapper mirror the docs drawer and correctly hook up
SidebarNavigationwith its ownscrollContainerRef. Functionally this looks good and matches the intended behavior.Given how similar the two drawer shells now are, you might reduce duplication by extracting a small shared mobile drawer shell component that takes
isOpen,onClose, andchildren(or anavigationprop), e.g.:function MobileDrawerShell({ isOpen, onClose, children, }: { isOpen: boolean; onClose: () => void; children: React.ReactNode; }) { return ( <> {isOpen && ( <div className="fixed inset-0 top-[69px] bg-black/20 z-40 md:hidden" onClick={onClose} aria-hidden="true" /> )} <div className={cn( "fixed top-[69px] left-0 h-[calc(100vh-69px)] w-72 bg-white/80 backdrop-blur-sm border-r border-neutral-100 shadow-2xl shadow-neutral-900/20 z-50 md:hidden transition-transform duration-300 ease-in-out", isOpen ? "translate-x-0" : "-translate-x-full", )} style={{ paddingLeft: "env(safe-area-inset-left)" }} > {children} </div> </> ); }Then both
MobileDocsDrawerandMobileHandbookDrawercan just supply their respectiveSidebarNavigationcontent inside this shell. Not required for correctness, but it will make future tweaks to the drawer behavior/layout easier to apply in one place.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/src/routes/_view/route.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid creating a bunch of types/interfaces if they are not shared. Especially for function props, just inline them instead.
Never do manual state management for form/mutation. Use useForm (from tanstack-form) and useQuery/useMutation (from tanstack-query) instead for 99% of cases. Avoid patterns like setError.
If there are many classNames with conditional logic, usecn(import from@hypr/utils). It is similar toclsx. Always pass an array and split by logical grouping.
Usemotion/reactinstead offramer-motion.
Files:
apps/web/src/routes/_view/route.tsx
🧬 Code graph analysis (1)
apps/web/src/routes/_view/route.tsx (1)
apps/web/src/components/sidebar-navigation.tsx (1)
SidebarNavigation(4-71)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Redirect rules - hyprnote
- GitHub Check: Header rules - hyprnote
- GitHub Check: Pages changed - hyprnote
- GitHub Check: fmt
- GitHub Check: ci
- GitHub Check: Devin
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
apps/web/src/routes/_view/route.tsx (2)
107-118: Optional: Consider simplifying the DOM structure.The additional inner wrapper div adds an extra DOM node. The
refand scroll behavior could potentially be applied directly to the drawer container (lines 99-106) by moving the scroll-related classes there.However, if the separation is intentional for layout/styling purposes, the current structure is acceptable.
74-122: Recommended: Extract shared drawer logic.Both
MobileDocsDrawerandMobileHandbookDrawerhave nearly identical backdrop and drawer structure. Consider extracting a sharedMobileDrawercomponent that accepts the content as children to reduce duplication and improve maintainability.Example structure:
function MobileDrawer({ isOpen, onClose, ariaLabel, children }: { isOpen: boolean; onClose: () => void; ariaLabel: string; children: React.ReactNode; }) { // Shared backdrop + drawer structure }This would eliminate ~40 lines of duplicated code.
Also applies to: 124-209
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/src/routes/_view/route.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid creating a bunch of types/interfaces if they are not shared. Especially for function props, just inline them instead.
Never do manual state management for form/mutation. Use useForm (from tanstack-form) and useQuery/useMutation (from tanstack-query) instead for 99% of cases. Avoid patterns like setError.
If there are many classNames with conditional logic, usecn(import from@hypr/utils). It is similar toclsx. Always pass an array and split by logical grouping.
Usemotion/reactinstead offramer-motion.
Files:
apps/web/src/routes/_view/route.tsx
🧬 Code graph analysis (1)
apps/web/src/routes/_view/route.tsx (1)
apps/web/src/components/sidebar-navigation.tsx (1)
SidebarNavigation(4-71)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Redirect rules - hyprnote
- GitHub Check: Header rules - hyprnote
- GitHub Check: Pages changed - hyprnote
- GitHub Check: fmt
- GitHub Check: ci
- GitHub Check: Devin
ComputelessComputer
deleted the
devin/1764919728-mobile-drawer-dismiss
branch
Summary
Adds click-outside-to-dismiss behavior for the mobile docs and company handbook navigation drawers. When the drawer is open, a transparent backdrop overlay appears behind it. Clicking on this backdrop dismisses the drawer.
The implementation adds an invisible overlay at z-40 (below the drawer at z-50) that covers the viewport below the header. Both
MobileDocsDrawerandMobileHandbookDrawercomponents receive the same treatment.Updates since last revision
bg-black/20) per user request - the overlay is now invisible but still captures clicks to dismiss the drawerReview & Testing Checklist for Human
/docs/*and/company-handbook/*pagesRecommended test plan: Open the site on a mobile viewport, navigate to
/docsor/company-handbook, open the sidebar drawer via the hamburger menu, then tap anywhere outside the drawer panel to verify it closes.Notes