TextInput: Refactor TextInputInnerAction to use the default icon button tooltip
#4733
Conversation
🦋 Changeset detectedLatest commit: 69244dd 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 |
broccolinisoup
temporarily deployed
to
github-pages
GitHub Actions
Inactive
size-limit report 📦
|
| {icon && !children && ariaLabel ? ( | ||
| <IconButton | ||
| {...accessibleLabel} | ||
| tooltipDirection={tooltipDirection ?? 's'} |
There was a problem hiding this comment.
Sparkle @joshblack for helping me fix this obscure type issue ✨
broccolinisoup
changed the title
TextInputInnerAction to use the default icon button tooltip
siddharthkp
left a comment
siddharthkp
left a comment
There was a problem hiding this comment.
1 clarifying question, LGTM other than that
Approving in advance!
| } | ||
|
|
||
| const accessibleLabel = ariaLabel | ||
| ? {'aria-label': ariaLabel} |
There was a problem hiding this comment.
This gives priority to aria-label over aria-labelledby which is opposite of what browsers do. Is that intentional?
aria-labelledby takes precedence over all other methods of providing an accessible name, including aria-label
Ref: https://arc.net/l/quote/ksytdmqp
There was a problem hiding this comment.
Yes, kind of.
Because aria-label is a prop on the TextInputInnerAction component and it is used to provide tooltip text as well.
If there is aria-labelledby, the tooltip wiring needs to be manually. So to my understanding, aria-label is encouraged and preferred.
Let me know if you have any concerns or anything I am missing here.
Closes #4696
Changelog
Changed
IconButtonin theTextInputInnerActioncomponent to have the right a11y props (aria-labelandaria-labelledby) and addtooltipDirectionto specify the direction of the tooltipRemoved
IconButtonin theTextInputInnerActionRollout strategy
Testing & Reviewing
Merge checklist