-
Notifications
You must be signed in to change notification settings - Fork 646
Fixes issue with Tooltip description id overriding existing description ids #6200
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
Conversation
🦋 Changeset detectedLatest commit: 02ede7d The changes in this PR will be included in the next version bump. 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 |
|
👋 Hi, this pull request contains changes to the source code that github/github depends on. If you are GitHub staff, we recommend testing these changes with github/github using the integration workflow. Thanks! |
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
This PR ensures that when using the Tooltip in description mode, any existing aria-describedby values on the trigger are preserved and the tooltip’s ID is appended, rather than replaced.
- Append tooltip ID to existing
aria-describedbyinstead of overriding it - Add a unit test and a story to validate the merged description behavior
- Update the changelog with a patch release entry
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/react/src/TooltipV2/tests/Tooltip.test.tsx | Added test component and case for existing aria-describedby |
| packages/react/src/TooltipV2/Tooltip.tsx | Changed aria-describedby logic to append tooltip ID |
| packages/react/src/TooltipV2/Tooltip.features.stories.tsx | Added a feature story demonstrating external description usage |
| .changeset/two-aliens-wait.md | Documented patch release for this change |
size-limit report 📦
|
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Relates to https://github.com/github/accessibility-audits/issues/10890
Blocks rollout of this PR: https://github.com/github/github/pull/383309
Changelog
N/A
New
Changed
The TooltipV2 no longer clears the existing
aria-describedbyproperty when using description type.Removed
Rollout strategy
Testing & Reviewing
It seems like the test I added isn't run using
npm run test- I think because it's in the Ignore Modules list. I managed to run it with the following command:Merge checklist