Skip to content
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

Refactor Export menu with Floating UI #1642

Merged
merged 2 commits into from
May 16, 2024
Merged

Refactor Export menu with Floating UI #1642

merged 2 commits into from
May 16, 2024

Conversation

axelboc
Copy link
Contributor

@axelboc axelboc commented May 16, 2024

Supersedes #1567

Since Ariakit and React Aria were unsatisfactory options with regard to bundle size, I decided to try a more specialised solution: Floating UI (a full rebuild of popper.js and react-popper). It's really well designed; all based on hooks (along with helper components that I didn't end up using).

The two main hooks I use are useFloating, which provides the refs and the popover's positioning logic, and useListNavigation, which provides the arrow up/down navigation (including opening the menu on arrow down, looping, focus management, etc.) This weighs roughly 30 kB, which is way more reasonable than the other two headless UI libraries I tried...

At first I was also using useDismiss, to close the menu on Escape and click outside, and <FloatingFocusManager /> to close the menu when the focus leaves the menu. But both of those added an extra 16 kB. Since we already use useClickOutside and useKeyboardListener from react-hookz, and since <FloatingFocusManager /> has a lot of features we don't need (e.g. focus trap for modals), I decided to reimplement the same behaviours myself. As I said, Floating UI is very well designed, and this was super easy to do.

All in all, I'm very happy with the result at last, so I will continue the migration from react-aria-menubutton in separate PRs by refactoring other components like Selector.

@axelboc
Copy link
Contributor Author

axelboc commented May 16, 2024

/approve

@axelboc axelboc requested a review from loichuder May 16, 2024 09:16
@axelboc axelboc removed the request for review from loichuder May 16, 2024 09:20
@axelboc axelboc marked this pull request as draft May 16, 2024 09:20
@axelboc
Copy link
Contributor Author

axelboc commented May 16, 2024

Missed a bug: the menu doesn't shift to stay within the bounds of the body as I thought. I need to add another hook or middleware. BRB

EDIT: yep it was the shift middleware. Works like a charm:

image

I've also added a custom middleware to make sure the menu is always at least as wide as the trigger:

image

@axelboc axelboc force-pushed the floating-ui branch 2 times, most recently from 6c757ab to 0c08ab5 Compare May 16, 2024 09:49
@axelboc axelboc marked this pull request as ready for review May 16, 2024 09:52
@axelboc axelboc requested a review from loichuder May 16, 2024 09:52
@axelboc
Copy link
Contributor Author

axelboc commented May 16, 2024

Now with all the required ARIA props ... sorry for the spam. Again, to save bundle size, I didn't use useRole but only replicated it's logic for role="menu". I also just find it useful to make the ARIA attributes explicit in the source code, since they are relied upon for styling.

@axelboc axelboc merged commit 77025c4 into main May 16, 2024
8 checks passed
@axelboc axelboc deleted the floating-ui branch May 16, 2024 11:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants