Skip to content

UnderlineNav2: Add aria-hidden and sr-only class support for a descriptive "More" button label - #2552

Merged
broccolinisoup merged 8 commits into
mainfrom
broccolinisoup/UnderlineNav-sign-off-remediations-3
Nov 10, 2022
Merged

UnderlineNav2: Add aria-hidden and sr-only class support for a descriptive "More" button label#2552
broccolinisoup merged 8 commits into
mainfrom
broccolinisoup/UnderlineNav-sign-off-remediations-3

Conversation

@broccolinisoup

@broccolinisoupbroccolinisoup commented Nov 8, 2022

Copy link
Copy Markdown
Member

Addressing an accessibility sign-off review issue that described below.

The label of the "More" overflow button is not descriptive, and could easily result in multiple buttons with the same name if the component is used multiple times on the same page. The heading text should be added to the accessible name of the button, e.g. "More Repository navigation items" or "More: Repository navigation", e.g. using an off-screen span inside the button and hiding the existing label from screen reader users with aria-hidden. Note that, as with the counter, the visible text of "More" must be included in the accessible name verbatim.

Merge checklist

  • Added/updated tests
  • Added/updated documentation
  • Tested in Chrome
  • Tested in Firefox
  • Tested in Safari
  • Tested in Edge

Take a look at the What we look for in reviews section of the contributing guidelines for more information on how we review PRs.

@changeset-bot

changeset-botBot commented Nov 8, 2022

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: eaeff03

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
NameType
@primer/reactPatch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@broccolinisoupbroccolinisoup changed the title UnderlineNav2: aria-hidden for visible More menu text and sr-only equivalent for screen readers and add list for ulUnderlineNav2: Add aria-hidden and sr-only class support for a descriptive "More" button labelNov 8, 2022
@github-actions

github-actionsBot commented Nov 8, 2022

Copy link
Copy Markdown
Contributor

size-limit report 📦

PathSize
dist/browser.esm.js78.89 KB (0%)
dist/browser.umd.js79.54 KB (0%)

@broccolinisoup
broccolinisoup temporarily deployed to github-pages November 8, 2022 03:38 Inactive
@broccolinisoup
broccolinisoup marked this pull request as ready for review November 8, 2022 04:39

@joshblackjoshblack left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Left a comment above for a structure question, let me know what you think!

Comment threadsrc/UnderlineNav2/UnderlineNav.tsx Outdated
…button label
Co-authored-by: Josh Black <joshblack@github.com>
@broccolinisoup
broccolinisoup temporarily deployed to github-pages November 8, 2022 22:44 Inactive
@broccolinisoup
broccolinisoup temporarily deployed to github-pages November 8, 2022 22:58 Inactive
@broccolinisoup
broccolinisoup temporarily deployed to github-pages November 10, 2022 01:12 Inactive
@broccolinisoup
broccolinisoup enabled auto-merge (squash) November 10, 2022 01:18
@broccolinisoup
broccolinisoup temporarily deployed to github-pages November 10, 2022 01:22 Inactive
@broccolinisoup
broccolinisoup merged commit 7877f89 into mainNov 10, 2022
@broccolinisoup
broccolinisoup deleted the broccolinisoup/UnderlineNav-sign-off-remediations-3 branch November 10, 2022 01:38
@broccolinisoup
broccolinisoup temporarily deployed to github-pages November 10, 2022 01:40 Inactive
@primer-cssprimer-css mentioned this pull request Nov 10, 2022
Sign up for freeto 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.

3 participants

@broccolinisoup@ericwbailey@joshblack