feat: File token group [internal] #2950
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2950 +/- ##
==========================================
+ Coverage 96.27% 96.30% +0.02%
==========================================
Files 769 771 +2
Lines 21764 21812 +48
Branches 7038 7446 +408
==========================================
+ Hits 20954 21005 +51
+ Misses 802 799 -3
Partials 8 8 ☔ View full report in Codecov by Sentry. |
katiegeorge
force-pushed
the
kg/file-token-group-internal
branch
3 times, most recently
from
7dd2651
to
e9235ef
Compare
src/internal/components/file-token-group/__tests__/file-token-group.test.tsx
Show resolved
Hide resolved
src/internal/components/file-token-group/__tests__/file-token-group.test.tsx
Outdated
Show resolved
Hide resolved
katiegeorge
force-pushed
the
kg/file-token-group-internal
branch
from
6413910
to
92a8510
Compare
|
I found a regression. The current file tokens allow the text to wrap and are responsive even if the file name is long. The new tokens do not allow wrapping and are not properly responsive. The horizontal ones do use truncation but it only works when the component size does not change after rendering. Screen.Recording.2024-11-08.at.10.09.54.mov |
src/internal/components/file-token-group/__tests__/file-token-group.test.tsx
Outdated
Show resolved
Hide resolved
src/internal/components/file-token-group/__tests__/file-token-group.test.tsx
Show resolved
Hide resolved
| const fileNameContainerRef = useRef<HTMLDivElement>(null); | ||
| const [showTooltip, setShowTooltip] = useState(false); | ||
|
|
||
| function isEllipsisActive() { |
There was a problem hiding this comment.
Can we use css text-overflow for ellipsis instead?
There was a problem hiding this comment.
We do. We need this to determine when to show the tooltip, so it needs to be in javascript. Not sure if renaming this "isTooltipActive" would be more apt, since this is literally what its doing?
| * Specifies if the control is read-only, which prevents the | ||
| * user from modifying the value. A read-only control is still focusable. | ||
| */ | ||
| readOnly?: boolean; |
There was a problem hiding this comment.
Why do we have readOnly on the group level and disabled on the file level?
There was a problem hiding this comment.
Good catch, I decided to remove the disabled property for now since we don't have a use case. I'll remove that property. As an FYI, the reason one is on the group and one on the token is consistent with the token group.
| @use '../internal/styles/tokens' as awsui; | ||
| @use './constants' as constants; | ||
|
|
||
| @mixin token-box-styles { |
Description
This PR updates the file-upload component to use the newly refactored file token group. The only changes externally are adding a horizontal variant to the file upload.
This PR is part one of multiple for file upload decomposition, to make the pull requests easier to review.
Full plan:
File dropzone (internal)File input (internal)file-uploadvariant to button groupRelated links, issue #, if available: n/a
How has this been tested?
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.