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

Improve the top bar tools interaction and consistency #12073

Merged
merged 2 commits into from
Nov 26, 2018

Conversation

afercia
Copy link
Contributor

@afercia afercia commented Nov 19, 2018

Fixes #12040

This PR seeks to improve semantics, interaction, and consistency of the Undo, Redo, "Content structure", and "Block navigation" tools in the top bar, trying to have a more consistent user experience.

Currently, some of these controls are initially disabled and not focusable, other ones are not really disabled and focusable, also the tooltip is visible or not visible depending on the disabled state. See below for more details.

This PR:

  • simplifies and makes BlockNavigationDropdown and the table-of-contents Dropdown work similarly
  • when there is no content, their buttons use now aria-disabled so they're still focusable and discoverable, also their tooltip is visible
  • removes the text 'No blocks created yet.' and returns early if there are no blocks yet
  • removes an unnecessary test

Quoting from the related issue:

Currently, there are at least 3 different initial behaviors that are potentially confusing for users:

Undo and Redo buttons:

  • they're not really disabled, they use an aria-disabled="true" attribute
  • they're still focusable
  • their tooltip is visible

As discussed in previous issues / PRs, they're not disabled to avoid a focus loss when using a keyboard and there are no more undo / redo steps.

Content structure button:

  • it uses a disabled attribute, so it's really disabled
  • it's not focusable
  • the tooltip is not visible

As it's not focusable and its tooltip is not visible, the discoverability of this tool is a bit reduced.

Block Navigation button:

  • it's always enabled
  • it's always focusable
  • the tooltip is visible
  • its popover is always rendered even when it's empty because there are no blocks inserted yet, which is a noticeable different behavior compared to the Content structure tool

From an accessibility perspective, using aria-disabled="true" and "noop'ing" an UI control is equivalent to disabling it with a disable attribute, as long as it is perceived as disabled by all users, i.e. also visually. The relevant difference is that a control with aria-disabled="true" is still focusable. Removing focusability from disabled elements can offer users both advantages and disadvantages. A must read recommendation can be found in the ARIA authoring practices:

5.7 Focusability of disabled controls
https://www.w3.org/TR/wai-aria-practices-1.1/#kbd_disabled_controls

Although those recommendations apply to ARIA widgets, the Gutenberg top bar can be considered a "toolbar", where the ability to discover the functionalities within the toolbar is a primary concern. I'd recommend to adopt a consistent convention for the focusability of these controls and always use aria-disabled="true" / "noop". The Block navigation menu should not open its panel when there are no blocks.

Lastly, please do let me know if more consistency in title-case / sentence-case for the tooltips text is desirable. Right now, they're not so consistent:

screenshot 2018-11-19 at 17 34 55

@afercia afercia added the [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). label Nov 19, 2018
@afercia afercia requested a review from youknowriad November 19, 2018 16:51
@jasmussen
Copy link
Contributor

I can't recall if this is ticketed yet. I think it might be, perhaps #9053 though I might go ahead and change the title — I feel strongly that we should find a way to merge the Document Outline tool, and the Block Traversal button into a single one. Out of scope for this PR, though, but worth mentioning.

@gziolo gziolo added this to the 4.6 milestone Nov 22, 2018
@afercia
Copy link
Contributor Author

afercia commented Nov 22, 2018

I remember @paaljoachim mentioned it too.

@paaljoachim
Copy link
Contributor

Since the Document outline and the Block Navigation is so similar it is natural to merge these. I will create an issue for it now....

@jasmussen
Copy link
Contributor

@paaljoachim Can you hold off on that for a bit? We might be able to use #9053 for it instead. There's valuable context worth keeping.

@afercia afercia force-pushed the update/top-bar-tools-interaction-consistency branch from ad32fb5 to 675eb15 Compare November 22, 2018 15:36
Copy link
Member

@noisysocks noisysocks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

Here's what it looks like visually. The focused button get a little darker.

tab

Should we make the Preview button behave this way as well? It seems to be the only button in the toolbar that gets completely disabled now.

@@ -74,6 +74,10 @@ function BlockNavigation( { rootBlock, rootBlocks, selectedBlockClientId, select
)
);

if ( ! rootBlocks || rootBlocks.length === 0 ) {
return null;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: Could move this check to the top of the function or use the ifCondition HOC so that we're not unnecessarily computing hasHierarchy.

@noisysocks
Copy link
Member

Does this need to go in WordPress 5.0.0 / Gutenberg 4.6 or can it wait for WordPress 5.0.1 / Gutenberg 4.7?

@afercia
Copy link
Contributor Author

afercia commented Nov 26, 2018

Thanks @noisysocks.

Should we make the Preview button behave this way as well?

From an a11y perspective, I'm mostly interested in the buttons in the left part of the top bar because they're within a role=toolbar. The Preview button is more an usability / design thing, I'd defer to the team.

Does this need to go in WordPress 5.0.0 / Gutenberg 4.6 or can it wait for WordPress 5.0.1 / Gutenberg 4.7?

I'd prefer to have it in the first release, to get user used to this kind of interaction since the beginning.

@noisysocks noisysocks merged commit c33e917 into master Nov 26, 2018
@noisysocks noisysocks deleted the update/top-bar-tools-interaction-consistency branch November 26, 2018 22:37
@youknowriad youknowriad modified the milestones: 4.6, 4.7 Nov 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants