Show affordance for closing nav menu on mobile #790
Conversation
WalkthroughThe modifications update the mobile navigation header. In the Changes
Sequence Diagram(s)sequenceDiagram
participant U as Mobile User
participant H as Header Component
participant D as Drawer Component
U->>H: Tap mobile navigation
H->>D: Open Drawer with updated ModalProps (transparent backdrop)
D->>U: Render Drawer with marginTop style applied
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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 (1)
ui/src/components/Header.tsx (1)
128-128: Consider using theme spacing or dynamic values instead of hardcoded margin.While the hardcoded margin achieves the desired layout, it might cause issues with different screen sizes or future header height changes. Consider these alternatives:
- marginTop: "60px", + marginTop: (theme) => theme.spacing(7.5), // Or adjust the multiplier as needed // OR + marginTop: (theme) => `${theme.mixins.toolbar.minHeight}px`,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ui/src/components/Header.tsx(2 hunks)
🔇 Additional comments (2)
ui/src/components/Header.tsx (2)
118-120: LGTM! Clean implementation of transparent backdrop.The transparent backdrop implementation aligns well with the PR objectives and uses the proper Material-UI API.
152-152: LGTM! Fixed typo in display property.The correction improves code quality by fixing the display property typo.
Saw that there's a PR on this since before, but I implemented a slightly different solution. Someone had already prepared for a toggle between hamburger and close icon, and I personally didn't love how the logo swiped in with the rest of the full screen menu from the left (or the logo being at two places in the code). Here I've removed the logo from the full screen menu and added a margin so that the OG logo is displayed regardless of full screen mobile menu or not, and removed drawer backdrop. Downside is hardcoded top margin, upside is neater code and better user experience (imo that is :)).
Please tell me if you want me to modify anything, and also if this should hade stage as base rather than main.
Solve #279
Summary by CodeRabbit