Improve the responsive toolbar accessibility

Can I get a demo please.

- I had to navigate to the bottom of the page to find the toolbar, past the Powered by Drupal. If I wasn't looking I wouldn't have found it.
- There is a "Menu button" with no label that seems to do nothing when clicked
- There are a lot of actions, in different lists, and it is difficult to distinguish what I might find where.

JAWS reads:

navigation region start
Toolbar navigation
list of 2 items
Menu button
Home
list end

Toolbar user actions
list of 2 items
Hello toolbar_tester
Log out
list end

Shortcuts
list of 2 items
Add content
Find content
list end
Edit shortcuts
navigation region start
Administration menu
list of 8 items
Content
Structure
Appearance
People
Extend
Configuration
Reports
Help
list end
navigation region end
navigation region end

Marking this as postponed on #1137920: Fix toolbar on small screen sizes and redesign toolbar for desktop.

Would be good to have a full review of the toolbar.

@caecus (he is another screen reader user :-)) and I discussed the toolbar accessibility in the skype accessibility group some time ago. I promised to report here the conclusion that we came to, but I never did it! :(

First off, we both agreed on the fact that the toolbar cannot be compared to a standard toolbar (e.g. the toolbar of a wysiwyg editor) and as a consequence the toolbar ARIA role is not appropriate for it. But we came to the conclusion that our toolbar is a composite widget and that it doesn't convey this information to a screen reader user.

So, we should make sure that the toolbar is wrapped in a container which the "group" ARIA role is assigned to either by wrapping it in a container or adding the attribute to an existing one. In addition to this, a (translatable) ARIA-label containing the text "Drupal toolbar" (the string could be improved, but we didn't have a better idea :-)) should be added to this container. Furthermore, each toolbar tray should be wrapped in a "nag" container which the navigation ARIA role is assigned to. The tray label (that is currently shown inside an heading before the tray content) should be added as an aria-label to that container.

The first section of the toolbar (the one that contains the buttons to show/hide the various sections of the toolbar) should be wrapped in a "nag" container which the navigation ARIA role is assigned to as well. In addition to this the "toolbar items" label (that is currently shown inside an heading before the various buttons) should be added as an aria-label to that container and its text should be changed both in the heading and in the aria-label from "Toolbar items" to a more significant string (the most appropriate string has not been identified yet, help is greatly appreciated :-)).

The headings shouldn't be removed from the toolbar, so that the same information (the section label) is conveyed through the aria-label and the heading. This prevents us from degrading the uix for drupal users using a screen reader that does not support the aria-label attribute.

Please note that this is an experimental solution. This means that an eventual patch should be carefully tested with various screen readers before getting committed. I wanted to post this proposal to get the discussion started! :-) Any different proposal/idea is very very welcome.

Might be a better title. Also tagging.

@jessebeach, well, I think that the problem is mostly because of the "Toolbar items" heading that is positioned before the buttons. It doesn't allow you to understand easily what the buttons are supposed to do. I don't think that clicking on a button to make a menu show up is an accessibility issue; letting blind people understand this behavior easily is an accessibility issue, though! :-)

Also, if people agree on the proposed solution in #8 I can submit a patch. However, it will need to change core/modules/toolbar/templates/toolbar.html.twig significantly by moving things around (especially due to the conditional check for tray.label before printing it out).

So, basically the end result will be that if you add a tray to the toolbar but don't specify a tray.label the tray will not be shown. Of course I'm completely fine with this behavior because it will enforce the usage of tray.label.

See related issue #2052473-2: Add aria-label or aria-describedby attributes to all <nav> elements - Would be good to add aria-labels to nav elements in the toolbar here rather than there.

Here's a patch to help extend discussion about #6 above.

1) Add ARIA group

So, we should make sure that the toolbar is wrapped in a container which the "group" ARIA role is assigned to either by wrapping it in a container or adding the attribute to an existing one.

Any reason not to add role="group" to the <nav>?

2) Add ARIA-label

In addition to this, a (translatable) ARIA-label containing the text "Drupal toolbar" (the string could be improved, but we didn't have a better idea :-)) should be added to this container.

Why not just link the container to the H2 with labelledby?

3) Add ARIA to each toolbar tray

