fix(CollectionControl): a11y drag handle and vertical axis lock#39863
fix(CollectionControl): a11y drag handle and vertical axis lock#39863
Conversation
dnd-kit attributes (which include role="button", tabIndex, and aria-roledescription) were spread onto a <span> wrapping an <Icon role="img">, producing a button-role span containing an img-role child — a confusing a11y tree that's not natively keyboard-activatable. - Render the drag handle as <button type="button"> via setActivatorNodeRef so the activator (button) is distinct from the drop node, and gets proper button semantics + keyboard activation. Move the aria-label to the button and mark the icon aria-hidden. - Add restrictToVerticalAxis modifier so dragged items can't drift horizontally (the original react-sortable-hoc implementation used lockAxis="y"). Adds @dnd-kit/modifiers (peer-compatible with @dnd-kit/core 6.3+). - Add KeyboardSensor with sortableKeyboardCoordinates so the list can be reordered via keyboard.
Code Review Agent Run #609b51Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
dosubot
Bot
added
change:frontend
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #39863 +/- ##
=======================================
Coverage 64.36% 64.37%
=======================================
Files 2569 2569
Lines 134745 134746 +1
Branches 31278 31278
=======================================
+ Hits 86732 86738 +6
+ Misses 46515 46510 -5
Partials 1498 1498
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
rusackas
left a comment
rusackas
left a comment
There was a problem hiding this comment.
Approving two small nits if you want to address them in this PR or follow-up:
(1) the hand-rolled with reset CSS could be a Button type="text" from @superset-ui/core/components and pick up theme focus styles for free
(2) a quick RTL test asserting aria-label="Drag to reorder" on the activator would close the Codecov gap on the new keyboard branch.
alexandrusoare
left a comment
alexandrusoare
left a comment
There was a problem hiding this comment.
LGTM with just 1 nit
| })} | ||
| > | ||
| <span {...attributes} {...listeners}> | ||
| <button |
There was a problem hiding this comment.
Can we use the button from AntDesign here
|
No, AntDesign's Button component isn't used in this Superset project. The code uses a custom HTML button with CSS for drag handles, consistent with the project's UI patterns. |
3 participants
SUMMARY
Follow-up to #38563.
dnd-kit's
attributes(which includerole="button",tabIndex={0},aria-roledescription) were spread onto a<span>wrapping an<Icon role="img">, producing a button-role span containing an img-role child — a confusing a11y tree that isn't natively keyboard-activatable. The original (pre-React-18) implementation also usedlockAxis="y"; the new dnd-kit version had no axis lock, so dragged items could drift horizontally.<button type="button">viasetActivatorNodeRefso the activator (button) is distinct from the drop node and gets proper button semantics + native keyboard activation. Movearia-label="Drag to reorder"to the button; mark the iconaria-hidden.restrictToVerticalAxismodifier to lock dragging to the y-axis (requires@dnd-kit/modifiers, peer-compatible with@dnd-kit/core6.3+).KeyboardSensorwithsortableKeyboardCoordinatesso the list can be reordered via keyboard.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A — no visual change in the rest state. Drag now stays vertical and the handle is keyboard-activatable.
TESTING INSTRUCTIONS
CollectionControl(e.g. annotation layer list, custom controls list).ADDITIONAL INFORMATION
@dnd-kit/modifiers