Skip to content

Commit

Permalink
Improve vertical sort algorithm to better handle arbitrary gaps.
Browse files Browse the repository at this point in the history
The CSS gap directive on flex layouts does not use margins for example.
As a result, the previous implementation would fail to correctly
position the sorted elements as it didn't properly account for the gap
size. Instead, explicitly calculate the gap from the layout data when
sorting. The algorithm is approximately:

If the current item is the active draggable
  If it was moved over a droppable that is later in the vertical list
    Move it down by the delta between its bottom edge and the droppable
    bottom edge.
  Else it was moved earlier in the vertical list so
    Move it up by the delta between its top edge and the droppable top
    edge.
Else
  If the draggable was moved above the current item (shifting it down)
    Move it down by the delta of the draggable height plus any gap that
    was between the draggable and the item previously before it.
  Else the current item is being shifted up so
    Move it up by the delta of the draggable height plus any gap that
    was between the draggable and the item previously after it.

Related to this, remove the now redundant recording of `outerHeight` and
`outerWidth` in `elementLayout()`.
  • Loading branch information
martinpengellyphillips committed Jul 12, 2021
1 parent af598b4 commit ad4504d
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 29 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@ const MyComponent = (props) => {
`transform`.
- Assume valid `transform` value is always present. In particular,
`transformStyle` no longer checks for a `null` transform value.
- Improve vertical sort algorithm to better handle arbitrary gaps (such as those
created by the CSS gap property on flexbox). As part of this, remove the now
redundant `outerHeight` and `outerWidth` properties from the layout data
returned by `elementLayout`.
- Update `README.md` to reflect simpler directive interface for draggables and
droppables.

Expand Down
43 changes: 25 additions & 18 deletions src/create-sortable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,32 +21,39 @@ export const createSortable = (options) => {

const transform = () => {
const delta = noopTransform();
const resolvedInitialIndex = initialIndex();
const resolvedCurrentIndex = currentIndex();

if (!anyDraggableActive() || currentIndex() === initialIndex()) {
if (
!anyDraggableActive() ||
resolvedCurrentIndex === resolvedInitialIndex
) {
return delta;
}

const activeDraggableId = dndState.active.draggable;
const activeDraggableLayout = layoutById({ id: activeDraggableId });
const activeDraggableInitialIndex =
sortableState.initialIds.indexOf(activeDraggableId);
const draggableId = dndState.active.draggable;
const draggableInitialIndex = sortableState.initialIds.indexOf(draggableId);
const draggableLayout = layoutById({ id: draggableId });

if (draggable.isActiveDraggable) {
const activeDroppableId = dndState.active.droppable;
const activeDroppableLayout = layoutById({ id: activeDroppableId });
const activeDroppableInitialIndex =
sortableState.initialIds.indexOf(activeDroppableId);
delta.y =
activeDroppableInitialIndex > activeDraggableInitialIndex
? activeDroppableLayout.y +
activeDroppableLayout.outerHeight -
(activeDraggableLayout.y + activeDraggableLayout.outerHeight)
: activeDroppableLayout.y - activeDraggableLayout.y;
const droppableId = dndState.active.droppable;
const droppableLayout = layoutById({ id: droppableId });
if (resolvedCurrentIndex > resolvedInitialIndex) {
delta.y = droppableLayout.bottom - draggableLayout.bottom;
} else {
delta.y = droppableLayout.top - draggableLayout.top;
}
} else {
if (activeDraggableInitialIndex > initialIndex()) {
delta.y += activeDraggableLayout.outerHeight;
if (resolvedCurrentIndex > resolvedInitialIndex) {
const leadingId = sortableState.initialIds[draggableInitialIndex - 1];
const leadingLayout = layoutById({ id: leadingId });
const leadingGap = draggableLayout.top - leadingLayout.bottom;
delta.y += draggableLayout.height + leadingGap;
} else {
delta.y -= activeDraggableLayout.outerHeight;
const trailingId = sortableState.initialIds[draggableInitialIndex + 1];
const trailingLayout = layoutById({ id: trailingId });
const trailingGap = trailingLayout.top - draggableLayout.bottom;
delta.y -= draggableLayout.height + trailingGap;
}
}

Expand Down
11 changes: 0 additions & 11 deletions src/layout.ts
Original file line number Diff line number Diff line change
@@ -1,22 +1,11 @@
export const elementLayout = ({ element }) => {
const { x, y, width, height } = element.getBoundingClientRect();
const style = window.getComputedStyle(element);

const outerHeight = ["top", "bottom"]
.map((side) => parseInt(style.getPropertyValue(`margin-${side}`)))
.reduce((total, side) => total + side, height);

const outerWidth = ["left", "right"]
.map((side) => parseInt(style.getPropertyValue(`margin-${side}`)))
.reduce((total, side) => total + side, width);

return {
x,
y,
width,
height,
outerWidth,
outerHeight,
get left() {
return this.x;
},
Expand Down

0 comments on commit ad4504d

Please sign in to comment.