Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 8 additions & 56 deletions packages/devextreme/js/__internal/__tests__/draggable.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,70 +5,22 @@ import {
expect,
it,
} from '@jest/globals';
import eventsEngine from '@js/common/core/events/core/events_engine';
import type { dxElementWrapper } from '@js/core/renderer';
import $ from '@js/core/renderer';
import Draggable from '@js/ui/draggable';

const DRAGGABLE_ELEMENT_ID = 'draggable';

interface DraggableTest extends Draggable {
getDragInProgress: () => boolean;
}

interface FiredEvent {
_cancelPreventDefault?: boolean;
isDefaultPrevented: () => boolean;
}

const fire = (element: Element, type: string, x = 0, y = 0): FiredEvent => {
// @ts-expect-error -- Event is absent from the public eventsEngine type
const event: FiredEvent = eventsEngine.Event({
type,
pageX: x,
pageY: y,
clientX: x,
clientY: y,
offset: { x, y },
pointerType: 'touch',
pointers: type === 'dxpointerup' ? [] : [{ pointerId: 1 }],
pointerId: 1,
which: 1,
});

// @ts-expect-error -- trigger is absent from the public eventsEngine type
eventsEngine.trigger(element, event);

return event;
};
import { createDraggable, disposeDraggable, fire } from './utils';

const swipe = (target: Element): void => {
fire(target, 'dxpointerdown', 0, 0);
fire(target, 'dxpointermove', 0, 20);
fire(target, 'dxpointermove', 0, 40);
fire(target, 'dxpointerup', 0, 40);
};

const appendElement = (): dxElementWrapper => $('<div>')
.attr('id', DRAGGABLE_ELEMENT_ID)
.css({ width: '100px', height: '40px' })
.appendTo(document.body);

const createDraggable = (options: Record<string, unknown> = {}): DraggableTest => {
const $element = appendElement();

return new Draggable($element.get(0), { autoScroll: false, ...options }) as DraggableTest;
fire(target, 'dxpointerdown');
fire(target, 'dxpointermove', { y: 20 });
fire(target, 'dxpointermove', { y: 40 });
fire(target, 'dxpointerup', { y: 40 });
};

beforeEach(() => {
document.body.innerHTML = '';
});

afterEach(() => {
const instance = Draggable.getInstance($(`#${DRAGGABLE_ELEMENT_ID}`).get(0));

instance?.dispose();
});
afterEach(disposeDraggable);

