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

Use button instead of span for disabled prev/next controls #3277

Conversation

adrianababakanian
Copy link
Contributor

Describe your changes here.

Per accessibility audit feedback, the current spans used for the prev/next links when disabled are not announcable by screen readers. Instead of spans, we should use buttons so that these disabled elements are focusable.

I also updated the CSS to match the text styling of the buttons to the same styling used when the links aren't disabled.

Screenshots

Please provide before/after screenshots for any visual changes

Before:
Screenshot 2023-05-12 at 5 00 01 PM

After:
Screenshot 2023-05-12 at 4 59 22 PM

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.

@adrianababakanian adrianababakanian requested review from a team and colebemis May 12, 2023 21:01
@changeset-bot
Copy link

changeset-bot bot commented May 12, 2023

🦋 Changeset detected

Latest commit: 7f5cb41

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

@adrianababakanian adrianababakanian temporarily deployed to github-pages May 12, 2023 21:07 — with GitHub Actions Inactive
Copy link
Contributor

@colebemis colebemis left a comment

Choose a reason for hiding this comment

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

Thank you 💖

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