Convert core toolbar buttons into ToolbarButton

[diegohaz]

Context

This PR is part of #18619, whose main goal is to implement roving tabindex on the @wordpress/components' Toolbar component and to use it on the block toolbars so they become a single tab stop as recommended by the WAI-ARIA Toolbar Pattern. Related issues are #15331 and #3383.

This PR improves NavigableToolbar and converts the core toolbar buttons (those buttons that appear on all blocks: movers, block switcher and more options) into ToolbarButton or ToolbarItem.

ToolbarItem is the generic headless component that is used by ToolbarButton underneath and can be used to make other components, such as DropdownMenu, aware of the context state provided by the parent Toolbar component, which enables the roving tabindex method.

This also converts a few other buttons so the block toolbar on rich text blocks and other basic blocks already works with roving tabindex.

Toolbar vs. ToolbarGroup

Or .components-toolbar vs. .components-toolbar-group

Most of the changes on this PR are related to this. On #18534, Toolbar was renamed to ToolbarGroup, but the old class name (components-toolbar) was kept as there's a lot of code relying on it.

However, when ToolbarGroup is used within the accessible Toolbar component, the class name will be components-toolbar-group. That's why it appears duplicated in many parts.

NavigableToolbar

NavigableToolbar is the component that is currently used by block toolbars and by the top toolbar. It handles focus when the user presses Alt+F10.

With this PR, if all the tabbable DOM elements inside NavigableToolbar are rendered by ToolbarButton (or ToolbarItem), then NavigableToolbar will render Toolbar instead of NavigableMenu. If there's any other tabbable element, it'll work just like it does right now on master.

gutenberg/packages/block-editor/src/components/navigable-toolbar/index.js

Lines 80 to 101 in 70937da

	if ( isAccessibleToolbar ) {
	return (
	<Toolbar
	__experimentalAccessibilityLabel={ props[ 'aria-label' ] }
	ref={ wrapper }
	{ ...props }
	>
	{ children }
	</Toolbar>
	);
	}
	return (
	<NavigableMenu
	orientation="horizontal"
	role="toolbar"
	ref={ wrapper }
	{ ...props }
	>
	{ children }
	</NavigableMenu>
	);

This is so we can gradually migrate to this approach, and not break blocks that aren't using ToolbarButton for their toolbar buttons, like plugin custom blocks or other blocks like Image (when there's an image). They still work with the Tab key.

In short, this code would render NavigableMenu because, even though the first item is a ToolbarButton, the others are Button, button and DropdownMenu, which aren't aware of ToolbarContext and, therefore, can't join the roving tabindex party:

<NavigableToolbar aria-label="Block contextual toolbar">
  <ToolbarButton />
  <Button />
  <button />
  <DropdownMenu />
</NavigableToolbar>

The code below would render Toolbar with roving tabindex enabled:

<NavigableToolbar aria-label="Block contextual toolbar">
  <ToolbarButton />
  <ToolbarItem>
    { ( itemProps ) => ( <Button { ...itemProps } /> ) }
  </ToolbarItem>
  <ToolbarItem>
    { ( itemProps ) => ( <button { ...itemProps } /> ) }
  </ToolbarItem>
  <ToolbarItem>
    { ( itemProps ) => ( <DropdownMenu toggleProps={ itemProps } /> ) }
  </ToolbarItem>
</NavigableToolbar>

How to test

• Check rich text blocks and other basic blocks are working with roving tabindex.
• Check if blocks that contain non-ToolbarItem buttons are working just like today on master.
• Check if block toolbars are correctly styled, including nested blocks with horizontal movers.
• The top toolbar should work just like on master as the other buttons there haven't been converted yet.

What's next

• Convert the Top Toolbar buttons.
• Convert toolbar buttons on other blocks.

[diegohaz]

@gziolo

• <Toolbar __experimentalAccessibilityLabel="Block toolbar" /> changes the toolbar to behave according to WAI-ARIA specs (fixes also this issue from a11y audit: Toolbar elements are tabbable #15331), otherwise it’s just a proxy to ToolbarGroup to make it backward compatible
• <ToolbarGroup /> is used for grouping items, it also turns into dropdown when the flag for collapsing is enabled
• <ToolbarItem /> and <ToolbarButton /> need to be used for individual items, or the whole toolbar will fallback to the incorrect legacy behavior for toolbars

Exactly that! 👍

I'm going to add a comment on Toolbar to avoid confusion for people looking at the code before the old behavior is deprecated.

[enriquesanchez]

Hi @diegohaz!

To clarify, this is not yet implemented in all blocks, correct? I'm seeing weird behavior with blocks such as columns and group.

[diegohaz]

@enriquesanchez That's correct! It should work for columns though. Could you elaborate on how it’s weird? I just did a quick test and didn’t find the problem.

[gziolo]

I remember having to do the same trick in the past for other fills. It's a bit unfortunate that we need this trick but as long it works it feels like a good trade-off.

[gziolo]

Once it becomes an official prop, we should refactor code to stop using aria-label directly and convert it to pass-through with the props object.

[gziolo]

Aside: So much legacy here :( Ideally, we should be promoting label and isPressed to make it easier for developers to use API. At the same time, disabled in the Button should be really isDisabled 😃
Nothing actionable for now, I just wanted to complain a bit 🙃

[diegohaz]

I think we should eventually deprecate those props and make the API the same as on Button. I chose to add those props here because there is already a ton of code using ToolbarButton with them, and the migration would be more painful.

[gziolo]

Agreed 💯

[gziolo]

Awesome work Diego. In my testing, only Block Toolbar uses new behavior when it isn't docked in the header. I'm looking forward to seeing the rest of the toolbars converted 🙌

There was the following comment from alexstine on WordPress Slack in #accessibility channel:

Testing in Firefox with NV Access.

1. Can tab in to the toolbar.
2. Can use arrow navigation to move through the toolbar.
3. Tab will move to each toolbar element.

(3) seems concerning, but I should be more clear and let everyone know that this new behavior applies only to the floating toolbar.

My final recommendation would be to replace the div that wraps the toolbar with a section element. This should allow you to jump by landmark to the Document tools section and then here it announced in some screen readers when you leave that section and land on the "Publish" button.
I've not mocked up the code so I cannot say how it would work exactly, but I think it would be worth a try to add a bit more support from jumping to and from that Document tools section.

Sharing also the above for visibility, I don't think it's something we should include in this PR.

[enriquesanchez]

@diegohaz Oh, I apologize! There was confusion on my part. I just tested again and works as expected.

[diegohaz]

@enriquesanchez No worries! Thanks for testing this! ❤️

[SergioEstevao]

Just circling back to this PR, while we appreciate the change that was done, if a .native file is being deleted it's a good indicator that this code is being used in GB-mobile too.

It would be nice in the future to ping the GB-mobile team just to make sure that the change is ok for the GB-mobile side of things.

[diegohaz]

@SergioEstevao Thanks for the heads up! Created #22229 to re-add that file.