Skip to content

Commit

Permalink
feat(core): add new preventDragFromKeys grid option, fixes #1537 (#…
Browse files Browse the repository at this point in the history 
…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
  • Loading branch information
@ghiscoding
ghiscoding authored May 23, 2024
1 parent 2a4ff8f commit 803fbee
Show file tree
Hide file tree
Showing 5 changed files with 113 additions and 40 deletions.
64 changes: 56 additions & 8 deletions packages/common/src/core/__tests__/slickInteractions.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -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 });
Expand All @@ -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', () => {
Expand Down Expand Up @@ -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 });
Expand Down
2 changes: 2 additions & 0 deletions packages/common/src/core/slickGrid.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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),
Expand Down
81 changes: 49 additions & 32 deletions packages/common/src/core/slickInteractions.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
}
Expand All @@ -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) {
Expand Down
3 changes: 3 additions & 0 deletions packages/common/src/interfaces/gridOption.interface.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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[];

Expand Down
3 changes: 3 additions & 0 deletions packages/common/src/interfaces/interactions.interface.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down

0 comments on commit 803fbee

Please sign in to comment.