feat(core): add new preventDragFromKeys grid option, fixes #1537 (#1538)

* feat(core): add `preventDragFromKeys` grid option, fixes #1537
- the issue was brought in discussion #1537, the root cause was the mouse drag sometime kicks in when the user selects a few row by using the Ctrl+click combo, however in rare occasion the user might move by even a single pixel and that sends an onDrag event which the SlickCellRangeSelector picks up when then sends a new event `onCellRangeSelecting` and then the SlickRowSelectionModel assumes it's a new range from a mouse drag and override the previous range. However we should really prevent this mouse drag from happening when the user is pressing the Ctrl/Meta keys to avoid having the issue brought in #1537

Author: ghiscoding

packages/common/src/core/__tests__/slickInteractions.spec.ts

@@ -30,7 +30,7 @@ describe('Draggable class', () => {
 
     dg = Draggable({ containerElement, allowDragFrom: 'div.slick-cell', onDrag: dragInitSpy });
 
-    containerElement.dispatchEvent(new Event('mousedown'));
+    containerElement.dispatchEvent(new MouseEvent('mousedown'));
 
     expect(dg).toBeTruthy();
     expect(dragInitSpy).not.toHaveBeenCalled();
@@ -43,7 +43,7 @@ describe('Draggable class', () => {
 
     dg = Draggable({ containerElement, allowDragFrom: 'div.slick-cell', onDrag: dragSpy, onDragInit: dragInitSpy });
 
-    containerElement.dispatchEvent(new Event('mousedown'));
+    containerElement.dispatchEvent(new MouseEvent('mousedown'));
 
     expect(dg).toBeTruthy();
     expect(dragInitSpy).toHaveBeenCalled();
@@ -52,6 +52,22 @@ describe('Draggable class', () => {
     dg.destroy();
   });
 
+  it('should NOT trigger dragInit event when user is pressing mousedown and mousemove + Ctrl key combo that we considered as forbidden via "preventDragFromKeys"', () => {
+    const dragInitSpy = jest.fn();
+    const dragSpy = jest.fn();
+    containerElement.className = 'slick-cell';
+
+    dg = Draggable({ containerElement, allowDragFrom: 'div.slick-cell', preventDragFromKeys: ['ctrlKey'], onDrag: dragSpy, onDragInit: dragInitSpy });
+
+    containerElement.dispatchEvent(new MouseEvent('mousedown', { ctrlKey: true }));
+
+    expect(dg).toBeTruthy();
+    expect(dragInitSpy).not.toHaveBeenCalled();
+    expect(dragSpy).not.toHaveBeenCalled();
+
+    dg.destroy();
+  });
+
   it('should trigger mousedown and expect a dragInit and a dragStart and drag to all happen since it was triggered by an allowed element and we did move afterward', () => {
     const removeListenerSpy = jest.spyOn(document.body, 'removeEventListener');
     const dragInitSpy = jest.fn();
@@ -62,13 +78,13 @@ describe('Draggable class', () => {
 
     dg = Draggable({ containerElement, allowDragFrom: 'div.slick-cell', onDrag: dragSpy, onDragInit: dragInitSpy, onDragStart: dragStartSpy, onDragEnd: dragEndSpy });
 
-    const mdEvt = new Event('mousedown');
+    const mdEvt = new MouseEvent('mousedown');
     Object.defineProperty(mdEvt, 'clientX', { writable: true, configurable: true, value: 10 });
     Object.defineProperty(mdEvt, 'clientY', { writable: true, configurable: true, value: 10 });
     containerElement.dispatchEvent(mdEvt);
 
-    const mmEvt = new Event('mousemove');
-    const muEvt = new Event('mouseup');
+    const mmEvt = new MouseEvent('mousemove');
+    const muEvt = new MouseEvent('mouseup');
     Object.defineProperty(mmEvt, 'clientX', { writable: true, configurable: true, value: 12 });
     Object.defineProperty(mmEvt, 'clientY', { writable: true, configurable: true, value: 10 });
     Object.defineProperty(muEvt, 'clientX', { writable: true, configurable: true, value: 12 });
@@ -83,6 +99,38 @@ describe('Draggable class', () => {
     expect(dragEndSpy).toHaveBeenCalled();
     expect(removeListenerSpy).toHaveBeenCalledTimes(5 * 2);
   });
+
+  it('should NOT trigger dragInit,dragStart events when user is pressing mousedown and mousemove + Meta key combo that we considered as forbidden via "preventDragFromKeys"', () => {
+    const removeListenerSpy = jest.spyOn(document.body, 'removeEventListener');
+    const dragInitSpy = jest.fn();
+    const dragSpy = jest.fn();
+    const dragStartSpy = jest.fn();
+    const dragEndSpy = jest.fn();
+    containerElement.className = 'slick-cell';
+
+    dg = Draggable({ containerElement, allowDragFrom: 'div.slick-cell', preventDragFromKeys: ['metaKey'], onDrag: dragSpy, onDragInit: dragInitSpy, onDragStart: dragStartSpy, onDragEnd: dragEndSpy });
+
+    const mdEvt = new MouseEvent('mousedown', { metaKey: true });
+    Object.defineProperty(mdEvt, 'clientX', { writable: true, configurable: true, value: 10 });
+    Object.defineProperty(mdEvt, 'clientY', { writable: true, configurable: true, value: 10 });
+    containerElement.dispatchEvent(mdEvt);
+
+    const mmEvt = new MouseEvent('mousemove', { metaKey: true });
+    const muEvt = new MouseEvent('mouseup', { metaKey: true });
+    Object.defineProperty(mmEvt, 'clientX', { writable: true, configurable: true, value: 12 });
+    Object.defineProperty(mmEvt, 'clientY', { writable: true, configurable: true, value: 10 });
+    Object.defineProperty(muEvt, 'clientX', { writable: true, configurable: true, value: 12 });
+    Object.defineProperty(muEvt, 'clientY', { writable: true, configurable: true, value: 10 });
+    document.body.dispatchEvent(mmEvt);
+    document.body.dispatchEvent(muEvt);
+
+    expect(dg).toBeTruthy();
+    expect(dragInitSpy).not.toHaveBeenCalledWith(mdEvt, { startX: 10, startY: 10, deltaX: 2, deltaY: 0, dragHandle: containerElement, dragSource: containerElement, target: document.body });
+    expect(dragStartSpy).not.toHaveBeenCalled();
+    expect(dragSpy).not.toHaveBeenCalled();
+    expect(dragEndSpy).not.toHaveBeenCalled();
+    expect(removeListenerSpy).toHaveBeenCalledTimes(5 * 2);
+  });
 });
 
 describe('MouseWheel class', () => {
@@ -188,13 +236,13 @@ describe('Resizable class', () => {
 
     rsz = Resizable({ resizeableElement: containerElement, resizeableHandleElement: containerElement, onResize: resizeSpy, onResizeStart: resizeStartSpy, onResizeEnd: resizeEndSpy });
 
-    const mdEvt = new Event('mousedown');
+    const mdEvt = new MouseEvent('mousedown');
     Object.defineProperty(mdEvt, 'clientX', { writable: true, configurable: true, value: 10 });
     Object.defineProperty(mdEvt, 'clientY', { writable: true, configurable: true, value: 10 });
     containerElement.dispatchEvent(mdEvt);
 
-    const mmEvt = new Event('mousemove');
-    const muEvt = new Event('mouseup');
+    const mmEvt = new MouseEvent('mousemove');
+    const muEvt = new MouseEvent('mouseup');
     Object.defineProperty(mmEvt, 'clientX', { writable: true, configurable: true, value: 12 });
     Object.defineProperty(mmEvt, 'clientY', { writable: true, configurable: true, value: 10 });
     Object.defineProperty(muEvt, 'clientX', { writable: true, configurable: true, value: 12 });

packages/common/src/core/slickGrid.ts

@@ -256,6 +256,7 @@ export class SlickGrid<TData = any, C extends Column<TData> = Column<TData>, O e
     forceSyncScrolling: false,
     addNewRowCssClass: 'new-row',
     preserveCopiedSelectionOnPaste: false,
+    preventDragFromKeys: ['ctrlKey', 'metaKey'],
     showCellSelection: true,
     viewportClass: undefined,
     minRowBuffer: 3,
@@ -904,6 +905,7 @@ export class SlickGrid<TData = any, C extends Column<TData> = Column<TData>, O e
           allowDragFrom: 'div.slick-cell',
           // the slick cell parent must always contain `.dnd` and/or `.cell-reorder` class to be identified as draggable
           allowDragFromClosest: 'div.slick-cell.dnd, div.slick-cell.cell-reorder',
+          preventDragFromKeys: this._options.preventDragFromKeys,
           onDragInit: this.handleDragInit.bind(this),
           onDragStart: this.handleDragStart.bind(this),
           onDrag: this.handleDrag.bind(this),

packages/common/src/core/slickInteractions.ts

@@ -28,7 +28,7 @@ import type { DragItem, DragPosition, DraggableOption, MouseWheelOption, Resizab
  */
 export function Draggable(options: DraggableOption) {
   let { containerElement } = options;
-  const { onDragInit, onDragStart, onDrag, onDragEnd } = options;
+  const { onDragInit, onDragStart, onDrag, onDragEnd, preventDragFromKeys } = options;
   let element: HTMLElement | null;
   let startX: number;
   let startY: number;
@@ -52,7 +52,7 @@ export function Draggable(options: DraggableOption) {
     }
   }
 
-  function executeDragCallbackWhenDefined(callback?: (e: DragEvent, dd: DragPosition) => boolean | void, evt?: MouseEvent | Touch | TouchEvent, dd?: DragItem) {
+  function executeDragCallbackWhenDefined(callback?: (e: DragEvent, dd: DragPosition) => boolean | void, evt?: MouseEvent | Touch | TouchEvent | KeyboardEvent, dd?: DragItem) {
     if (typeof callback === 'function') {
       return callback(evt as DragEvent, dd as DragItem);
     }
@@ -65,45 +65,62 @@ export function Draggable(options: DraggableOption) {
     }
   }
 
-  function userPressed(event: MouseEvent | TouchEvent) {
+  /** Do we want to prevent Drag events from happening (for example prevent onDrag when Ctrl key is pressed while dragging) */
+  function preventDrag(event: MouseEvent | TouchEvent | KeyboardEvent) {
+    let eventPrevented = false;
+    if (preventDragFromKeys) {
+      preventDragFromKeys.forEach(key => {
+        if ((event as KeyboardEvent)[key]) {
+          eventPrevented = true;
+        }
+      });
+    }
+    return eventPrevented;
+  }
+
+  function userPressed(event: MouseEvent | TouchEvent | KeyboardEvent) {
     element = event.target as HTMLElement;
-    const targetEvent: MouseEvent | Touch = (event as TouchEvent)?.touches?.[0] ?? event;
-    const { target } = targetEvent;
-
-    if (!options.allowDragFrom || (options.allowDragFrom && (element.matches(options.allowDragFrom)) || (options.allowDragFromClosest && element.closest(options.allowDragFromClosest)))) {
-      originaldd.dragHandle = element as HTMLElement;
-      const winScrollPos = windowScrollPosition();
-      startX = winScrollPos.left + targetEvent.clientX;
-      startY = winScrollPos.top + targetEvent.clientY;
-      deltaX = targetEvent.clientX - targetEvent.clientX;
-      deltaY = targetEvent.clientY - targetEvent.clientY;
-      originaldd = Object.assign(originaldd, { deltaX, deltaY, startX, startY, target });
-      const result = executeDragCallbackWhenDefined(onDragInit as (e: DragEvent, dd: DragPosition) => boolean | void, event, originaldd as DragItem);
-
-      if (result !== false) {
-        document.body.addEventListener('mousemove', userMoved);
-        document.body.addEventListener('touchmove', userMoved);
-        document.body.addEventListener('mouseup', userReleased);
-        document.body.addEventListener('touchend', userReleased);
-        document.body.addEventListener('touchcancel', userReleased);
+    if (!preventDrag(event)) {
+      const targetEvent: MouseEvent | Touch = (event as TouchEvent)?.touches?.[0] ?? event;
+      const { target } = targetEvent;
+
+      if (!options.allowDragFrom || (options.allowDragFrom && (element.matches(options.allowDragFrom)) || (options.allowDragFromClosest && element.closest(options.allowDragFromClosest)))) {
+        originaldd.dragHandle = element as HTMLElement;
+        const winScrollPos = windowScrollPosition();
+        startX = winScrollPos.left + targetEvent.clientX;
+        startY = winScrollPos.top + targetEvent.clientY;
+        deltaX = targetEvent.clientX - targetEvent.clientX;
+        deltaY = targetEvent.clientY - targetEvent.clientY;
+        originaldd = Object.assign(originaldd, { deltaX, deltaY, startX, startY, target });
+        const result = executeDragCallbackWhenDefined(onDragInit as (e: DragEvent, dd: DragPosition) => boolean | void, event, originaldd as DragItem);
+
+        if (result !== false) {
+          document.body.addEventListener('mousemove', userMoved);
+          document.body.addEventListener('touchmove', userMoved);
+          document.body.addEventListener('mouseup', userReleased);
+          document.body.addEventListener('touchend', userReleased);
+          document.body.addEventListener('touchcancel', userReleased);
+        }
       }
     }
   }
 
-  function userMoved(event: MouseEvent | TouchEvent) {
+  function userMoved(event: MouseEvent | TouchEvent | KeyboardEvent) {
     const targetEvent: MouseEvent | Touch = (event as TouchEvent)?.touches?.[0] ?? event;
-    deltaX = targetEvent.clientX - startX;
-    deltaY = targetEvent.clientY - startY;
-    const { target } = targetEvent;
+    if (!preventDrag(event)) {
+      deltaX = targetEvent.clientX - startX;
+      deltaY = targetEvent.clientY - startY;
+      const { target } = targetEvent;
+
+      if (!dragStarted) {
+        originaldd = Object.assign(originaldd, { deltaX, deltaY, startX, startY, target });
+        executeDragCallbackWhenDefined(onDragStart, event, originaldd as DragItem);
+        dragStarted = true;
+      }
 
-    if (!dragStarted) {
       originaldd = Object.assign(originaldd, { deltaX, deltaY, startX, startY, target });
-      executeDragCallbackWhenDefined(onDragStart, event, originaldd as DragItem);
-      dragStarted = true;
+      executeDragCallbackWhenDefined(onDrag, event, originaldd as DragItem);
     }
-
-    originaldd = Object.assign(originaldd, { deltaX, deltaY, startX, startY, target });
-    executeDragCallbackWhenDefined(onDrag, event, originaldd as DragItem);
   }
 
   function userReleased(event: MouseEvent | TouchEvent) {

packages/common/src/interfaces/gridOption.interface.ts

@@ -668,6 +668,9 @@ export interface GridOption<C extends Column = Column> {
   /** Defaults to false, do we want prevent the usage of DocumentFragment by the library (might not be supported by all environments, e.g. not supported by Salesforce) */
   preventDocumentFragmentUsage?: boolean;
 
+  /** Defaults to `['ctrlKey', 'metaKey']`, list of keys that when pressed will prevent Draggable events from triggering (e.g. prevent onDrag when Ctrl key is pressed while dragging) */
+  preventDragFromKeys?: Array<'altKey' | 'ctrlKey' | 'metaKey' | 'shiftKey'>;
+
   /** Preselect certain rows by their row index ("enableCheckboxSelector" must be enabled) */
   preselectedRows?: number[];
 

packages/common/src/interfaces/interactions.interface.ts

@@ -17,6 +17,9 @@ export interface DraggableOption {
   /** when defined, will allow dragging from a specific element or its closest parent by using the .closest() query selector. */
   allowDragFromClosest?: string;
 
+  /** Defaults to `['ctrlKey', 'metaKey']`, list of keys that when pressed will prevent Draggable events from triggering (e.g. prevent onDrag when Ctrl key is pressed while dragging) */
+  preventDragFromKeys?: Array<'altKey' | 'ctrlKey' | 'metaKey' | 'shiftKey'>;
+
   /** drag initialized callback */
   onDragInit?: (e: DragEvent, dd: DragPosition) => boolean | void;