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

Convert Assertions Table into Fieldset List #387

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

jkva
Copy link
Contributor

@jkva jkva commented Dec 10, 2021

This PR addresses the following item from PAC's "ARIA-AT App Screen Reader Accessibility Observations" document:

Screen reader output is quite verbose and confusing when navigating into the radio button group within an assertions table. For example when tabbing to a good output radio button, an NVDA user may hear something like: "Good Output for assertion Role 'combobox' is conveyed Success case. Good Output for assertion Role 'combobox' is conveyed radio button not checked". This is technically accurate, but is also a lot of information to take in (particularly for users reading on a braille display).

However, the output is completely bewildering when Up Arrowing to the last radio button in the group, which is in a different column of the table. An NVDA user may hear something like: "No Output for assertion Role 'combobox' is conveyed Incorrect Output for assertion Role 'combobox' is conveyed Failure cases. Incorrect Output for assertion Role 'combobox' is conveyed radio button checked". This is just plain wrong, because it starts with "no output" even though the "incorrect output" radio is focused, and it may lead to testers leaving the wrong feedback.

Rather than the individual issues exhibited by the table being enumerated here, the current format, and any desired improvements, should be discussed further. Overall, the table format seems unnecessary, and may not scale on smaller screens when mobile screen readers are targeted by the project. We suggest that a simple radio button group would be sufficient, with no table mark-up at all.

This PR duplicates heading improvements in #375.


Effective changes:

  • Abstract list of assertions into own component;
  • convert table to ul of fieldset;

@jkva jkva requested a review from jscholes December 10, 2021 11:46
@jkva
Copy link
Contributor Author

jkva commented Dec 10, 2021

Note: this triggers a test check failure:

Check failure on line 1055 in client/components/TestRenderer/index.jsx

GitHub Actions
/ Using NodeJS and Postgres 12

client/components/TestRenderer/index.jsx#L1055

'data.description.toLowerCase' is missing in props validation

Not sure why this is as this change does not seem to touch that typedef.

Copy link
Contributor

@jscholes jscholes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jkva Love this. Some minor changes:

  1. Remove aria-labelledby from the nested lists of assertions (ul.assertion-input-group), to avoid hearing each assertion twice when Tabbing into radio groups.
  2. Mark up each assertion as an h5.
  3. Add whitespace between the visible and off-screen radio button label text (i.e. after "input|output"). Currently there is only a standard space and the text runs together in Chrome.
  4. File a separate issue re: improvements to the radio button labels. They're still not unique, even with the added off-screen text, and "for assertion" only buys verbosity rather than anything concrete. Given the grouping, I may even vote for the off-screen text to be removed, but we can discuss once the issue has been created.

@jkva
Copy link
Contributor Author

jkva commented Dec 10, 2021

@jscholes my latest commits have addressed your points 1 through 3.

@jkva jkva requested a review from jscholes December 10, 2021 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants