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

Add Descriptive Heading Before Each "Assertions" Table #375

Merged
merged 2 commits into from
Feb 1, 2022

Conversation

jkva
Copy link
Contributor

@jkva jkva commented Dec 7, 2021

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

In the command-specific areas of the "Record Results" section for a test, there is no heading between the "After ..." input and assertions table.


Effective changes:

  • Introducing a level 4 heading with the text Assertions [description] before each assertions table.

@jkva jkva requested a review from jscholes December 7, 2021 13:51
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

  1. Can you give an example of what the heading text would be? If it references the specific testing task and/or command and/or whatever, there should probably be a colon after "Assertions".
  2. The table should be aria-labelledby the heading.

@jkva
Copy link
Contributor Author

jkva commented Dec 7, 2021

@jscholes agreed on describing the table by the heading, thanks. Other than that - the heading would be "Assertions After 'Enter'" when the instruction was to press 'Enter'.

I considered a colon separator, yet since the instruction seems to be consistently "After [X]", having the heading read "Assertions After [X]" seems to work just as well or perhaps more natural than "Assertions: After [X]". What do you think?

@jkva
Copy link
Contributor Author

jkva commented Dec 8, 2021

@jscholes I've aria-labelledby each table by its heading, and have introduced an explicit semi-colon separator in two separate commits.

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 Sorry I didn't respond to this yesterday. I actually agreed with you that no separator was best, given the formatting. A colon makes it a bit weird.

@jkva
Copy link
Contributor Author

jkva commented Dec 8, 2021

@jscholes that's why it's a separate commit! Will rebase and drop that one.

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.

Approving this, caveat emptor my comments in #379 (review).

Copy link
Contributor

@alflennik alflennik left a comment

Choose a reason for hiding this comment

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

I'm definitely receptive to the argument that we add headlines at each level of the information hierarchy. My concern is that it is adding clutter, both visual clutter and additional verbosity, but maybe it's not adding information that isn't apparent from context.

If the reader doesn't understand that the headline's purpose is to act as a label, they might end up carefully reading each headline, struggling to determine its semantic purpose and causing a distraction.

Perhaps this is a case where more information may not be better.

One thing we could do is add an aria-label to the table, which I think would improve the experience on screen readers when jumping between tables.

This is just an opinion, and so I'd definitely be curious how your team sees the pros / cons of this approach?

@jscholes
Copy link
Contributor

@alflennik People assigned to run tests for this project are assumed to have a baseline level of screen reader proficiency. This implies that, after learning the structure of the page they use often, they will be able to adapt their navigational keystrokes to that structure in whichever way suits them. If, for instance, they find it easier to Tab from the output field to the radios in the assertions table, they will do so. If they find it easier to jump directly to the table With T, they can do so. And the headings are there as a fallback, and to accomplish the initial task of learning about the page layout.

In other words: adding additional headings should provide a boost for people who find them helpful, and not get in the way of people who don't. I feel that in this instance, this requirement is met.

@evmiguel evmiguel merged commit 37c9512 into main Feb 1, 2022
@evmiguel evmiguel deleted the add-heading-before-assertions-table branch February 1, 2022 20:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants