Tab interface accessibility

The attached patch adds a number of aria attributes, and sets unique ids not only for the tab panels (as it did previously) but now also for the tabs themselves, to properly populate the aria-labelledby property on the panels.

Here's an example of what the markup looks like for me after the patch:

<div class="layout layout--tabs">
    <div class="layout__region layout__region--tabs tabs-region">


<!-- Nav tabs -->
<nav class="nav layout-tabs" role="navigation">
  <ul class="tabs tab--primary" role="tablist">
              <li class="tabs__tab" role="presentation"><a class="nav-item nav-link is-active" href="#panel-193535ae-87e5-4a59-8540-4f56385223a6" id="tab-193535ae-87e5-4a59-8540-4f56385223a6" role="tab" data-toggle="tab" aria-controls="panel-193535ae-87e5-4a59-8540-4f56385223a6" tabindex="0" aria-selected="true">
   <div class="label"> Tab 1</div>
  </a></li>
        <li class="tabs__tab" role="presentation"><a class="nav-item nav-link " href="#panel-486143ad-fa45-4ca3-a6ce-7cdef48fda75" id="tab-486143ad-fa45-4ca3-a6ce-7cdef48fda75" role="tab" data-toggle="tab" aria-controls="panel-486143ad-fa45-4ca3-a6ce-7cdef48fda75" tabindex="0" aria-selected="false">
   <div class="label"> Tab 2</div>
  </a></li>
    </ul>
</nav>

<!-- Tab panes -->
<div class="tab-content">
    <div class="tab-pane active" id="panel-193535ae-87e5-4a59-8540-4f56385223a6" role="tabpanel" aria-labelledby="tab-193535ae-87e5-4a59-8540-4f56385223a6" tabindex="0" aria-hidden="false" aria-expanded="true" style="">
    
<!-- Tab Pane content from block -->

  </div>
    <div class="tab-pane " id="panel-486143ad-fa45-4ca3-a6ce-7cdef48fda75" role="tabpanel" aria-labelledby="tab-486143ad-fa45-4ca3-a6ce-7cdef48fda75" tabindex="0" aria-hidden="true" aria-expanded="false" style="display: none;">
    
<!-- Tab Pane content from block -->
  
  </div>
</div>
</div>
</div>

Yes the ARIA: tab role infromation suggests the tabindex on the destination tabpanel so I added that, though I do notice that this adds a noticeable focus ring ring around the tab content. I can see where this is helpful to make it explicit what has happened, but I can also see where some clients/site owners may not consider this desirable. I will confess that I am far from an "ultimate authority" on accessibility. Perhaps we could recruit the opinion of a certain accessibility consultant we both know?

I also just came across this WCAG-provided example where not only does the tab panel on have a tabindex value, only the active tab does: https://www.w3.org/TR/wai-aria-practices/examples/tabs/tabs-2/tabs.html

On the keyboard you can use the right and left arrows to move between tabs instead. Maybe this is a better approach? Curious to know if a keyboard user would consider this a common pattern.

There's also this, which has a number of similarities (including the fact that it doesn't shift focus to the tab panel) but directly changes the selected tab based on the arrow keys: https://matthiasott.github.io/a11y-accordion-tabs/

Additional comments from a conversion with @andrewmachpherson in the #accessibility slack channel:

mandclu 14 hours ago
I suppose the three main variations I can see are:
Manual vs. automatic activation of tabs - according to section 6.4 of the W3C spec automatic is fine as long as the content is already in the DOM
Tabindex for the tabpanel - The W3C example and the Mozilla have tabindex for the panel, which as you point out allows focus on the container, which may not be desirable
Use of the focus ring to highlight the active element - Surprisingly the W3C examples are the most subtle, not using the focus ring at all, instead using the top border colour and shading of the tab backgrounds to communicate which tab is active

andrewmacpherson 12 minutes ago
You're correct. The same section explains that manual is mainly important when there are network lookups which could take a while. The earlier section 3.22 says (more explicitly) that automatic activation is recommended, so long as there's no noticable latency.
Yeah, for me tabindex on a container is a deal-breaker. Lot's of Gatsby-based sites suffer from this, because their JS router uses tabindex=-1 on the main container. It means I am forced to be a keyboard-only user, instead of a keyboard-mostly user.
The visual focus ring is for operable controls. A container isn't operable. That's another reason why tabindex is a bad idea for a container.
As an alternative to putting tabindex on a large container, you can put tabindex on a smaller element near the start of the container. For example a heading, or an empty span. Drupal core's skip-to-main-content target works this way.

andrewmacpherson 9 minutes ago
I studied the mattiasscott accordion-tabs. When it's wide enough for tabs, it behaves very well. However when it's displayed as an accordion, there are some problems with missing roles. I'll see if I can find time to file some issues with the upstream project.

andrewmacpherson 1 minute ago
For the layout builder use-case, some things to bear in mind:
As a developer (or even as a site builder) we have NO idea how much content is going to be put into a given tab panel. This is strong reason to avoid a tabindex on the tabpanel container. The larger the content, the more likely this is to confound mixed keyboard/pointer use.

andrewmacpherson < 1 minute ago
I'll see if I can test your patch manually. It's got potential to become a very popular module, so it's great that you're addressing the accessibility. (edited)

