-
Notifications
You must be signed in to change notification settings - Fork 204
feat: Add UAP buttons to resizable table column headers #3892
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
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3892 +/- ##
==========================================
- Coverage 97.16% 97.15% -0.02%
==========================================
Files 850 850
Lines 24785 24872 +87
Branches 8735 8761 +26
==========================================
+ Hits 24082 24164 +82
+ Misses 696 659 -37
- Partials 7 49 +42 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| const resizerToggleRef = useRef<HTMLButtonElement>(null); | ||
| const resizerSeparatorRef = useRef<HTMLSpanElement>(null); | ||
|
|
||
| const [isPointerDown, setIsPointerDown] = useState(false); |
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.
Making a distinction between isPointerDown and isDragging because we want to support a "click" interaction. Currently, a mousedown starts a drag and a mouseup ends it, which means, if you click the divider slightly off center, it jumps before showing the buttons, which looks really janky. So with this, the column widths are only adjusted when the mouse starts moving.
| updateTrackerPosition, | ||
| ]); | ||
|
|
||
| useEffect(() => { |
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.
We added both isDragging and isPointerDown to our dependencies in the useEffect above, which means this timer get destroyed and created twice. Not a problem for real users, but this really screws with tests (since jest's time-skipping timer methods won't work if the timer is recreated).
| position: relative; | ||
| display: inline-block; | ||
| .contents { | ||
| display: contents; |
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.
Previously, the drag handle wrapper had three nested divs I had to contend with styling them all. Since two of those divs only exist for event handling/capture reasons, I can safely just set them to display: contents and only worry about styling one div.
| if (event.button !== 0) { | ||
| return; | ||
| } | ||
| event.preventDefault(); |
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.
Not sure why this behavior was there before, but clicking on the drag handle doesn't move focus to it. But now it does, like any regular button. Helps with simplifying the UAP logic, and is also nicer because you can switch between drag and keyboard.
3ec898d to
74517a2
Compare
fbb2479 to
e701a6b
Compare
Description
Screen.Recording.2025-09-26.at.11.53.37.mov
Related links, issue #, if available: AWSUI-60549
API changes
This PR adds a new property to the table's
ariaLabelsproperty:interface AriaLabels { ... + resizerTooltipText?: string; ... }Let's consider the 👍s on this as approval for the API changes.
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.