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

CounterLabel: Accessibility improvements for better screen reader announcing #2604

Merged
merged 4 commits into from
Nov 29, 2022

Conversation

broccolinisoup
Copy link
Member

@broccolinisoup broccolinisoup commented Nov 22, 2022

Closes https://github.com/github/primer/issues/1507

This PR brings improvement on the CounterLabel to make the counter data have an accessible name.

  • Refactored CounterLabel to abandon styled-components
  • Re-wrote unit tests with the user behavioural paradigm approach rather than checking styled-components specific implementation details
  • Add aria-hidden=true to the existing span (visual representation of the counter)
  • Add visually hidden span with extra spacing and parenthesis for screen readers to announce the counter more accessible.

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.

@broccolinisoup broccolinisoup requested review from a team and josepmartins November 22, 2022 06:18
@changeset-bot
Copy link

changeset-bot bot commented Nov 22, 2022

🦋 Changeset detected

Latest commit: d6259ba

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

This PR includes changesets to release 1 package
Name Type
@primer/react Patch

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

@github-actions
Copy link
Contributor

github-actions bot commented Nov 22, 2022

size-limit report 📦

Path Size
dist/browser.esm.js 78.95 KB (+0.04% 🔺)
dist/browser.umd.js 79.58 KB (+0.04% 🔺)

@github-actions github-actions bot temporarily deployed to storybook-preview-2604 November 22, 2022 06:26 Inactive
@broccolinisoup broccolinisoup temporarily deployed to github-pages November 22, 2022 06:31 Inactive
@github-actions github-actions bot temporarily deployed to storybook-preview-2604 November 22, 2022 06:31 Inactive
@broccolinisoup broccolinisoup marked this pull request as draft November 22, 2022 06:42
@broccolinisoup broccolinisoup temporarily deployed to github-pages November 22, 2022 06:59 Inactive
@github-actions github-actions bot temporarily deployed to storybook-preview-2604 November 22, 2022 06:59 Inactive
@broccolinisoup broccolinisoup temporarily deployed to github-pages November 22, 2022 07:18 Inactive
@github-actions github-actions bot temporarily deployed to storybook-preview-2604 November 22, 2022 07:18 Inactive
@broccolinisoup broccolinisoup marked this pull request as ready for review November 22, 2022 08:45
@broccolinisoup broccolinisoup temporarily deployed to github-pages November 24, 2022 03:39 Inactive
@github-actions github-actions bot temporarily deployed to storybook-preview-2604 November 24, 2022 03:39 Inactive
Copy link
Contributor

@josepmartins josepmartins left a comment

Choose a reason for hiding this comment

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

LGTM! 🚀

@josepmartins josepmartins temporarily deployed to github-pages November 29, 2022 10:19 Inactive
@broccolinisoup broccolinisoup merged commit 7e9e367 into main Nov 29, 2022
@broccolinisoup broccolinisoup deleted the counter-label-a11y-counters branch November 29, 2022 23:18
@primer-css primer-css mentioned this pull request Nov 29, 2022
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.

None yet

2 participants