Updated patch, trying to draw on best practices from the examples cited in this thread. In particular:

• Only the active tab is tab-focusable
• Now uses the arrow keys (left and right, as per the WCAG spec) to move between tabs, and updates the visible tab panel as per WCAG guidelines

I got some feedback that a label to indicate the way to switch between tabs would be helpful, so that's in the attached patch.

Missed a closing double-quote, new patch.

Using macOS Catalina VoiceOver software, tab navigation aria-label instructions are read clearly to the user. Navigating tabs with the arrow keys while using VO as per the aria-label instructions works.

Thanks everyone, this has been merged in. Should have out a new release shortly.

The <nav> element here has several problems:

+<nav class="nav layout-tabs" role="navigation" aria-label="{{ 'Please use the left and right arrow keys to move between tabs'|trans }}">
+  <ul class="tabs tab--primary" role="tablist">

• You don't need to wrap a role=tablist with <nav role=navigation>. The navigation role is a landmark region, but tablist isn't. Moreover, tabs don't generally amount to navigation; it's a misuse of the role.
• If you intended to treat the content in the set of tabs as a landmark, you'd need to the wrap the entire set of tabbed content (tablist and tabpanels together) with a role=region and an appropriate label.
• However, I don't recommend using a landmark region at all here. The purpose of a landmark region is to identify the major sections of the page (e.g. header, footer, sidebar, main content, and site menu) OR some special content which users need to be able to reach quickly (e.g. a notification area, or shopping cart summary). A layout-builder section might qualify as one of these, but we can't say that for sure. Page content certainly doesn't qualify as a landmark region merely because it's a Drupal layout builder section. These sections are solely about layout, and don't connote any particular semantics, context, purpose, or importance (and it's already inside of the <main> landmark). Nor does it qualify as a landmark merely because it contains some tabs.
• If an assistive tech user is inside a tabpanel, and wants to change to another tabpanel, they don't need the <nav> landmark region. The tablist and tabpanel roles are sufficient. The active tabpanel follows directly after the tablist (because the inactive tabpanels are hidden), so all the user needs to do to find the tablist is read backwards a little bit.
• A page can have too many landmarks, and some Drupal themes already have a landmark count which is on the high side. About 4-6 landmark regions is nice, but 10 is likely to be too many. Layout builder allows an arbitrary number of sections; some landing pages might have a dozen. If every layout builder section used a landmark region wrapper, that would likely lead to far too many landmarks.

'Please use the left and right arrow keys to move between tabs' isn't appropriate as an aria-label:

• It reads more like a set of instructions, which isn't the intended purpose of aria-label. It's for the name of the landmark, not how to operate the controls inside.
• If instructions are necessary, they're better associated with the tablist itself, using aria-describedby, and visible for all users.
• In this case, it's overkill. The tablist-and-tab roles cause screen readers to announce those controls as tabs. This sets up the user expectation that they can pick a tab using the arrow keys, because it's the normal behaviour of tabs in all desktop applications. There isn't really a need to provide your own instructions for standard controls. At best it's noise; at worst it's a bit condescending and discriminatory - particularly since you aren't telling sighted keyboard users how to operate them!
• If a landmark were necessary, it would need a distinct name. Their purpose is to identify the major sections of the page, but that doesn't work if they have duplicate names. As it stands, if you have 2 of these tabbed layout-builder sections, they'll end up having the same name.

Recommendation: you can fix all of this at once by removing the <nav> wrapping element. (Or you change it to a <div>, if you need to keep the classes for styling; but without a role or aria-label.)

The HTML source (from the twig template) contains lots of ARIA attributes which are better added using Javascript. It's a good practice to keep ARIA roles, properties, and states relating to dynamic behaviour out of the static HTML, because they are only relevant when the JS runs.

The "tabs" are actually a list of internal fragment links with overridden roles and behaviour. This is good because it can work without Javascript, and the tab behaviour is a progressive enhancement.

If JS doesn't run for some reason, the tab behaviour doesn't work. However the list and link roles are still overridden, so the presence of tabs will still be announced. This is undesirable because it's telling assistive technology users to expect behaviour which isn't there.

I recommend taking these roles and properties out of the Twig template, and initializing them in Javascript:

• role=tablist (on the UL)
• role=presentation (on the LI)
• role=tab (on the links)
• aria-controls (on the links)
• aria-selected (on the links)
• tabindex="-1" - this is crucial to move to JS, as it's currently stopping the links from working as links when JS doesn't run.
• role=tabpanel (on the div.tab-pane)
• aria-hidden (on the div.tab-pane)
• aria-selected (on the div.tab-pane)
• Optionally, move the aria-labelledby (on the div.tab-pane) to JS. Without a role attribute, aria-labelledby has no effect on a div.

What is data-toggle="tab" used for? It isn't mentioned in the module's Javascript.

@andrewmacpherson Thanks for the very detailed feedback! The attached patch should address your feedback, and also seems to make the code more readable.

I read patch #20 - yes, that's very much what I had in mind. Note I haven't manually tested this, just a code review.

$(this).attr('aria-controls', thisId.replace('tab', 'panel'));

The use of replace is nice - very easy to follow.

Merging this in to make an updated release. Thanks for the feedback, everyone.