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

Address Incorrect Mark-Up for Test Navigator Landmark #362

Merged
merged 3 commits into from Feb 1, 2022

Conversation

jkva
Copy link
Contributor

@jkva jkva commented Nov 25, 2021

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

The landmark wrapping the "Test Navigator" is marked up as a <nav>, but has been given a role of "complementary". This is incorrect; the entire section is used for navigation and as such, the default role implied by the <nav> would be more semantically accurate.

Meanwhile, the <nav> has no accessible name, and doesn't encompass the " Test Navigator" heading or toggle button. We recommend changing the heading to "Test Navigation", moving it and button inside the <nav>, removing the role attribute from the <nav>, and then applying an aria-label attribute to the <nav> with a value of "Test". The <ol> of tests should also be aria-labelledby the heading.


Effective changes:

  • the complementary role on the nav element has been removed;
  • the nav element is labelled by its leading h2 via aria-labelledby IDREF; and
  • the ol element containing the tests is similarly labelled by this h2.

@jkva jkva requested a review from jscholes November 25, 2021 12:37
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 Can we change the heading text to "Test Navigation", rather than "Navigator"? Then, because users will hear "Test Navigation navigation" (owing to the word "Navigation" in the heading and accessible role of the nav), exchange aria-labelledby on the nav for aria-label="Test". aria-labelledby can stay on the list.

@jkva jkva requested a review from jscholes November 29, 2021 15:22
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, missed this before: the heading needs to be inside the nav.

@jkva jkva requested a review from jscholes November 29, 2021 20:03
@evmiguel evmiguel merged commit f903e88 into main Feb 1, 2022
@evmiguel evmiguel deleted the label-navigator-nav-and-remove-landmark-role branch February 1, 2022 18:14
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

4 participants