feat: Add TruncatedText component#4543
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4543 +/- ##
=======================================
Coverage 97.42% 97.43%
=======================================
Files 938 941 +3
Lines 29680 29720 +40
Branches 10787 10798 +11
=======================================
+ Hits 28917 28957 +40
Misses 756 756
Partials 7 7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
avinashbot
requested review from
a team,
georgylobko and
jperals
and removed request for
a team and
georgylobko
| export default function TruncatedTextSimple() { | ||
| return ( | ||
| <ScreenshotArea> | ||
| <h1>TruncatedText examples</h1> |
There was a problem hiding this comment.
Minor: the page header should preferably be ourside the screenshot area
| <Box> | ||
| <Box variant="awsui-key-label">With CopyToClipboard, internal truncation</Box> | ||
| <div style={containerStyle}> | ||
| <CopyToClipboard |
There was a problem hiding this comment.
Is this wrapping after the icon expected because builders should instead wrap the CopyToClipboard inside TruncatedText, as in the previous element?
There was a problem hiding this comment.
It's not entirely expected; there's a parallel update to the component to support single-line wrapping; which has released since this PR was created. Will rebase.
There was a problem hiding this comment.
Still reproducible after updating the branch
There was a problem hiding this comment.
Works fine in the permutations page - just didn't update this one to add wrapText.
| if (element) { | ||
| // useResizeObserver fires initially at layoutEffect-time where the initial calculation | ||
| // is performed, but the calculation isn't always correct at that stage. | ||
| setTimeout(() => setIsTruncated(element.scrollWidth > element.clientWidth), 1); |
There was a problem hiding this comment.
Is there any alternative where arbitrary timeouts are not needed, such as requestAnimationFrame? We use requestAnimationFrame in 36 other places.
There was a problem hiding this comment.
I tried to use rAF, but it was still flaky. It's hard to tell at what point during rendering that wrapping/truncation happens, but it looks like every browser is a little unique in that regard, and this seemed to work the most consistently (timeout of 0 didn't work either).
| return ( | ||
| <ScreenshotArea> | ||
| <h1>TruncatedText examples</h1> | ||
| <SpaceBetween size="m"> |
There was a problem hiding this comment.
Note: permutations make the screenshots more isolated, and handle future additions more gracefully
avinashbot
force-pushed
the
feat/truncated-text-component
branch
from
275de6a to
876cd0b
Compare
| expect(element).toHaveTextContent('Some text'); | ||
| }); | ||
|
|
||
| test('applies the test-utils root class', () => { |
There was a problem hiding this comment.
Not blocking but I'd say this test is superfluous —what matters is that the test utils work. Which is kind of covered since renderTruncatedText relies on findTruncatedText, although not very explicitly.
2 participants
Description
Releases a new component, TruncatedText, based on tooltip, which handles things like truncation detection, correct
ARIA roles, and pointer and keyboard behavior.
Related links, issue #, if available: n/a
How has this been tested?
Unit tests, and a new visual test page for screenshot testing (will add pipeline test after merge)
Review checklist
The following items are to be evaluated by the author(s) and the reviewer(s).
Correctness
CONTRIBUTING.md.CONTRIBUTING.md.Security
checkSafeUrlfunction.Testing
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.