-
Notifications
You must be signed in to change notification settings - Fork 0
add hamburger menu to navbar for small screens #406
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
✅ Deploy Preview for cell-catalog ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
src/component-queries/NavBar.tsx
Outdated
| // crossing the breakpoint. | ||
| useEffect(() => { | ||
| const handleResize = () => { | ||
| const isMobile = window.innerWidth <= 768; |
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.
this should use the breakpoint constant.
Also, 768 is more standard, but the designs travis made have the cuttoff at 744. I don't have strong opinions but it should be consistent
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.
there is also also already a useWindowWidth hook with a resize listener, did that not work for this?
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.
Thanks for pointing that out. I forgot about it. I just added a little wrapper for that in this PR that i think is an improvement... If you want to review that first and I can bring that change in here?
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.
Pull request overview
This PR adds a responsive hamburger menu to the navbar that appears on small screens (≤768px). The changes enable a mobile-friendly navigation experience by hiding the standard dropdown buttons and displaying a drawer-based menu instead.
Changes:
- Added a new
HamburgerMenucomponent that renders a drawer with organized sections for Catalogs, Protocols, and Collections - Implemented responsive CSS to toggle between standard dropdowns and hamburger menu based on screen width
- Added theme styling for the new Drawer and Menu components
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/components/HamburgerMenu/HamburgerMenu.tsx | New component implementing the hamburger menu with drawer UI |
| src/style/hamburgerMenu.module.css | Styles for the hamburger button and drawer content |
| src/style/navbar.module.css | Media query logic to show/hide hamburger menu and dropdowns |
| src/style/theme.ts | Theme configuration for Drawer and Menu components |
| src/component-queries/NavBar.tsx | Integration of hamburger menu with resize handling logic |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/component-queries/NavBar.tsx
Outdated
| // crossing the breakpoint. | ||
| useEffect(() => { | ||
| const handleResize = () => { | ||
| const isMobile = window.innerWidth <= 768; |
Copilot
AI
Jan 10, 2026
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.
The breakpoint value 768 is hardcoded here and also in navbar.module.css line 60. Consider extracting this to a shared constant to ensure consistency and ease of maintenance.
src/component-queries/NavBar.tsx
Outdated
| const handleResize = () => { | ||
| const isMobile = window.innerWidth <= 768; | ||
| if (isMobile) { | ||
| setDropdownKey((prev) => prev + 1); | ||
| } | ||
| }; |
Copilot
AI
Jan 10, 2026
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.
The resize event listener fires continuously during window resizing. Consider debouncing this handler to avoid excessive state updates and re-renders.
| open={open} | ||
| width={300} | ||
| className={drawerContainer} | ||
| destroyOnHidden |
Copilot
AI
Jan 10, 2026
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.
The prop name 'destroyOnHidden' appears incorrect. The Ant Design Drawer component uses 'destroyOnClose' to control whether to destroy child elements when the drawer is closed.
| destroyOnHidden | |
| destroyOnClose |
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.
That is wrong.
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.

Problem
Closes #404
Solution
Can improve/modify this in conversation with UX but this is a functional patch for the navbar below a mobile breakpoint
Screenshots (optional):
Keyfiles (delete if not relevant):
Thanks for contributing!