fix(IconButton): respect consumer-supplied aria-label - #1140
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: 791edba 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 |
|
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
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.
|
Pushed a follow-up commit for both review points.
Added tests for both. |
|
Both of these are already handled in the current revision. The inner |
There was a problem hiding this comment.
Pull request overview
Fixes IconButton accessibility behavior by ensuring a consumer-supplied aria-label is not silently overridden, improving the accessible name exposed to assistive technologies.
Changes:
- Update
IconButtonto prefer a passedaria-labeland fall back to the icon token when absent (including empty string). - Mark the inner
Iconasaria-hiddento keep the control’s accessible name focused on the button itself. - Add unit tests covering label precedence and fallback behavior; add a patch changeset.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/components/IconButton/IconButton.tsx | Adjusts aria-label precedence and hides the inner icon from assistive tech. |
| src/components/IconButton/IconButton.test.tsx | Adds tests for aria-label fallback/override and icon accessibility behavior. |
| .changeset/fix-iconbutton-aria-label-override.md | Documents the patch-level change for release notes/versioning. |
|
Thanks for the look. Nothing outstanding from my side, so I'll leave it with @ariser. |
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.