Closed vitokhangnguyen closed 2 years ago
This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.
🔍 Inspect: https://vercel.com/andrewnt219/rent-near-me/7Lu4HbZikk22MDQnrMrQLg595Q8U
✅ Preview: https://rent-near-me-git-vitokhangnguyen-issue78menu-andrewnt219.vercel.app
@Andrewnt219 comments added. Ready for review
a11y is a real pain huh.
There's a lib for that, ppl said... It's gonna be easy ppl said...
What do you do if the lib doesn't support your use case or, god forbid, has a bug? :)
Resolves #78
Adding comments, feel free to revierw the implementation first (shouldn't be very hectic)Changelog:
@reach/menu-button
Menu
componentUserMenu
to use the newMenu
moduleuseDropdownMenu
hook).useClickOutside
hook is worth keeping but is modified to receiveref
instead of returning a newref
because in some cases, we might want to pass aref
into many hooks (e.g.: we want to applyuseClickOutside
anduseOverflow
on the sameref
). Therefore, this should be the way to design hook that works with native elements in the future.RouteProps
object was a redundant abstraction, embedding links into theUserMenu
is easily understandable. This helps when we mismatch links and buttons in the menu as well.Notes:
UserMenu
currently renders 2 separated lists of items because of a potential bug in Reach UI that throws off the focus order of items inside if an item is rendered asynchronously https://github.com/reach/reach-ui/issues/876