fix(navbar): wire MobileNavMenu onClose - #302
Conversation
|
@ryzen-xp is attempting to deploy a commit to the Threadflow Team on Vercel. A member of the Team first needs to authorize it. |
|
@ryzen-xp Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR adds a close button to the mobile navbar menu, removes an unused ESLint suppression, and keeps nearby prop/type formatting changes. It also reformats the my-claims Jest test without changing its assertions. ChangesNavbar close control
My-claims test cleanup
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
Benjtalkshow
left a comment
There was a problem hiding this comment.
Thanks, CI is green and the eslint disable is properly removed. One visual
bug: ml-auto on a flex-column child does nothing (it only pushes right in
flex-row), so the close button will render at the left edge instead of the
top-right — please change ml-auto to self-end, or wrap the button in
<div className="flex w-full justify-end">. Also please limit the
reformatting in Navbar, NavbarButton, and my-claims.test.ts to lines you're
actually changing (the @jest/globals import removal is fine, the rest is
Prettier-on-save churn that inflates the diff). Finally, attach a
screenshot of the open mobile menu showing the close button position so I
can verify the fix visually before merging.
|
working on !!! |
1 similar comment
|
working on !!! |
Revert the 4-space to 2-space reformatting in my-claims.test.ts and the Navbar/NavbarButton indentation changes in resizable-navbar.tsx. Only the close button wiring and the eslint-disable removal belong in this PR.


Summary
Wire up the existing
onCloseprop onMobileNavMenuby adding a close button inside the mobile menu, and remove the inline eslint suppression. This keeps the component contract explicit and gives users a direct way to dismiss the menu.Changes
MobileNavMenuthat callsonClose()@typescript-eslint/no-unused-varsdisable fromcomponents/ui/resizable-navbar.tsx@jest/globalsimport incomponents/reputation/__tests__/my-claims.test.tsso TypeScript can pass in this workspaceSummary by CodeRabbit