fix(Tabs): improve screen reader support

[emyarod]

Closes #6984

This PR updates the role attribute on the tab anchor and moves the aria attributes from the parent list item element to the anchor as well. The anchor is also updated to be a button now to prevent things like "Open link in new window" or "Copy link address" from appearing in the context menu

Changelog

Changed

• deprecate renderAnchor (to be replaced with renderButton, unless we have a better name in mind)
• move role and aria attributes from list item to inner anchor
• convert anchor to button element

Removed

• outdated tests

Testing / Reviewing

Ensure the updated tabs match the existing tabs in behavior and appearance and confirm the updated markup matches the expected markup from the referenced issue

[netlify]

Deploy preview for carbon-elements ready!

Built with commit 82e2f98

https://deploy-preview-6989--carbon-elements.netlify.app

[netlify]

Deploy preview for carbon-components-react ready!

Built with commit 82e2f98

https://deploy-preview-6989--carbon-components-react.netlify.app

[netlify]

Deploy preview for carbon-elements ready!

Built with commit bb28519

https://deploy-preview-6989--carbon-elements.netlify.app

[netlify]

Deploy preview for carbon-components-react ready!

Built without sensitive environment variables with commit bb28519

https://deploy-preview-6989--carbon-components-react.netlify.app

[andreancardona]

@emyarod looks good!

[carmacleod]

Still not working in screen readers. Still need to:

• delete tabindex="-1" from the <li>s (don't need them to be clickable because the buttons are clickable).
• add role="tab" to the <button> (because a tablist needs to have tab children).
• remove the href from the button (just to clean up)

Also, I looked more closely at the tabpanels this time. There's some work needed there as well - sorry I didn't look earlier.

• remove the aria-live attribute from the tabpanel. The user does not expect or want the contents of every tabpanel read to them when they are switching tabs in the tablist. They know that they can type the Tab key to go from the tablist to the current tabpanel if they want to go through the tabpanel's contents.

• don't need to use aria-hidden="true"/"false" on the tabpanel, because HTML hidden takes care of hiding content from screen readers, so we can clean up some code here (less code always runs faster, and it's easier to read... ;) )

• either the tabpanel element itself, or "the first element containing meaningful content inside the tabpanel" must be focusable. In this example, which mostly has non-interactive paragraphs in the tabpanels, each tabpanel should have tabindex="0" so that the user can type Tab to go from the tablist into the current tabpanel. The 4th tabpanel of this example has some focusable content, but I'm not sure whether jumping to the "renderContent" button qualifies as "meaningful content"? I think that what the guidance means about "meaningful content" is more like if the tabpanel contains a form with form controls, then it's ok to let the focus go to the first form control instead of giving the tabpanel a tabindex="0". This choice will likely need to be a new prop because it's app-specific logic - we can discuss.

[emyarod]

I updated the PR with your suggestions @carmacleod . The only exception is the focus order change for the tabpanels, but I think we can make that change separately after discussing the pattern and user expectations more

[carmacleod]

This is way better! The browsers and screen readers now recognize this as a tablist with tabs! 🎉
Now we can test the details (and we can look at adding the tabpanels to the focus order). 😄

• Note that div#tab-4__panel is missing role="tabpanel" for some reason (which really messes up Firefox).

I will test more later.

[emyarod]

Note that div#tab-4__panel is missing role="tabpanel" for some reason (which really messes up Firefox).

that tab was rendering some example custom content so I've added the role attribute to that example

[carmacleod]

that tab was rendering some example custom content so I've added the role attribute to that example

It would be better if the component didn't leave the ARIA markup up to the author?

Would it be difficult to code it so that the custom content is added as a child of the tabpanel div? So in the initial state, the tabpanel div is there, but simply empty? It can be hidden until it's selected. (Is the visually-hidden necessary?)

So, instead of:

<div id="tab-4__panel" hidden="" aria-labelledby="tab-4" class="bx--visually-hidden"></div>

Would it be possible to mark the tabpanel div up like this, and maybe use the div.some-content to add the custom content?

<div role="tabpanel" id="tab-4__panel" hidden="" aria-labelledby="tab-4" class="bx--tab-content tab-content"> 
    <div class="some-content"></div> <!-- Either have this div here initially, or add as child when tab selected -->
</div>

