Merge pull request #1657 from atlassian/distance-from-draggable-origin

Fix the distance calculation to support varied lists
atlassian · Dec 4, 2019 · 5f43206 · 5f43206
2 parents 0388fec + 73aecc8
commit 5f43206
Show file tree

Hide file tree

Showing 2 changed files with 66 additions and 39 deletions.
diff --git a/src/state/get-droppable-over.js b/src/state/get-droppable-over.js
@@ -9,7 +9,7 @@ import type {
 } from '../types';
 import { toDroppableList } from './dimension-structures';
 import isPositionInFrame from './visibility/is-position-in-frame';
-import { distance } from './position';
+import { distance, patch } from './position';
 import isWithin from './is-within';
 
 // https://stackoverflow.com/questions/306316/determine-if-two-rectangles-overlap-each-other
@@ -33,17 +33,40 @@ type WithDistance = {|
   distance: number,
   id: DroppableId,
 |};
-function getFurthestAway(
-  home: DroppableDimension,
-  droppables: DroppableDimension[],
-): ?DroppableId {
-  const center: Position = home.page.borderBox.center;
-
-  const sorted: WithDistance[] = droppables
-    .map((item: DroppableDimension): WithDistance => {
+
+type GetFurthestArgs = {|
+  pageBorderBox: Rect,
+  draggable: DraggableDimension,
+  candidates: DroppableDimension[],
+|};
+
+function getFurthestAway({
+  pageBorderBox,
+  draggable,
+  candidates,
+}: GetFurthestArgs): ?DroppableId {
+  // We are not comparing the center of the home list with the target list as it would
+  // give preference to giant lists
+
+  // We are measuring the distance from where the draggable started
+  // to where it is *hitting* the candidate
+  // Note: The hit point might technically not be in the bounds of the candidate
+
+  const startCenter: Position = draggable.page.borderBox.center;
+  const sorted: WithDistance[] = candidates
+    .map((candidate: DroppableDimension): WithDistance => {
+      const axis: Axis = candidate.axis;
+      const target: Position = patch(
+        candidate.axis.line,
+        // use the current center of the dragging item on the main axis
+        pageBorderBox.center[axis.line],
+        // use the center of the list on the cross axis
+        candidate.page.borderBox.center[axis.crossAxisLine],
+      );
+
       return {
-        id: item.descriptor.id,
-        distance: distance(center, item.page.borderBox.center),
+        id: candidate.descriptor.id,
+        distance: distance(startCenter, target),
       };
     })
     // largest value will be first
@@ -127,6 +150,9 @@ export default function getDroppableOver({
   // Multiple options returned
   // Should only occur with really large items
   // Going to use fallback: distance from home
-  const home: DroppableDimension = droppables[draggable.descriptor.droppableId];
-  return getFurthestAway(home, candidates);
+  return getFurthestAway({
+    pageBorderBox,
+    draggable,
+    candidates,
+  });
 }
diff --git a/test/unit/state/get-droppable-over/preferencing.spec.js b/test/unit/state/get-droppable-over/preferencing.spec.js
@@ -14,7 +14,7 @@ import getDroppableOver from '../../../../src/state/get-droppable-over';
 import { toDroppableMap } from '../../../../src/state/dimension-structures';
 import { afterCrossAxisPoint } from '../../../util/after-point';
 
-const droppableLarge: DroppableDimension = getDroppableDimension({
+const droppableOrigin: DroppableDimension = getDroppableDimension({
   descriptor: {
     id: 'large',
     type: 'standard',
@@ -28,7 +28,7 @@ const droppableLarge: DroppableDimension = getDroppableDimension({
   },
 });
 
-const droppableSmall: DroppableDimension = getDroppableDimension({
+const droppableFirst: DroppableDimension = getDroppableDimension({
   descriptor: {
     id: 'small',
     type: 'standard',
@@ -42,7 +42,7 @@ const droppableSmall: DroppableDimension = getDroppableDimension({
   },
 });
 
-const droppableSecondary: DroppableDimension = getDroppableDimension({
+const droppableSecond: DroppableDimension = getDroppableDimension({
   descriptor: {
     id: 'secondary',
     type: 'standard',
@@ -52,20 +52,21 @@ const droppableSecondary: DroppableDimension = getDroppableDimension({
     top: 1000,
     left: 1200,
     right: 1300,
-    bottom: 1100,
+    // This is really tall to test the distance calculation against varied lists
+    bottom: 8000,
   },
 });
 
-const droppableTertiary: DroppableDimension = getDroppableDimension({
+const droppableThird: DroppableDimension = getDroppableDimension({
   descriptor: {
     id: 'tertiary',
     type: 'standard',
     mode: 'standard',
   },
   borderBox: {
     top: 1000,
-    left: 1100,
-    right: 1200,
+    left: 1400,
+    right: 1500,
     bottom: 1100,
   },
 });
@@ -74,10 +75,10 @@ const draggable: DraggableDimension = getDraggableDimension({
   descriptor: {
     id: 'my draggable',
     index: 0,
-    type: droppableLarge.descriptor.type,
-    droppableId: droppableLarge.descriptor.id,
+    type: droppableOrigin.descriptor.type,
+    droppableId: droppableOrigin.descriptor.id,
   },
-  borderBox: droppableLarge.client.borderBox,
+  borderBox: droppableOrigin.client.borderBox,
 });
 
 /**
@@ -86,28 +87,28 @@ const draggable: DraggableDimension = getDraggableDimension({
  */
 it('should prefer the furthest away droppable when multiple lists are hit', () => {
   const offset = getOffsetForCrossAxisEndEdge({
-    crossAxisEndEdgeOn: droppableTertiary.page.borderBox.center,
+    crossAxisEndEdgeOn: droppableThird.page.borderBox.center,
     dragging: draggable.page.borderBox,
-    axis: droppableTertiary.axis,
+    axis: droppableThird.axis,
   });
 
   const pageBorderBox: Rect = offsetRectByPosition(
     draggable.page.borderBox,
-    afterCrossAxisPoint(droppableTertiary.axis, offset),
+    afterCrossAxisPoint(droppableThird.axis, offset),
   );
 
   const result = getDroppableOver({
     pageBorderBox,
     draggable,
     droppables: toDroppableMap([
-      droppableLarge,
-      droppableSmall,
-      droppableSecondary,
-      droppableTertiary,
+      droppableOrigin,
+      droppableFirst,
+      droppableSecond,
+      droppableThird,
     ]),
   });
 
-  expect(result).toEqual(droppableTertiary.descriptor.id);
+  expect(result).toEqual(droppableThird.descriptor.id);
 });
 
 /**
@@ -116,26 +117,26 @@ it('should prefer the furthest away droppable when multiple lists are hit', () =
  */
 it('should prefer the second furthest away droppable when multiple lists are hit', () => {
   const offset = getOffsetForCrossAxisEndEdge({
-    crossAxisEndEdgeOn: droppableSecondary.page.borderBox.center,
+    crossAxisEndEdgeOn: droppableSecond.page.borderBox.center,
     dragging: draggable.page.borderBox,
-    axis: droppableSecondary.axis,
+    axis: droppableSecond.axis,
   });
 
   const pageBorderBox: Rect = offsetRectByPosition(
     draggable.page.borderBox,
-    afterCrossAxisPoint(droppableSecondary.axis, offset),
+    afterCrossAxisPoint(droppableSecond.axis, offset),
   );
 
   const result = getDroppableOver({
     pageBorderBox,
     draggable,
     droppables: toDroppableMap([
-      droppableLarge,
-      droppableSmall,
-      droppableSecondary,
-      droppableTertiary,
+      droppableOrigin,
+      droppableFirst,
+      droppableSecond,
+      droppableThird,
     ]),
   });
 
-  expect(result).toEqual(droppableSecondary.descriptor.id);
+  expect(result).toEqual(droppableSecond.descriptor.id);
 });