Components: Make Toolbar accessible

[diegohaz]

Description

Fixes #15331.
Fixes #3383.
Related to #16514.

This PR fixes Toolbar transforming it into an actual toolbar, addressing the accessibility concerns discussed on #15331, and leveraging the design discussions on #16514.

I tried to touch the API as little as possible. But we have a few changes, all backward compatible:

1. Current Toolbar has been renamed to ToolbarGroup.
2. New Toolbar needs an accessibilityLabel prop.
3. New Toolbar requires that its buttons are rendered using @wordpress/components.

1. Current Toolbar has been renamed to ToolbarGroup

The current Toolbar on master still works, but now it will emit a deprecation warning with a suggestion to use ToolbarGroup instead, which name better describes its semantics.

ToolbarGroup has the same implementation as the current Toolbar on master. In fact, most of the code was copied and pasted to the new location.

import { ToolbarGroup } from '@wordpress/components';

<ToolbarGroup>
  ...
</ToolbarGroup>

// or
<ToolbarGroup controls={...} />

// or
<ToolbarGroup isCollapsed controls={...} />

Just like today, <ToolbarGroup isCollapsed /> only accepts controls, not children.

2. New Toolbar needs an accessibilityLabel prop.

To use the new Toolbar, it's necessary to pass an accessibilityLabel prop to the component. Other than that, the API remains the same:

import { Toolbar, ToolbarButton, ToolbarGroup } from '@wordpress/components';

<Toolbar accessibilityLabel="Block options">
  <ToolbarButton icon="thumbs-up" title="Thumbs Up" onClick={...} />
  <ToolbarButton icon="thumbs-down" title="Thumbs Down" onClick={...} />
  <ToolbarGroup>
    <ToolbarButton ... />
    <ToolbarButton ... />
  </ToolbarGroup>
   ...
</Toolbar>

When using Toolbar without accessibilityLabel, the component will fallback to ToolbarGroup and emit a warning.

3. New Toolbar requires that its buttons are rendered using @wordpress/components.

Like Button, IconButton, ToolbarButton etc.

Ideally, only ToolbarButton would be accepted, and it would be flexible enough to render all kinds of toolbar items. Unfortunately, that's not the case for ToolbarButton, and even Gutenberg uses different components as toolbar items to achieve different results.

This means that block authors that aren't using @wordpress/components for toolbar items will see a warning.

import {
  BlockControls,
  Button,
  IconButton,
  ToolbarButton,
  DropdownMenu,
} from '@wordpress/components';

// still works, but will fallback to the old navigable menu
// less accessible, but without breaking changes
<BlockControls>
  <button />
  <button />
  <button />
  <button />
</BlockControls>

// after
<BlockControls>
  <Button />
  <IconButton />
  <ToolbarButton />
  <DropdownMenu />
</BlockControls>

Other stuff

How is this different from NavigableToolbar

NavigableToolbar finds all the tabbable elements within the toolbar whenever the user presses left and arrow keys. It determines which one is active and focuses the previous/next one. To conform with the WAI-ARIA recommendations, it still needs to:

• set tabIndex="-1" on inactive items so the toolbar has only one tab stop.
• keep record of the last active item so focus will move back to it the user tabs into the toolbar.
• listen to PageUp PageDown Home End keys.

The new Toolbar solves all these issues. Also, it registers Buttons when they're mounted and unregisters them when they're unmounted. Because of that, it doesn't need to run a query on the DOM every time an arrow key is pressed, which leads to a better performance.

Reakit

This PR introduces a new dependency: Reakit. It's an open source library that provides low level components and hooks to address common accessibility aspects of design systems. It has 1000s of lines of tests to make sure accessible behaviors work. It's being used by projects like CodeSandbox and Gatsby. And is used here to add the roving tabindex method to Toolbar.

reakit/Toolbar adds around 3.9 kB gzipped to the bundle. My plan is to address higher-level cases like Toolbar, Popover or whatever is more urgent first. I want to see how well Reakit fits into the current @wordpress/components implementation while trying to touch the components' API as little as possible.

Once we have a few high-level components working with proper accessibility and less dependent of @wordpress/components' abstractions, we can go deeper and determine what to do with things like NavigableContainer.

I think that's the path that will give us more flexibility to change things (even removing Reakit later, if needed) at the same time we provide more accessible ready-to-use components faster.

Next steps

As I said, I tried to touch the Toolbar API as little as possible. But this should be improved. ToolbarButton must be more flexible and work for more cases besides only IconButton. So, next steps would be improving Toolbar API. Some discussions are happening on #16514.

How has this been tested?

Checked the header toolbar and the block toolbar of many (if not all) blocks. Many end-to-end tests failed on the first iterations, so I went through each of them and fixed the issues.

Screenshots

Storybook

Playground

Types of changes

New feature

Checklist:

• My code is tested.
• My code follows the WordPress code style.
• My code follows the accessibility standards.
• My code has proper inline documentation.
• I've included developer documentation if appropriate.

[diegohaz]

I see this when setting the full width for the Table block when in the mobile viewport, but this might be present in master

Checked it and it was introduced by this PR. Working on a fix.

Nevermind, looks like it's on master indeed. Trying to find the commit that changed this.

