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

Fix Nested Interactive Accessibility Violation #10396

Closed
19 tasks
pixiwyn opened this issue Jun 5, 2024 · 8 comments
Closed
19 tasks

Fix Nested Interactive Accessibility Violation #10396

pixiwyn opened this issue Jun 5, 2024 · 8 comments
Assignees

Comments

@pixiwyn
Copy link
Collaborator

pixiwyn commented Jun 5, 2024

As a user of Dawson, so that I can navigate the application using a screen reader and other accessibility tools, I need the tabs in Dawson to be free of accessibility violations.

Pre-Conditions

10382

Acceptance Criteria

  • All cypress accessibility tests should pass without excluding the axe core rule, nested interactive.

Notes

Tasks

Test Cases

Story Definition of Ready (updated on 12/23/22)

The following criteria must be met in order for the user story to be picked up by the Flexion development team.
The user story must:

  • Is framed in business/user need, the value has been addressed.
  • Includes acceptance criteria
  • Has been refined
  • Pre conditions have been satisfied.

Process:
Flexion developers and designers will test if the story meets acceptance criteria and test cases in Flexion dev and staging environments (“standard testing”). If additional acceptance criteria or testing scenarios are discovered while the story is in progress, a new story should be created, added to the backlog and prioritized by the product owner.

Definition of Done (Updated 5-19-22)

Product Owner

Engineering

  • Automated test scripts have been written, including visual tests for newly added PDFs.
  • Field level and page level validation errors (front-end and server-side) integrated and functioning.
  • New screens have been added to cypress accessibility axe
  • All new functionality verified to work with keyboard and macOS voiceover https://www.apple.com/voiceover/info/guide/_1124.html.
  • Swagger docs have been updated if API endpoints have been added or updated.
  • UI should be touch optimized and responsive for external users.
  • Interactors should validate entities before calling persistence methods.
  • Features have been optimized where possible to reduce response times. For example, reducing api response times, parallelizing client network calls, optimizing database reads, etc.
  • Types have been added to all added and updated functions.
  • Code refactored for clarity and to remove any known technical debt.
  • Acceptance criteria for the story has been met.
  • If there are special instructions in order to deploy into the next environment, add them as a comment in the story.
  • Code that resides in the shared folder that only runs on the API or browser has been moved to either /web-client or /web-api.
@ttlenard
Copy link
Collaborator

ttlenard commented Jun 28, 2024

Test Cases

  • Cypress tests pass
  • Screen readers work when accessing the system/Tabs
  • Works with screen readers on mobile devices.

@Mwindo Mwindo self-assigned this Jul 19, 2024
@ttlenard
Copy link
Collaborator

ttlenard commented Jul 29, 2024

Some testing feedback:

When I use NVDA and I hover over these tabs in DAWSON on TEST, it no longer reads aloud. I double checked Production, and the screen reader is reading these tabs when I hover my mouse over them. @Mwindo did some research and found that this may be an existing issue with NVDA and that fixing the violation may actually have made it "correct" behavior. For documentation purposes, I am listing the menus that are NOT being read aloud when hovering over. Some menu's in DAWSON still do read aloud (like trial sessions menu tabs and the Search menu tabs).

Note: Edge's Read aloud tool reads the menu's on Test and so does the Microsoft Narrator.

@Mwindo - Please let me know what you find out, are we still in violation, or is this actually desirable behavior?

Docket Record/Document View
image.png

Tracked Items Sub menu
image.png

Case Messages:
image.png

Case Information sub menu
image.png

Document QC (my document QC and Section)
image.png

Messages (My Messages and Section)
image.png

Judge's Dashboard:
image.png

Judge's Activity Report:
image.png

@Mwindo
Copy link
Collaborator

Mwindo commented Jul 29, 2024

Just a quick update: the tabs that continued to be read aloud by NVDA are those that have a <button><span>Some text</span></button>; the tabs that were no longer read aloud have <button><h2>Some text</h2></button> (or some other heading, e.g., h1, h3, etc.). In fact, I think this does point out another issue: we should not be using h2 (etc.) within a <button>. This likely explains the apparently inconsistent behavior of NVDA.

@Mwindo
Copy link
Collaborator

Mwindo commented Jul 30, 2024

Our Tabs component takes in an optional headingLevel argument. Base on the git history, it looks like this was done for purely semantic reasons (i.e., not for styling, which is done separately). (Currently, we only ever pass a headingLevel of 2. Often we pass nothing at all. This is the reason our tabs look different and respond differently to NVDA.)

I have pushed changes so that we render <h2><button><span>Some text</span></button></h2>.

EDIT: The above fix won't work (the button, which has role="tab", must be the direct descendant of the "tablist", but the heading separates them). This is actually a bit of a tricky issue. Outline of the issue:

We originally used h2 headers to address this ticket: https://app.zenhub.com/workspaces/flexionef-cms-5bbe4bed4b5806bc2bec65d3/issues/gh/flexion/ef-cms/8106. However, the solution that closed this ticket was actually problematic; in fact, I don't even think it solved the issue because descendants of tab are presentational (which means they are no longer part of the accessibility tree, rendering the heading basically useless). In this case, we are sort of trying to have our cake and eat it too: we want to enforce the semantics of the heading while at the same time enforcing the interactivity of the tabs (via button). I'm not sure if there is a clean way to do this, and we might have to choose one or the other.

@Mwindo
Copy link
Collaborator

Mwindo commented Aug 1, 2024

I have thought about this off-and-on for a few days. It is surprisingly tricky. Long story short: the relationship between tablist and tab is enforced slightly differently depending on the standard/implementation: https://www.w3.org/TR/wai-aria-1.2/#tab says it is ok for tabs not to be direct children of tablist; https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/tab_role says tabs must be direct children or else be specified in the tablist aria-owns attribute; axe seems to enforce tablist having direct tab children period.

I will discuss options with the team.

EDIT: Actually, I think I finally found a good solution. We can put a screen-reader only (see, e.g., this example styling from bootstrap), duplicative header within the tab-panel content. This ensures:

  • Visually, the tabs will "look" like headings
  • For accessibility, the heading (h2) will still exist on the page at the top of the tab content
  • We are not skipping a heading level

@Mwindo
Copy link
Collaborator

Mwindo commented Aug 6, 2024

I've pushed this fix to test.

@ttlenard
Copy link
Collaborator

ttlenard commented Aug 7, 2024

There is now consistent behavior with NVDA. I do want to note that none tabs are being read aloud now with NVDA, and I'm not entirely sure if this is desirable behavior? I did some testing with Microsoft narrator and Edge's read aloud tool, and both of these tools do read the tabs aloud when hovering your mouse over them. It may just be an issue with NVDA???

@Mwindo
Copy link
Collaborator

Mwindo commented Aug 7, 2024

@ttlenard Yeah, I think different screen readers have slight differences in what they read out by default on hover. I think if tabs behavior is consistent (per tool), we should be good to go!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Closed
Development

No branches or pull requests

6 participants