fix(IconButton): respect consumer-supplied aria-label#1140
Open
munzzyy wants to merge 2 commits into
Open
Conversation
aria-label was set after the {...props} spread, so it always
overrode a consumer-supplied aria-label with the raw icon token
name (e.g. 'trash', 'sparkle'). Screen readers announced the
icon's internal name instead of the button's actual purpose.
Now a consumer aria-label wins, falling back to the icon name
only when none is given. Added tests covering both paths.
🦋 Changeset detectedLatest commit: 26b903f The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
Reviewed by Cursor Bugbot for commit b84db50. Configure here.
… fallback The Icon rendered inside IconButton still exposed its own role="img" aria-label, so assistive tech announced two names for one control. Mark it aria-hidden, matching how Button and ButtonGroup already hide their icons. Also switch the aria-label fallback from ?? to ||, so an explicit empty aria-label="" falls back to the icon name instead of leaving the button nameless.
Author
|
Pushed a follow-up commit for both review points.
Added tests for both. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Why?
IconButtonsetsaria-labelto the raw icon token ("trash","sparkle", etc.) and there's no way to override it. The line is placed after{...props}in the render, so if you pass your ownaria-label, it gets silently thrown away and screen readers just announce the icon's internal name instead of what the button actually does. Closes #1056.How?
aria-label={iconName}toaria-label={props['aria-label'] ?? iconName}so a consumer's label wins and the icon name is only the fallbackaria-labelnow actually sticksTickets?
Contribution checklist?
buildcommand runs locallySecurity checklist?
dangerouslySetInnerHTMLPreview?
N/A — one-line prop precedence fix, no visual change.