search

silx-kit / h5web

React components for data visualization and exploration
https://h5web.panosc.eu/
MIT License
161 stars 17 forks source link

Refactor Export menu with Floating UI #1642

Closed axelboc closed 1 month ago

axelboc commented 1 month ago

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 commented 1 month ago

/approve

axelboc commented 1 month ago

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 commented 1 month ago

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.