-
Notifications
You must be signed in to change notification settings - Fork 129
chore(ToggleSwitch): add aria-label to preview stories #3869
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
chore(ToggleSwitch): add aria-label to preview stories #3869
Conversation
|
Add default aria-label to ToggleSwitch component for accessibility.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Adds a default accessible name to the Primer::Alpha::ToggleSwitch button so it no longer renders without an ARIA name.
Changes:
- Introduces a default accessible name (
"toggle switch") when noaria-labelis provided. - Adds
aria-labelinto the merged button ARIA attributes alongsidearia-pressed.
Comments suppressed due to low confidence (1)
app/components/primer/alpha/toggle_switch.rb:82
- This change introduces new accessibility behavior (default
aria-label+ preserving any custom label). There are existing component tests for ToggleSwitch, but none currently assert the rendered button has an accessible name; adding/adjusting a test to cover the default label and that a customaria-labelis not overwritten would prevent regressions.
aria: merge_aria(
@system_arguments,
aria: { pressed: on?, label: @accessible_name}
)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@francinelucca I've opened a new pull request, #3870, to work on those changes. Once the pull request is ready, I'll request review from you. |
Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: francinelucca <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
|
@copilot open a new pull request against the main branch to apply changes based on the comments in this thread |
|
@francinelucca I've opened a new pull request, #3871, to work on those changes. Once the pull request is ready, I'll request review from you. |
TylerJDev
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you were planning on adding changes, so approving in advance!
Authors: Please fill out this form carefully and completely.
Reviewers: By approving this Pull Request you are approving the code change, as well as its deployment and mitigation plans.
Please read this description carefully. If you feel there is anything unclear or missing, please ask for updates.
What are you trying to accomplish?
Adds aria-label prop to preview ToggleSwich stories
Screenshots
Before:

(no aria name on button)
After:

(aria name on button)
Integration
N/A
List the issues that this change affects.
Closes https://github.com/github/accessibility-audits/issues/14754
Risk Assessment
What approach did you choose and why?
Confirmed the aria-label can be provided through system props, so just added aria-labels to preview stories to remediate the a11y issue.
Anything you want to highlight for special attention from reviewers?
Accessibility
Merge checklist
Take a look at the What we look for in reviews section of the contributing guidelines for more information on how we review PRs.