-
Notifications
You must be signed in to change notification settings - Fork 164
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
feat: Show popover on truncated breadcrumb text #527
Conversation
Codecov ReportBase: 92.52% // Head: 92.38% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #527 +/- ##
==========================================
- Coverage 92.52% 92.38% -0.15%
==========================================
Files 567 569 +2
Lines 16134 16218 +84
Branches 4414 4433 +19
==========================================
+ Hits 14928 14983 +55
- Misses 1126 1151 +25
- Partials 80 84 +4
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
if (!textWidth || !virtualTextRef || !virtualTextRef.current) { | ||
return null; | ||
} | ||
const virtualTextWidth = virtualTextRef.current.getBoundingClientRect().width; |
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 attempt to fix that comment (#527 (comment)) is a step backwards. Now the height is calculated on every render, not even on an effect update.
An expected implementation would be like this
onMouseEnter={() => isTruncated(textRef.current, virtualTextRef.current) && setShowPopover(true)}
This way the size check is only happening on the exact event and not anywhere else
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.
Now I get it. Thank you so much for the example!
Description
Show popover on focus/hover on truncated text in breadcrumb.
Dismiss popover on blur/mouseleave/escape.
Related links, issue AWSUI-19121
How has this been tested?
Added integration test.Review checklist
The following items are to be evaluated by the author(s) and the reviewer(s).
Correctness
CONTRIBUTING.md
.CONTRIBUTING.md
.Security
checkSafeUrl
function.Testing
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.