fix: Make truncated filename tooltip keyboard accessible (file-token-group)#4157
fix: Make truncated filename tooltip keyboard accessible (file-token-group)#4157
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4157 +/- ##
==========================================
- Coverage 97.19% 97.19% -0.01%
==========================================
Files 886 886
Lines 25996 26005 +9
Branches 9415 9418 +3
==========================================
+ Hits 25267 25275 +8
- Misses 723 724 +1
Partials 6 6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Received translations from i18n package (CR-252051089) and merged them into main of AWS-UI-Components-I18n |
src/file-token-group/file-token.tsx
Outdated
| checkTruncation(); | ||
|
|
||
| // Check on window resize | ||
| window.addEventListener('resize', checkTruncation); |
There was a problem hiding this comment.
There are things other than a full window resize that can cause the truncation to trigger (opening panels, content changes, layout updates) so a ResizeObserver (we have a useResizeObserver) might be more "correct" here. But I think it's fine — the max width of the token is 230px, it's probably very unlikely that the component will be placed in a layout that narrow, so there's no need to pay that performance tax unless we have to.
There was a problem hiding this comment.
I would recommend to switch to the resize observer as it has a better performance than the resize event
There was a problem hiding this comment.
Done! Switched to useResizeObserver to observe the filename container element directly. This gives us better performance and also handles container-level resizes (panels, layout changes) that the window resize event would miss.
| const wrapper = render({ items: [{ file: file3 }], alignment: 'horizontal' }); | ||
|
|
||
| fireEvent.focus(wrapper.findFileToken(1)!.findFileName().getElement()); | ||
| expect(document.querySelector(`.${tooltipStyles.root}`)).not.toBeNull(); |
There was a problem hiding this comment.
should you check the content of the tooltip instead of 'not null'?
There was a problem hiding this comment.
Good point! Updated the assertions to verify the tooltip displays the correct file name.
|
|
||
| expect(wrapper.findFileToken(1)!.findRemoveButton().getElement()).toHaveFocus(); | ||
| // After dismissal, focus moves to the filename container (first focusable element in the token) | ||
| const fileNameElement = wrapper.findFileToken(1)!.findFileName().getElement().parentElement; |
There was a problem hiding this comment.
[non-blocking]: I'd be easier to search by the container classname - file-name-button
There was a problem hiding this comment.
Done! Updated to use the file-name-container classname selector instead of parentElement.
src/file-token-group/file-token.tsx
Outdated
| > | ||
| <InternalSpaceBetween direction="vertical" size="xxxs"> | ||
| <div | ||
| className={styles['file-name-button']} |
There was a problem hiding this comment.
[nit] "button" in the classname is a bit misleading. I'd suggest to rename it to file-name-container instead
There was a problem hiding this comment.
Done! Renamed to file-name-container
|
|
||
| fireEvent.mouseEnter(wrapper.findFileToken(1)!.findFileName().getElement()); | ||
| expect(document.querySelector(`.${tooltipStyles.root}`)).not.toBeNull(); | ||
| expect(document.querySelector(`.${tooltipStyles.root}`)).toHaveTextContent(file3.name); |
There was a problem hiding this comment.
[non-blocking, could be a follow-up] Should you use test wrapper instead such as createWrapper().findTooltip()?
Description
Adds keyboard accessibility to the file token tooltip. The filename container is now focusable (tabIndex={0}) with onFocus/onBlur handlers, allowing keyboard users to view the full filename when text is truncated.
Related links, issue #, if available:
AWSUI-61600How 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.