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

<pf-tabs>: roles not being read #2692

Closed
ArathyKumar opened this issue Feb 8, 2024 · 12 comments · Fixed by #2731
Closed

<pf-tabs>: roles not being read #2692

ArathyKumar opened this issue Feb 8, 2024 · 12 comments · Fixed by #2731
Assignees
Labels
blocked Waiting on some other team or process bug priority: low Severity level: 3

Comments

@ArathyKumar
Copy link
Collaborator

Description of the issue

Elements with an ARIA [role] that require children to contain a specific [role] are missing some or all of those required children.

Impacted component(s)

  • All the pages which uses pfe-tabs

Screenshots

Screenshot 2024-02-08 at 3 23 25 PM

@ArathyKumar ArathyKumar added bug priority: low Severity level: 3 labels Feb 8, 2024
@bennypowers
Copy link
Member

@Arathy-s can you paste in the HTML here?

@ArathyKumar
Copy link
Collaborator Author

ArathyKumar commented Feb 8, 2024

Sure @bennypowers

<div class="overflow-tab-wrapper">
    <pf-tabs id="pf-tabs-p32svoeid" active-index="-1">
      <pf-tab id="users" slot="tab" aria-controls="pf-tab-panel-g6mjshlpm" tabindex="0" active="">Users</pf-tab>
      <pf-tab-panel id="pf-tab-panel-g6mjshlpm" tabindex="0" aria-labelledby="users">Users</pf-tab-panel>
      <pf-tab slot="tab" id="pf-tab-kvtmvh9o6" aria-controls="pf-tab-panel-29u49w0ww" tabindex="-1">Containers</pf-tab>
      <pf-tab-panel id="pf-tab-panel-29u49w0ww" tabindex="0" aria-labelledby="pf-tab-kvtmvh9o6" hidden="">Containers <a href="#">Focusable element</a></pf-tab-panel>
      <pf-tab slot="tab" id="pf-tab-h2hxamqi1" aria-controls="pf-tab-panel-ivkcx36yb" tabindex="-1">Database</pf-tab>
      <pf-tab-panel id="pf-tab-panel-ivkcx36yb" tabindex="0" aria-labelledby="pf-tab-h2hxamqi1" hidden="">Database</pf-tab-panel>
      <pf-tab slot="tab" disabled="" id="pf-tab-ec95bv707" aria-controls="pf-tab-panel-tw8aep4i5">Disabled</pf-tab>
      <pf-tab-panel id="pf-tab-panel-tw8aep4i5" tabindex="0" aria-labelledby="pf-tab-ec95bv707" hidden="">Disabled</pf-tab-panel>
      <pf-tab slot="tab" aria-disabled="true" id="pf-tab-lvtj31uqi" aria-controls="pf-tab-panel-9vlum7gix" tabindex="-1">Aria Disabled</pf-tab>
      <pf-tab-panel id="pf-tab-panel-9vlum7gix" tabindex="0" aria-labelledby="pf-tab-lvtj31uqi" hidden="">Aria Disabled</pf-tab-panel>
    </pf-tabs>
  </div>

@ArathyKumar
Copy link
Collaborator Author

ArathyKumar commented Feb 8, 2024

I believe the issue is occurring as the roles: region, tab, tabpanel, tablist are not mentioned.

@bennypowers
Copy link
Member

Which browser and version? Which os?

@ArathyKumar
Copy link
Collaborator Author

Browser: Chrome
Version: 121.0.6167.139
OS: macOS Sonoma 14.3

The issue can be reproduced on running a lighthouse scan on https://patternflyelements.org/components/tabs/.

@zeroedin
Copy link
Collaborator

zeroedin commented Feb 8, 2024

@bennypowers could this be a element internals/cross root aria issue, I'm leaning towards that it is. /cc @nikkimk