describe('Draggable dragInProgress reset', () => {
it('should reset dragInProgress when a drag is completed', () => {
Expand Down Expand Up @@ -124,8 +76,8 @@ describe('Draggable preventDefault on a drag move', () => {
const draggable = createDraggable();
const element = draggable.$element().get(0);

fire(element, 'dxpointerdown', 0, 0);
const event = fire(element, 'dxpointermove', 0, 20);
fire(element, 'dxpointerdown');
const event = fire(element, 'dxpointermove', { y: 20 });

expect(event._cancelPreventDefault).not.toBe(true);
expect(event.isDefaultPrevented()).toBe(true);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
import {
afterEach,
beforeEach,
describe,
expect,
it,
jest,
} from '@jest/globals';
import devices from '@ts/core/m_devices';

import { createDraggable, disposeDraggable, fire } from './utils';

jest.mock('@ts/core/m_devices', () => {
const originalModule: any = jest.requireActual('@ts/core/m_devices');
const device = { mac: false, deviceType: 'desktop', platform: 'generic' };

return {
__esModule: true,
default: {
...originalModule.default,
isSimulator: originalModule.default.isSimulator,
real: jest.fn().mockReturnValue(device),
current: jest.fn().mockReturnValue(device),
},
};
});

const setMac = (mac: boolean): void => {
(devices.real as jest.Mock).mockReturnValue({ mac, deviceType: 'desktop', platform: 'generic' });
};

beforeEach(() => {
document.body.innerHTML = '';
setMac(false);
});

afterEach(disposeDraggable);

describe('Draggable macOS Ctrl+click (T1328053)', () => {
it('should not start a drag when a mouse move follows a macOS Ctrl+click', () => {
setMac(true);
const onDragStart = jest.fn();
const draggable = createDraggable({ onDragStart });
const element = draggable.$element().get(0);

fire(element, 'dxpointerdown', { ctrlKey: true, pointerType: 'mouse' });
fire(element, 'dxpointermove', { y: 40, pointerType: 'mouse' });

expect(onDragStart).not.toHaveBeenCalled();
expect(draggable.getDragInProgress()).toBe(false);
});

it('should still start a drag for a regular (non-Ctrl) click on macOS', () => {
setMac(true);
const onDragStart = jest.fn();
const draggable = createDraggable({ onDragStart });
const element = draggable.$element().get(0);

fire(element, 'dxpointerdown', { pointerType: 'mouse' });
fire(element, 'dxpointermove', { y: 40, pointerType: 'mouse' });

expect(onDragStart).toHaveBeenCalledTimes(1);
expect(draggable.getDragInProgress()).toBe(true);
});

it('should still start a drag for a Ctrl+click on non-macOS platforms', () => {
setMac(false);
const onDragStart = jest.fn();
const draggable = createDraggable({ onDragStart });
const element = draggable.$element().get(0);

fire(element, 'dxpointerdown', { ctrlKey: true, pointerType: 'mouse' });
fire(element, 'dxpointermove', { y: 40, pointerType: 'mouse' });

expect(onDragStart).toHaveBeenCalledTimes(1);
expect(draggable.getDragInProgress()).toBe(true);
});
});
64 changes: 64 additions & 0 deletions packages/devextreme/js/__internal/__tests__/utils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
import eventsEngine from '@js/common/core/events/core/events_engine';
import $ from '@js/core/renderer';
import Draggable from '@js/ui/draggable';

export const DRAGGABLE_ELEMENT_ID = 'draggable';

export interface DraggableTest extends Draggable {
getDragInProgress: () => boolean;
}

export interface FiredEvent {
_cancelPreventDefault?: boolean;
isDefaultPrevented: () => boolean;
}

interface FireOptions {
x?: number;
y?: number;
ctrlKey?: boolean;
pointerType?: 'mouse' | 'touch';
}

export const fire = (
element: Element,
type: string,
{
x = 0, y = 0, ctrlKey = false, pointerType = 'touch',
}: FireOptions = {},
): FiredEvent => {
// @ts-expect-error -- Event is absent from the public eventsEngine type
const event: FiredEvent = eventsEngine.Event({
type,
pageX: x,
pageY: y,
clientX: x,
clientY: y,
offset: { x, y },
pointerType,
pointers: type === 'dxpointerup' ? [] : [{ pointerId: 1 }],
pointerId: 1,
which: 1,
ctrlKey,
});

// @ts-expect-error -- trigger is absent from the public eventsEngine type
eventsEngine.trigger(element, event);

return event;
};

export const createDraggable = (options: Record<string, unknown> = {}): DraggableTest => {
const $element = $('<div>')
.attr('id', DRAGGABLE_ELEMENT_ID)
.css({ width: '100px', height: '40px' })
.appendTo(document.body);

return new Draggable($element.get(0), { autoScroll: false, ...options }) as DraggableTest;
};

export const disposeDraggable = (): void => {
const instance = Draggable.getInstance($(`#${DRAGGABLE_ELEMENT_ID}`).get(0));

instance?.dispose();
};
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import Emitter from '@js/common/core/events/core/emitter';
import eventsEngine from '@js/common/core/events/core/events_engine';
import {
createEvent, eventData, eventDelta, isDxMouseWheelEvent, isTouchEvent, needSkipEvent,
createEvent, eventData, eventDelta, isDxMouseWheelEvent, isMouseEvent, isTouchEvent, needSkipEvent,
} from '@js/common/core/events/utils/index';
import $ from '@js/core/renderer';
import callOnce from '@js/core/utils/call_once';
Expand Down Expand Up @@ -87,7 +87,12 @@ const GestureEmitter = Emitter.inherit({
},

start(e) {
if (e._needSkipEvent || needSkipEvent(e)) {
// T1328053: macOS Ctrl+click opens the system context menu. Kept out of needSkipEvent()
// because importing m_devices into that low-level events util forces `new Devices()` into
// early module init and breaks init order (resizeCallbacks stops firing).
const isMacContextMenuClick = isMouseEvent(e) && Boolean(e.ctrlKey) && devices.real().mac;
Comment thread
anna-shakhova marked this conversation as resolved.

if (e._needSkipEvent || needSkipEvent(e) || isMacContextMenuClick) {
this._cancel(e);
return;
}
Expand Down
17 changes: 14 additions & 3 deletions packages/devextreme/js/__internal/events/utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,14 @@ const LEGACY_KEY_CODES = {
18: 'alt',

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file contains only refactoring, without fix.

};

const MOUSE_BUTTON_MAP = {
left: 1,
middle: 2,
right: 3,
back: 4,
forward: 5,
};

const EVENT_SOURCES_REGEX = {
dx: /^dx/i,
mouse: /(mouse|wheel)/i,
Expand Down Expand Up @@ -141,7 +149,6 @@ export const hasTouches = (e) => {
let skipEvents = false;
export const forceSkipEvents = () => { skipEvents = true; };
export const stopEventsSkipping = () => { skipEvents = false; };
// eslint-disable-next-line consistent-return
export const needSkipEvent = (e) => {
// TODO: for tests
if (skipEvents) {
Expand All @@ -151,7 +158,7 @@ export const needSkipEvent = (e) => {
// TODO: this checking used in swipeable first move handler. is it correct?
const { target } = e;
const $target = $(target);
const isContentEditable = target?.isContentEditable || target?.hasAttribute('contenteditable');
const isContentEditable: boolean = target?.isContentEditable || target?.hasAttribute('contenteditable');
const touchInEditable = $target.is('input, textarea, select') || isContentEditable;

if (isDxMouseWheelEvent(e)) {
Expand All @@ -167,12 +174,16 @@ export const needSkipEvent = (e) => {
}

if (isMouseEvent(e)) {
return touchInEditable || e.which > 1; // only left mouse button
const isNotLeftMouseButton = e.which > MOUSE_BUTTON_MAP.left;

return touchInEditable || isNotLeftMouseButton;
}

if (isTouchEvent(e)) {
return touchInEditable && focused($target);
}

return false;
};

export const setEventFixMethod = (func) => { fixMethod = func; };
Expand Down