chore: Split presentational and logical parts of internal drag handle component
Description
Big refactor. We previously had a "drag handle button" (which was a basic button with props for pointer/keypress handlers), and a "drag handle wrapper" (which tracked focus states, showed and hid buttons, etc). This turns all the components into simple presentational ones and moves all the wrapper logic into a hook. This refactor will open the door in the future to allow for multiple technical approaches with a simple component.
// Migrating the current logic without major refactors, it looks like this.
const { wrapperProps } = useDefaultDragBehavior(props);
return <DragHandleWrapper {...wrapperProps}><DragHandleButton ... /></DragHandleWrapper>
But in the future, we can support multiple different approaches to dragging, so this is a stepping stone to logic in the future that would look like:
/* HYPOTHETICAL CODE! */
// For a button and slider approach, à la table column resizer
// The hook would be able to automatically track and handle focus between the two buttons
const { wrapperProps, dragButtonProps, sliderButtonProps } = useButtonAndSliderDragBehavior(props);
return (
<DragHandleWrapper {...wrapperProps}>
{/* role="button" that listens for an onClick */}
<DragHandleButton {...dragButtonProps} />
{/* role="slider" that is focused on keyboard activation */}
<DragHandleButton {...sliderButtonProps} />
</DragHandle>
);
// For a plain slider approach, split panel style
const { wrapperProps, dragButtonProps } = useFocusSliderDragBehavior(props);
return (
<DragHandleWrapper {...wrapperProps}>
{/* role="slider", nothing too complicated here */}
<DragHandleButton {...dragButtonProps} />
</DragHandle>
);
// Board components, sigh...
const { wrapperProps, dragButtonProps } = useTwoDimensionalDragBehavior({ ...props, onDragStart, onDragEnd });
return (
<DragHandleWrapper {...wrapperProps}>
{/* role="application" and all the associated handlers */}
<DragHandleButton {...dragButtonProps} />
</DragHandle>
);
The "wrapper" as a concept still exists to pass props down to the tooltip or the direction buttons, so it's now basically a compositional convenience:
/* HIGH-LEVEL ABSTRACT CODE! */
<>
<div ref={ref}>{button}</div>
<Tooltip trackRef={ref}
<PortalOverlay track={ref}>
<DirectionButton direction="..." />
<DirectionButton direction="..." />
<DirectionButton direction="..." />
<DirectionButton direction="..." />
</PortalOverlay>
</>
See the internal document ztdXAfqVYW3T for more information on dragging interactions.
TODO: Write more.
Related links, issue #, if available: n/a
How has this been tested?
The tests are already there, and since this is a refactor, nothing should break.
Review checklist
The following items are to be evaluated by the author(s) and the reviewer(s).
Correctness
- Changes include appropriate documentation updates.
- Changes are backward-compatible if not indicated, see
CONTRIBUTING.md. - Changes do not include unsupported browser features, see
CONTRIBUTING.md. - Changes were manually tested for accessibility, see accessibility guidelines.
Security
- If the code handles URLs: all URLs are validated through the
checkSafeUrlfunction.
Testing
- Changes are covered with new/existing unit tests?
- Changes are covered with new/existing integration tests?
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Codecov Report
Attention: Patch coverage is 97.36842% with 1 line in your changes missing coverage. Please review.
Project coverage is 96.61%. Comparing base (
22deef7) to head (4251a6d). Report is 7 commits behind head on main.
| Files with missing lines | Patch % | Lines |
|---|---|---|
| ...ts/drag-handle/hooks/use-default-drag-behavior.tsx | 92.85% | 1 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## main #3651 +/- ##
==========================================
- Coverage 96.62% 96.61% -0.01%
==========================================
Files 817 818 +1
Lines 23804 23781 -23
Branches 8346 8345 -1
==========================================
- Hits 23000 22977 -23
Misses 750 750
Partials 54 54
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
- :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.