Furthermore, each toolbar tray should be wrapped in a "nag" container which the navigation ARIA role is assigned to. The tray label (that is currently shown inside an heading before the tray content) should be added as an aria-label to that container.

I'm blanking on "nag" container. You use that term here too #2052473: Add aria-label or aria-describedby attributes to all <nav> elements - not navigation.. First Mozilla's example above suggest using aria-owns.

4) Assign ARIA role to first section of the toolbar

The first section of the toolbar (the one that contains the buttons to show/hide the various sections of the toolbar) should be wrapped in a "nag" container which the navigation ARIA role is assigned to as well.

5) Add ARIA-label to container

In addition to this the "toolbar items" label (that is currently shown inside an heading before the various buttons) should be added as an aria-label to that container and its text should be changed both in the heading and in the aria-label from "Toolbar items" to a more significant string (the most appropriate string has not been identified yet, help is greatly appreciated :-)).

Not sure what we should call this.

6) Leave the headings

The headings shouldn't be removed from the toolbar, so that the same information (the section label) is conveyed through the aria-label and the heading. This prevents us from degrading the uix for drupal users using a screen reader that does not support the aria-label attribute.

Agreed.

@mgifford, uploading a new patch to show in practice what I meant in comment#6...

I'm going to reply to your question soon.

Again, I'd like to remark that this is experimental code and that I'm not 100% sure this is the right thing to do to improve the toolbar accessibility.

1. No, it looks good to me. However, I'd prefer assigning the "group" role to a (generic) div than to a (more semantically meaning) nav.
2. When I wrote comment#6 there was a general trend removing IDs wherever possible, so I didn't want to introduce new ones. It wouldn't be a problem adding IDs for me, though.
3. OMG. It looks like I didn't notice this typo some times... Of course I meant "nav".

Of course in the patch I posted there's an issue: we use
{{ tray.label }}
before cheking if it exists. This is the issue I was talking about in comment#6: the only solutions to this problem are:

1. Move the conditional check for tray.label one line above, so that it is performed before printing the tray container;
2. Remove this check and ensure elsewhere that each defined tray has a label.

Both solutions have a great advantage: they will enforce anyone that defines a new toolbar tray to define a label for that tray as well to get the tray printed out. IMO this is desirable from an accessibility point of view.

Thanks @falcon03

This applies nicely and doesn't cause any known problems that I can see.

I just looked at the patch and applied it to SimplyTest.me

@MiriamGonzalez, could you explain the reason why you set the issue status to "active"?

@mgifford, thanks. I'm waiting for feedback about the question in comment#20 so that I can complete the patch.

I'm for keeping it simple. Since this is already being tested for:

         {% if tray.label %}
           <h3 class="toolbar-tray-name visually-hidden">{{ tray.label }}</h3>
         {% endif %}

Let's just move the conditional check for tray.label one line above as you suggested in #20.

I do think that if there is a reason to have the ID's that it's fine and I really don't think that there is any way to add this semantic information into a dominant part of D8 without them.

How do we best document the enforcement around the new toolbar tray? We want it to be easier for there to be labels than not to have labels.

Hopefully this helps move this issue ahead. It's leveraging tray.label as suggested in #24.

This seems like an easy win. There are no UI changes, just additional semantics.

Patch still applies nicely.

No longer applies.

I realized we may not actually want to list Drupal here:

-      'role' => 'navigation',
-      'aria-label' => t('Site administration'),
+      'role' => 'group',
+      'aria-label' => t('Drupal toolbar'),

It might be better to just add the toolbar to the description like:

-      'role' => 'navigation',
-      'aria-label' => t('Site administration'),
+      'role' => 'group',
+      'aria-label' => t('Site administration toolbar'),

@mgifford, great idea. Let's do it...

Excellent. This patch looks good to go.

Back to RTBC, #33 was a testbot glitch.

The issue summary does not explain the scope of the patch or what it is fixing.

Filling in summary and moving it back to RTBC. Thanks @alexpott for getting us to improve the summary. If it needs more, do let me know.

Committed 77a9ea0 and pushed to 8.x. Thanks!