fix(interactions): recognise drag after 100ms and 4px (#848)

This fix reduces the situation where a drag event is fired instead of a click one. It adds the following thresholds: - a 100ms threshold before the mouse-down time and the current time to consider the movement for dragging purposes - a 4-pixel delta between mouse-down and the current position to consider the movement for dragging purposes fix #748
elastic · Oct 6, 2020 · 70626fe · 70626fe
1 parent d879e05
commit 70626fe
Show file tree

Hide file tree

Showing 4 changed files with 62 additions and 33 deletions.
diff --git a/integration/page_objects/common.ts b/integration/page_objects/common.ts
@@ -19,6 +19,7 @@
 
 import Url from 'url';
 
+import { DRAG_DETECTION_TIMEOUT } from '../../src/state/reducers/interactions';
 // @ts-ignore
 import defaults from '../defaults';
 import { toMatchImageSnapshot } from '../jest_env_setup';
@@ -246,6 +247,7 @@ class CommonPage {
     const { x: x1, y: y1 } = getCursorPosition(end, element);
     await page.mouse.move(x0, y0);
     await page.mouse.down();
+    await page.waitFor(DRAG_DETECTION_TIMEOUT);
     await page.mouse.move(x1, y1);
   }
 

diff --git a/src/chart_types/xy_chart/state/chart_state.interactions.test.ts b/src/chart_types/xy_chart/state/chart_state.interactions.test.ts
@@ -835,8 +835,8 @@ function mouseOverTestSuite(scaleType: XScaleType) {
       const end1 = { x: 75, y: 0 };
 
       store.dispatch(onMouseDown(start1, 0));
-      store.dispatch(onPointerMove(end1, 1));
-      store.dispatch(onMouseUp(end1, 3));
+      store.dispatch(onPointerMove(end1, 200));
+      store.dispatch(onMouseUp(end1, 300));
       if (scaleType === ScaleType.Ordinal) {
         expect(brushEndListener).not.toBeCalled();
       } else {
@@ -846,9 +846,9 @@ function mouseOverTestSuite(scaleType: XScaleType) {
       const start2 = { x: 75, y: 0 };
       const end2 = { x: 100, y: 0 };
 
-      store.dispatch(onMouseDown(start2, 4));
-      store.dispatch(onPointerMove(end2, 5));
-      store.dispatch(onMouseUp(end2, 6));
+      store.dispatch(onMouseDown(start2, 400));
+      store.dispatch(onPointerMove(end2, 500));
+      store.dispatch(onMouseUp(end2, 600));
       if (scaleType === ScaleType.Ordinal) {
         expect(brushEndListener).not.toBeCalled();
       } else {
@@ -858,9 +858,9 @@ function mouseOverTestSuite(scaleType: XScaleType) {
 
       const start3 = { x: 75, y: 0 };
       const end3 = { x: 250, y: 0 };
-      store.dispatch(onMouseDown(start3, 7));
-      store.dispatch(onPointerMove(end3, 8));
-      store.dispatch(onMouseUp(end3, 9));
+      store.dispatch(onMouseDown(start3, 700));
+      store.dispatch(onPointerMove(end3, 800));
+      store.dispatch(onMouseUp(end3, 900));
       if (scaleType === ScaleType.Ordinal) {
         expect(brushEndListener).not.toBeCalled();
       } else {
@@ -870,15 +870,24 @@ function mouseOverTestSuite(scaleType: XScaleType) {
 
       const start4 = { x: 25, y: 0 };
       const end4 = { x: -20, y: 0 };
-      store.dispatch(onMouseDown(start4, 10));
-      store.dispatch(onPointerMove(end4, 11));
-      store.dispatch(onMouseUp(end4, 12));
+      store.dispatch(onMouseDown(start4, 1000));
+      store.dispatch(onPointerMove(end4, 1100));
+      store.dispatch(onMouseUp(end4, 1200));
       if (scaleType === ScaleType.Ordinal) {
         expect(brushEndListener).not.toBeCalled();
       } else {
         expect(brushEndListener).toBeCalled();
         expect(brushEndListener.mock.calls[3][0]).toEqual({ x: [0, 0.5] });
       }
+
+      store.dispatch(onMouseDown(start4, 1300));
+      store.dispatch(onPointerMove(end4, 1390));
+      store.dispatch(onMouseUp(end4, 1400));
+      if (scaleType === ScaleType.Ordinal) {
+        expect(brushEndListener).not.toBeCalled();
+      } else {
+        expect(brushEndListener.mock.calls[4]).toBeUndefined();
+      }
     });
     test('can respond to a brush end event on rotated chart', () => {
       const brushEndListener = jest.fn<void, [XYBrushArea]>((): void => undefined);
@@ -907,8 +916,8 @@ function mouseOverTestSuite(scaleType: XScaleType) {
       const end1 = { x: 0, y: 75 };
 
       store.dispatch(onMouseDown(start1, 0));
-      store.dispatch(onPointerMove(end1, 1));
-      store.dispatch(onMouseUp(end1, 3));
+      store.dispatch(onPointerMove(end1, 100));
+      store.dispatch(onMouseUp(end1, 200));
       if (scaleType === ScaleType.Ordinal) {
         expect(brushEndListener).not.toBeCalled();
       } else {
@@ -918,9 +927,9 @@ function mouseOverTestSuite(scaleType: XScaleType) {
       const start2 = { x: 0, y: 75 };
       const end2 = { x: 0, y: 100 };
 
-      store.dispatch(onMouseDown(start2, 4));
-      store.dispatch(onPointerMove(end2, 5));
-      store.dispatch(onMouseUp(end2, 6));
+      store.dispatch(onMouseDown(start2, 400));
+      store.dispatch(onPointerMove(end2, 500));
+      store.dispatch(onMouseUp(end2, 600));
       if (scaleType === ScaleType.Ordinal) {
         expect(brushEndListener).not.toBeCalled();
       } else {
@@ -930,9 +939,9 @@ function mouseOverTestSuite(scaleType: XScaleType) {
 
       const start3 = { x: 0, y: 75 };
       const end3 = { x: 0, y: 200 };
-      store.dispatch(onMouseDown(start3, 7));
-      store.dispatch(onPointerMove(end3, 8));
-      store.dispatch(onMouseUp(end3, 9));
+      store.dispatch(onMouseDown(start3, 700));
+      store.dispatch(onPointerMove(end3, 800));
+      store.dispatch(onMouseUp(end3, 900));
       if (scaleType === ScaleType.Ordinal) {
         expect(brushEndListener).not.toBeCalled();
       } else {
@@ -942,9 +951,9 @@ function mouseOverTestSuite(scaleType: XScaleType) {
 
       const start4 = { x: 0, y: 25 };
       const end4 = { x: 0, y: -20 };
-      store.dispatch(onMouseDown(start4, 10));
-      store.dispatch(onPointerMove(end4, 11));
-      store.dispatch(onMouseUp(end4, 12));
+      store.dispatch(onMouseDown(start4, 1000));
+      store.dispatch(onPointerMove(end4, 1100));
+      store.dispatch(onMouseUp(end4, 1200));
       if (scaleType === ScaleType.Ordinal) {
         expect(brushEndListener).not.toBeCalled();
       } else {
@@ -993,8 +1002,8 @@ function mouseOverTestSuite(scaleType: XScaleType) {
       const end1 = { x: 0, y: 75 };
 
       store.dispatch(onMouseDown(start1, 0));
-      store.dispatch(onPointerMove(end1, 1));
-      store.dispatch(onMouseUp(end1, 3));
+      store.dispatch(onPointerMove(end1, 100));
+      store.dispatch(onMouseUp(end1, 200));
       if (scaleType === ScaleType.Ordinal) {
         expect(brushEndListener).not.toBeCalled();
       } else {
@@ -1011,9 +1020,9 @@ function mouseOverTestSuite(scaleType: XScaleType) {
       const start2 = { x: 0, y: 75 };
       const end2 = { x: 0, y: 100 };
 
-      store.dispatch(onMouseDown(start2, 4));
-      store.dispatch(onPointerMove(end2, 5));
-      store.dispatch(onMouseUp(end2, 6));
+      store.dispatch(onMouseDown(start2, 400));
+      store.dispatch(onPointerMove(end2, 500));
+      store.dispatch(onMouseUp(end2, 600));
       if (scaleType === ScaleType.Ordinal) {
         expect(brushEndListener).not.toBeCalled();
       } else {
@@ -1069,8 +1078,8 @@ function mouseOverTestSuite(scaleType: XScaleType) {
       const end1 = { x: 75, y: 75 };
 
       store.dispatch(onMouseDown(start1, 0));
-      store.dispatch(onPointerMove(end1, 1));
-      store.dispatch(onMouseUp(end1, 3));
+      store.dispatch(onPointerMove(end1, 100));
+      store.dispatch(onMouseUp(end1, 300));
       if (scaleType === ScaleType.Ordinal) {
         expect(brushEndListener).not.toBeCalled();
       } else {
@@ -1088,9 +1097,9 @@ function mouseOverTestSuite(scaleType: XScaleType) {
       const start2 = { x: 75, y: 75 };
       const end2 = { x: 100, y: 100 };
 
-      store.dispatch(onMouseDown(start2, 4));
-      store.dispatch(onPointerMove(end2, 5));
-      store.dispatch(onMouseUp(end2, 6));
+      store.dispatch(onMouseDown(start2, 400));
+      store.dispatch(onPointerMove(end2, 500));
+      store.dispatch(onMouseUp(end2, 600));
       if (scaleType === ScaleType.Ordinal) {
         expect(brushEndListener).not.toBeCalled();
       } else {

diff --git a/src/state/reducers/interactions.ts b/src/state/reducers/interactions.ts
@@ -20,6 +20,7 @@
 import { getSeriesIndex } from '../../chart_types/xy_chart/utils/series';
 import { LegendItem } from '../../commons/legend';
 import { SeriesIdentifier } from '../../commons/series_id';
+import { getDelta } from '../../utils/point';
 import { ON_KEY_UP, KeyActions } from '../actions/key';
 import {
   ON_TOGGLE_LEGEND,
@@ -33,6 +34,16 @@ import { ON_MOUSE_DOWN, ON_MOUSE_UP, ON_POINTER_MOVE, MouseActions } from '../ac
 import { InteractionsState } from '../chart_state';
 import { getInitialPointerState } from '../utils';
 
+/**
+ * The minimum amount of time to consider for for dragging purposes
+ * @internal
+ */
+export const DRAG_DETECTION_TIMEOUT = 100;
+/**
+ * The minimum number of pixel between two pointer positions to consider for dragging purposes
+ */
+const DRAG_DETECTION_PIXEL_DELTA = 4;
+
 /** @internal */
 export function interactionsReducer(
   state: InteractionsState,
@@ -51,11 +62,14 @@ export function interactionsReducer(
       return state;
 
     case ON_POINTER_MOVE:
+      const delta =
+        state.pointer.down && getDelta(state.pointer.down.position, action.position) > DRAG_DETECTION_PIXEL_DELTA;
       return {
         ...state,
         pointer: {
           ...state.pointer,
-          dragging: !!(state.pointer.down && state.pointer.down.time < action.time),
+          // enable the dragging flag only if the time between the down action and the move action is > 100ms
+          dragging: !!(state.pointer.down && action.time - state.pointer.down.time >= DRAG_DETECTION_TIMEOUT && delta),
           current: {
             position: {
               ...action.position,

diff --git a/src/utils/point.ts b/src/utils/point.ts
@@ -21,3 +21,7 @@ export interface Point {
   x: number;
   y: number;
 }
+/** @internal * */
+export function getDelta(start: Point, end: Point) {
+  return Math.sqrt(Math.pow(end.x - start.x, 2) + Math.pow(end.y - start.y, 2));
+}