-
Notifications
You must be signed in to change notification settings - Fork 138
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
fix: remove tooltip from tagoverflow #6463
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for ibm-products-web-components ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for carbon-for-ibm-products ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6463 +/- ##
==========================================
+ Coverage 79.59% 79.61% +0.01%
==========================================
Files 394 394
Lines 12871 12871
Branches 4268 4268
==========================================
+ Hits 10245 10247 +2
+ Misses 2626 2624 -2
|
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.
According to Codecov, I think we need to write some tests.
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.
Looks good, but found one glitch.
With this change we are unable to tab navigate through the tags. Since you removed Tooltip wrapper , removed button elements as well that made it not focusable i guess.
@amal-k-joy this appears to be working as intended as they are read only |
@makafsal coverage should be acceptable now 👍 |
Closes #5736
fixes an issue where the
Tag
component was being wrapped by abutton
fromTooltip
which was causing aninvalid dom nesting
error. since carbon tags includetitle
they should already be screen reader accessible when they are being truncated.