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

Toggle switch a11y take3 #3510

Merged
merged 8 commits into from
Jul 19, 2023
Merged

Toggle switch a11y take3 #3510

merged 8 commits into from
Jul 19, 2023

Conversation

camertron
Copy link
Collaborator

This PR addresses the accessibility feedback for the ToggleSwitch component, outlined in this issue: https://github.com/github/primer/issues/1867. It is a re-submission of #3251 and #3433 which had to be reverted. See #3429 for details.

Namely it:

  1. Removes role=switch from the button.
  2. Uses aria-pressed in favor of aria-checked.
  3. Makes the status label (on/off) clickable.

Screenshots

No visual changes

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.

* Address ToggleSwitch a11y feedback

* Remove unused import

* Add changeset

* Fix tests
@changeset-bot
Copy link

changeset-bot bot commented Jul 10, 2023

🦋 Changeset detected

Latest commit: 1087281

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 Minor

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 Jul 10, 2023

size-limit report 📦

Path Size
dist/browser.esm.js 102.73 KB (-0.02% 🔽)
dist/browser.umd.js 103.27 KB (-0.02% 🔽)

@camertron camertron temporarily deployed to github-pages July 10, 2023 17:17 — with GitHub Actions Inactive
@github-actions github-actions bot temporarily deployed to storybook-preview-3510 July 10, 2023 17:17 Inactive
@camertron camertron temporarily deployed to github-pages July 11, 2023 23:08 — with GitHub Actions Inactive
@github-actions github-actions bot temporarily deployed to storybook-preview-3510 July 11, 2023 23:08 Inactive
@camertron camertron temporarily deployed to github-pages July 12, 2023 21:33 — with GitHub Actions Inactive
@github-actions github-actions bot temporarily deployed to storybook-preview-3510 July 12, 2023 21:33 Inactive
@camertron camertron temporarily deployed to github-pages July 12, 2023 22:12 — with GitHub Actions Inactive
@github-actions github-actions bot temporarily deployed to storybook-preview-3510 July 12, 2023 22:12 Inactive
@camertron camertron marked this pull request as ready for review July 15, 2023 05:45
@camertron camertron requested review from a team and broccolinisoup July 15, 2023 05:45
@camertron camertron temporarily deployed to github-pages July 15, 2023 05:53 — with GitHub Actions Inactive
@github-actions github-actions bot temporarily deployed to storybook-preview-3510 July 15, 2023 05:53 Inactive
Copy link
Member

@broccolinisoup broccolinisoup left a comment

Choose a reason for hiding this comment

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

@camertron I assume with merging main after #3520 resolves the failing tests at dotcom?

@camertron
Copy link
Collaborator Author

@broccolinisoup yes, that's correct 👍

Copy link
Member

@broccolinisoup broccolinisoup left a comment

Choose a reason for hiding this comment

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

🚀

@camertron camertron added this pull request to the merge queue Jul 19, 2023
Merged via the queue into main with commit bdbcfd1 Jul 19, 2023
@camertron camertron deleted the toggle_switch_a11y_take3 branch July 19, 2023 18:15
@primer-css primer-css mentioned this pull request Jul 19, 2023
broccolinisoup added a commit that referenced this pull request Jul 21, 2023
broccolinisoup added a commit that referenced this pull request Jul 25, 2023
@broccolinisoup broccolinisoup mentioned this pull request Jul 25, 2023
4 tasks
github-merge-queue bot pushed a commit that referenced this pull request Jul 27, 2023
…3545)

* Add deprecated notice and run console warn only in DEV

* run when the env is not test

* Revert "Toggle switch a11y take3 (#3510)"

This reverts commit bdbcfd1.

* take the list changes back for underlinenav

* lint and test fix

* Add back "Toggle switch a11y take3 (#3510)"

This reverts commit 5f94786.

* Revert "take the list changes back for underlinenav"

This reverts commit 843e304.

* fix after revert commit

* remove console warning and update docs and changeset

* fix changeset and update prop docs
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