Found it: #17877 (comment)

[diegohaz]

Can you summarize in the comment what's left so we could decide if there any remaining blockers for this PR?

Marked the comments that were addressed as resolved. The ones missing are generally discussions.

Future tasks I can think of:

• Improve ToolbarButton so it can be used for more cases
• Refactor SlotFill

[jasmussen]

I see this when setting the full width for the Table block when in the mobile viewport, but this might be present in master:

Hey, Diego is right, #17877 is responsible for this. I created #18469 to fix it.

[ItsJonQ]

@diegohaz Amazing job with this PR!

I tested this locally in Gutenberg with keyboard + voiceover. It works as expected. There were a couple of hiccups, but those interactions already exist in Gutenberg today (unfortunately. We'll make it better!).

Based on the convo with @gziolo , I think a great fast follow PR would be to improve the SlotFIll related forceUpdate work around.

Thank you so much for all of your hard work.
Double thank you for your efforts and advocacy for Reakit and a11y!

Very excited to see how we can systematically improve the experience of Gutenberg as we start refactoring with Reakit within the components.

❤️

[youknowriad]

It's not clear to me yet how this impacts our existing public APIs and what's the potential of breakage of existing usage. Anyone able to write a summary?

[diegohaz]

@youknowriad I wrote a summary here: #17875 (comment)

It shouldn't break the current usage unless they're doing things within <BlockControls> that diverge a lot from what Gutenberg does with built-in blocks.

[youknowriad]

To be completely honest, I don't feel confident merging this PR as is. My concerns are:

1- Maybe it's just me but I find it too big, so hard to understand. I wonder if we can split it to increase confidence in the changes we're making.
2- Things like that:

It shouldn't break the current usage unless they're doing things within that diverge a lot from what Gutenberg does with built-in blocks.

This statement is a little bit unprecise to me. We can expect that plugin authors do all kind of weird things and Backwards compatibility is really important for WordPress, we can't break existing APIs.

Some exceptions are possible when we know exactly what we're breaking, how we're breaking it and if the breakage is small enough. In that case, we can write a dev note for plugin authors to let them know how they should update their code to avoid breakage.

[diegohaz]

@youknowriad
I think this PR could be simplified if all the new features are made experimental and no deprecation is made. Gutenberg itself wouldn't change (it would still use the old Toolbar without deprecation warnings) and the accessible Toolbar adoption could be broken into more PRs.

In the meanwhile, other projects using @wordpress/components could still benefit from this (knowing that it's experimental) and we would have room to try and change things.

My statement is so just because I don't have enough experience with how people create custom blocks, but maybe @gziolo may have a better perception.

[youknowriad]

I think this PR could be simplified if all the new features are made experimental and no deprecation is made. Gutenberg itself wouldn't change (it would still use the old Toolbar without deprecation warnings) and the accessible Toolbar adoption could be broken into more PRs.

If we make it experimental, when do you think we could get out of the experimental phase? What concerns me the most is not the deprecations. Deprecations are good because they let developpers know what they should do.

It seems like you're saying that we can add the new components without touching the existing Core blocks. That is reassuring. So aside deprecations which can potentially break e2e tests, if I'm a block author and I don't touch my code, is there something that can break?

[diegohaz]

If we make it experimental, when do you think we could get out of the experimental phase? What concerns me the most is not the deprecations. Deprecations are good because they let developers know what they should do.

How is the process that is currently used to take things out of the experimental phase?

The removal of the deprecation would allow us to change between both implementations and even revert this one entirely if it doesn't work out. One thing that I learned while working on this PR is that making things __experimental and not deprecating stuff in the first step is the best way to introduce new features on Gutenberg (as long as you continue working on that).

Steps I would propose (they could be separate PRs):

1. Simplify this PR so only packages/components/src/toolbar-* is touched.

   The accessible toolbar would be enabled by passing an __experimentalAccessibilityLabel prop to Toolbar. Only ToolbarButton would be accepted as a toolbar item (with the current state of this PR, it's Button, but only because ToolbarButton is still not flexible enough to serve all use cases).

   At this point, other projects using @wordpress/components could start experimenting it for simple toolbars.

2. Improve ToolbarButton so that it serves more use cases than just being an IconButton.

   At this point, the Gutenberg header toolbar (without the fixed block toolbar) could use it.

3. Refactor SlotFill so that the workarounds introduced by the current state of this PR (forcing fills to re-render when fillProps change, for example) are not necessary.

   At this point, block toolbars could start using it.

4. Convert the fixed toolbar buttons (block switcher, more tools etc.) to ToolbarButton.

   At this point, custom blocks could start using it, as long as they use ToolbarButton inside BlockControls or BlockFormatControls.

5. Convert core block toolbars.

   We could convert one per PR (for example, start with the Paragraph toolbar).

6. Let it be for a while.

7. If it proves to be useful and works out, deprecate the old usage and remove the __experimental prefix. Otherwise, revert it altogether, which would break the code for people using __experimentalAccessibilityLabel explicitly, but custom blocks would seamlessly rollback to the old usage.

[youknowriad]

An iterative process like that sounds good to me 👍

[diegohaz]

Closing in favor of #18534