flexion / ef-cms

An Electronic Filing / Case Management System.
23 stars 10 forks source link

Fix Nested Interactive Accessibility Violation #10396

Open pixiwyn opened 2 months ago

pixiwyn commented 2 months ago

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

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:

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

ttlenard commented 1 month ago

Test Cases

ttlenard commented 1 week ago

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 commented 1 week ago

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 commented 6 days ago

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 commented 4 days ago

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: