vmware-clarity / ng-clarity

Clarity Angular is a scalable, accessible, customizable, open-source design system built for Angular.
https://clarity.design
Other
341 stars 77 forks source link

[A11y] [ClrTabs] Incorrect order of tabs leads to wrong announcement by the screen reader #451

Closed Gesh4o closed 1 year ago

Gesh4o commented 1 year ago

Describe the bug

When using clarity tabs alongside structural directives like *ngIf the order in which they are announced by the screen reader will be messed up:

Example:

<clr-tabs>
  <clr-tab>
    <button>
      Organization
    </button>
  </clr-tab>

  <clr-tab *ngIf="true">
    <button>
      Recommended
    </button>
  </clr-tab>

  <clr-tab>
    <button>
      All
    </button>
  </clr-tab>
</clr-tabs>

Expected order announced by screen reader: Organization - 1/3 Recommended - 2/3 All - 3/3

Actual: Organization - 1/3 All - 2/3 <--- wrong Recommended - 3/3 <--- wrong

This issue was originally logged under VPAT-16650 (in internal vmware JIRA).

How to reproduce

Steps to reproduce the behavior:

  1. Open https://stackblitz.com/edit/clarity-light-theme-clr13-cds6-ng14-vefu2w?file=src%2Fapp%2Fapp.component.html alongside screenreader of your choice.
  2. Make the assisting technology announce the tabs
  3. See error

Notes

My understanding of the problem is that:

  1. The second tab is used with *ngIf and it will be scheduled for rendering after all the other tabs and
  2. Clarity assumes that the tab order is depended by WHEN the tab is rendered and not by WHERE exactly in the tab list the new tab is loaded
  3. Leads to the wrong order of ids inside aria-owns attribute that is used internally for the ul element.

example

Versions

Clarity version:

Framework version: ie: Angular 14

Device:

Additional notes

Add any other notes about the problem here.

Jinnie commented 1 year ago

This is an interesting one. The order in the DOM is fine (don't get mislead by the IDs numbers). But in the A11Y tree it's bugged:

Screen Shot 2023-01-10 at 13 44 26

The A11Y tree is managed by the browser. Not sure why it does not get updated, but semantically, our tabs are organized as list element items (LI) inside an Unordered list (UL). So it can't be unexpected that they don't keep track of the order.

Maybe we can try changing the UL to something else.

@AmyLiNow Amy, does the A11Y team have any observations on similar cases, or comments about my thoughts on the Unordered list?

AmyLiNow commented 1 year ago

Hi @Jinnie this is working as intended by the ARIA implementation. The issue here is what the reporter @Gesh4o pointed out in the screenshot, which is the order in aria-owns attribute value (aria-owns="clr-tab-link-0 clr-tab-link-1 clr-tab-link-2") does not reflect the intended reading order because it is out of order in the tablist id's (clr-tab-link-0,clr-tab-link-2, and then clr-tab-link-1). So the screen reader will read the last 2 out of order due to how it is listed (I've confirmed with screen reader on the stackblitz by removing aria-owns in the DOM).

Additionally, aria-owns should never be used when the DOM order already reflects the intended reading order.

The solution here is to remove aria-owns completely and let the natural DOM order dictate the reading order for assistive technologies, you can see also that W3C Manual Tab Example doesn't use aria-owns either.

If you like to learn more, this MDN aria-owns article does a great job explaining aria-owns attribute.

Hope this helps!

Jinnie commented 1 year ago

It helped a lot, thanks. PR is ready.

github-actions[bot] commented 1 year ago

:tada: This issue has been resolved in version 13.11.3 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

github-actions[bot] commented 1 year ago

Hi there 👋, this is an automated message. To help Clarity keep track of discussions, we automatically lock closed issues after 14 days. Please look for another open issue or open a new issue with updated details and reference this one as necessary.