this.#internals.role = 'tab'; is set in PfTab and this.#internals.role = 'tabpanel'; is set in PfTabPanel, but those don't seem to get reported to the role="tablist" that is set on the slot in the parent PfTabs.

A work around that passes the lighthouse check is to manually set role="tab" and role="tabpanel directly on the pf-tab and pf-tab-panel elements.

Tabs currently does report role="tabs" in the Accessibility tree and we do test for that in the spec tests.

image

If it indeed a cross root aria issue, then we should have the element write these attributes out on the host element instead of trying to use element internals here (scratch that doing so seems to cause other issues in axe DevTools). Thoughts?

@bennypowers
Copy link
Member

bennypowers commented Feb 11, 2024

yeah my gut reaction was that this is a ua thing, but it would be better to try and rule out our own code first. One thing we can do to check this is screen reader testing

@bennypowers bennypowers changed the title pfe-tabs: High visibility accessibility issue <pf-tabs>: roles not being read Feb 11, 2024
@nikkimk nikkimk linked a pull request Feb 13, 2024 that will close this issue
@hellogreg
Copy link
Collaborator

Doing some testing on this. One odd thing on the docs page is that, if you click on (or shift keyboard focus for) a tab in one of the demos, it also changes which tab is selected in the "Reacting to changes" HTML/Lit/React tablist!

If you click on the first tab in any example on the page (e.g., Users in this screencap), then HTML will be selected. If you click on the second tab (e.g., Containers below), then Lit gets selected.
screencap of the Tabs docs page

This video shows what I mean:
Video of Tabs docs page issue

Not sure if this is at all related, but something I noticed in multiple browsers.

@hellogreg
Copy link
Collaborator

Did some basic real-world keyboard and screen reader testing too, with the following desktop combos:

  • Mac Safari and VoiceOver
  • Win Chrome and JAWS
  • Win Firefox and NVDA

First, keyboard usage without a screen reader on works in all browsers.

However, with screen readers on...

Mac

Safari and VoiceOver work fine for navigating the tabs. The screen reader switches to focus mode when focus is within the tab group, and the tabs can be operated via the arrow keys.

However, if you are just having VoiceOver read the page content, it does not read the individual tab headings. Instead, it announces the tab group, and then does so again for each of the tabs within.

In other words, for the following example...
screencap of a tab group

VoiceOver says, "Users and two more items, tab group," four times in a row. Once for the tab group, and once for each tab.

Windows

Interestingly, neither Chrome/JAWS nor Firefox/NVDA automatically enter focus mode for the tab groups. So the arrow keys do nothing, At least not at first.

In JAWS, the tabs can be navigated in browse mode via the apostrophe (and shift-apostrophe) key, and then selected. Focus mode cannot be entered manually (via the enter key), though this should be possible.

In NVDA, the user can enter focus mode manually (via the NVDA-space key combo), after which the tabs work as expected via the arrow keys.

Our tabs compared to WAI

I tested the WAI tabs example with the same combos.

The results:

  • Safari/VoiceOver: Works as expected (and doesn't have the read mode issue).
  • Firefox/NVDA: Works as expected. Enters focus mode automatically (unlike ours).
  • Chrome: Does not auto-enter focus mode (like ours). However, focus mode can be turned on manually (via enter key). Apostrophe navigation works in browse mode.

@bennypowers
Copy link
Member

bennypowers commented Feb 14, 2024

I want to make some changes in TabsController which I believe will help with some of the issues raised here. see #2695. this boils down to cross-root aria issues with the shadow button in PfTab, so i'm exploring the option of using a div in place of button and exposing the tab semantics on the host instead

@bennypowers bennypowers added the blocked Waiting on some other team or process label Feb 29, 2024
@bennypowers
Copy link
Member

blocked by #2677

@bennypowers
Copy link
Member

@hellogreg @nikkimk @Arathy-s is this still an issue after #2677 and #2695 were both merged?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked Waiting on some other team or process bug priority: low Severity level: 3
Projects
None yet
5 participants