WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
10.52k stars 4.21k forks source link

Components: reconcile `ItemGroup`/`Item` with `MenuGroup`/`MenuItem` #35210

Open ciampo opened 3 years ago

ciampo commented 3 years ago

What

This conversation was started in https://github.com/WordPress/gutenberg/pull/35142#discussion_r717409093

We should look into refactoring MenuGroup and MenuItem to rely on the lower level ItemGroup and Item. This would enable the usage of MenuGroup and MenuItem outside of DropdownMenu and would give us a chance to enforce a more consistent look&feel (including spacing) across menu layouts.

Why

The MenuGroup and MenuItem components were created as high-level components, tailored to be used inside the DropdownMenu component. As such, they are quite opinionated in terms of:

On the other hand, the recently introduced ItemGroup and Item components offer a more low-level approach, are much less opinionated and rely on composition with other components. Their low-level approach can require a lot of tweaking when used directly in complex UIs, and therefore can easily lead to inconsistencies when used in different parts of the editor.

That's why we should look into uniforming the look&feel of menus across different UI patterns (dropdown menus, navigation menus, modal menus, sidebar menus...) and consider rewriting MenuGroup and MenuItem using ItemGroup and Item, with the intention of unlocking their usage in more situations outside of DropdownMenu.

A/C

youknowriad commented 3 years ago

We should also consider semantics/a11y. What makes most sense for these components. Should we have a new Menu wrapper (role="menu"). Should the low level components stay without any built-in semantics...

ciampo commented 3 years ago

Currently, ItemGroup has a default role="list" and Item has a default role="listitem", which I believe are good defaults — although they can be both easily overridden.

We could also use the recently introduced Context system to suggest the correct roles (in case they are not explicitly set) — e.g., DropdownMenu could use the Context System to suggest to its child ItemGroup and Item components to have role="group" and role="menuitem".