(Note that we still need to discuss adding tabpanels to the tab order, so the above doesn't add tabindex="0"... yet. :)

[emyarod]

It would be better if the component didn't leave the ARIA markup up to the author?

it isn't left up to the author by default but if they require a custom tab wrapper, it is available to them as an option. what you proposed is a possibility, but I think that eliminates existing functionality for the consumer which would be a breaking change. we also have to evaluate whether or not we should restrict tab panel content to a div since it would limit the versatility of the component

[carmacleod]

I think that eliminates existing functionality for the consumer

Maybe it can be written in such a way the change is completely transparent to the consumer?

we also have to evaluate whether or not we should restrict tab panel content to a div since it would limit the versatility of the component

The tabpanel itself is already a div, so maybe just add the content provided by the consumer/author as a child of the tabpanel?

Edit: Actually, I think there is something that I am completely not understanding here. The Tabs in this Tabs component are always rendered when selected (i.e. arrow to Tab, see content), so I'm not sure what the renderContent prop actually does?

[carmacleod]

Finally had a chance to test the scrolling arrow buttons, and I think that having them in the tab order and visible to screen readers is definitely problematic. Please note the following points in @aagonzales's keyboard navigation design:

• tab: traps focuses in the tab component starting at the select tab.
  • press tab again and focus will move out of the tab component and not the next tab inside the component.
  • clicking tab at any point while inside tab component will move focus to the next focusable item on page.
• Scrolling arrow buttons will be excluded from screen reader and keyboard arrow selections.
• home key takes the user to the first tab.
• end key takes the user to the last tab.

Together with the diagrams, and given that she was answering my scroll arrow button questions from #4758 (comment), I interpreted the first 2 design points above to mean that the scroll arrow buttons would not be in the tab order, and they would be hidden from screen readers.

We can do that by adding aria-hidden="true" tabindex="-1" to the scrolling arrow buttons (and then the SVG wouldn't need `aria-hidden="true" because descendants of an aria-hidden element are also aria-hidden).

Note also that right-click currently activates the scroll buttons, which feels really odd.

There's also a strange state that a keyboard user can get into where the tablist has the focus. We should make sure that doesn't happen. (maybe just taking the scrolling arrow buttons out of the tab order will fix that? I can test again afterwards).

Edit: Just realized I still need to test and think about mobile, and what happens in mobile screen readers if the scroll arrow buttons are taken out of the tab order. Argh. Are you allowed to conditionally take the scroll buttons out of the tab order on desktop and leave them in for mobile (if necessary)? Just asking, for now.

[emyarod]

the renderContent prop allows the consumer to render any custom content they want to display. In our example we have rendered something similar to the default tab content divs but in theory it can be used to render anything

regarding the suggested changes, the scroll buttons are no longer tab focusable and also marked with aria-hidden="true"

the SVG wouldn't need `aria-hidden="true"

just a side note, this is automatically applied to our icons by default when they are generated

Note also that right-click currently activates the scroll buttons, which feels really odd.

scroll buttons should now only activate with left mouse button

There's also a strange state that a keyboard user can get into where the tablist has the focus

I haven't been able to replicate that. Can you provide a list of steps to reliably enter this state?

Are you allowed to conditionally take the scroll buttons out of the tab order on desktop and leave them in for mobile (if necessary)?

This would require us to differentiate between mobile and desktop clients, which is possible but it would be very hacky and inconsistent I believe

for the Home and End key enhancements, I think we can create a separate ticket for that to curb the scope creep that's occurring to this ticket

[carmacleod]

This seems better, but the "strange state" is even more pronounced now, so I can't quite test fully yet:

There's also a strange state that a keyboard user can get into where the tablist has the focus

I haven't been able to replicate that. Can you provide a list of steps to reliably enter this state?

Here's how to replicate (saw this in FF and Chrome on Windows - didn't try it anywhere else):

• shrink your browser window so that at least one scroll arrow appears
• either:
  • refresh the browser and then type the tab key
  • or click on a tab in the tablist (i.e. Tab label 1, or Tab label 2) and then type shift+tab

Somehow, the ul is in the tab order, which is really unexpected, because it doesn't have a tabindex or anything. I wondered how this was possible, and I think this SO answer makes sense. So presumably we can give the ul a tabindex="-1" and that should take it out of the tab order even in this weird scrollable overflow situation. That seems to work for me if I just add tabindex="-1" to the ul in DevTools. (Of course, I can't test after refreshing the page, but I assume that would work).

Sorry about the scope creep. Some of it was because I can't test the screen readers until the markup reaches the point where screen readers recognize the component. :)

I've opened separate issues for:

• home/end keys in tablist
• add tabpanel focus api

Still need to open one for further discussion of renderContent api.
I still need to test mobile to see if the scroll arrow buttons need to be focusable, which may be an issue - not sure yet.