-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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: tooltip word break #18502
fix: tooltip word break #18502
Conversation
✅ Deploy Preview for v11-carbon-web-components ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for carbon-elements ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for v11-carbon-react ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #18502 +/- ##
==========================================
- Coverage 84.16% 84.04% -0.12%
==========================================
Files 408 408
Lines 14450 14470 +20
Branches 4666 4680 +14
==========================================
Hits 12162 12162
- Misses 2123 2141 +18
- Partials 165 167 +2 ☔ View full report in Codecov by Sentry. |
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.
LGTM 👍
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! Just a note about removing the test story.
export const Test = { | ||
render: () => html` | ||
<cds-tooltip align="bottom"> | ||
<button | ||
class="sb-tooltip-trigger" | ||
role="button" | ||
aria-labelledby="content"> | ||
${Information16()} | ||
</button> | ||
<cds-tooltip-content id="content"> | ||
gfsghfsfsgggggghgsghsghgtuurutcxcrtstshgstsyststshsststststtstsffsfsgssjdstysyhgwtjyrkjgrutrwqtwyfyhygfhyiijfioyfdsrtkwjiutwjhqgwiyusrqjfterhtwgyqurtwu | ||
</cds-tooltip-content> | ||
</cds-tooltip> | ||
`, | ||
}; | ||
|
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.
This can be removed, right?
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.
Yes this will be removed as mentioned in PR description.
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 to me! I think we can remove the test story before merging
ca5f92f
Closes #18161
Adds word-break support for tooltip content to handle long text without spaces
Changelog
Changed
word-break: break-word
to tooltip content styles to properly handle long text without spacesTesting / Reviewing
Steps to verify:
Go to Test story in tooltip word without space should not overflow , it should break.
Note- Test file will be deleted once the PR gets reviewed.