Skip to content

Commit 8f8bd6d

Browse files
committed
fix(drag-drop): events fired multiple times for short drag sequences on touch devices
Fixes the `started` and `ended` events being fired multiple times for short drag sequences on touch devices. The issue comes from the fact that we listen both for mouse and touch events, which means that we also pick up the fake events that are fired by mobile browsers. Fixes #13125.
1 parent bef9a17 commit 8f8bd6d

File tree

2 files changed

+56
-5
lines changed

2 files changed

+56
-5
lines changed

src/cdk/drag-drop/drag.spec.ts

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -388,7 +388,32 @@ describe('CdkDrag', () => {
388388
expect(dragElement.style.transform).toBeFalsy();
389389
}));
390390

391-
});
391+
it('should not dispatch multiple events for a mouse event right after a touch event',
392+
fakeAsync(() => {
393+
const fixture = createComponent(StandaloneDraggable);
394+
fixture.detectChanges();
395+
396+
const dragElement = fixture.componentInstance.dragElement.nativeElement;
397+
398+
// Dispatch a touch sequence.
399+
dispatchTouchEvent(dragElement, 'touchstart');
400+
fixture.detectChanges();
401+
dispatchTouchEvent(dragElement, 'touchend');
402+
fixture.detectChanges();
403+
tick();
404+
405+
// Immediately dispatch a mouse sequence to simulate a fake event.
406+
startDraggingViaMouse(fixture, dragElement);
407+
fixture.detectChanges();
408+
dispatchMouseEvent(dragElement, 'mouseup');
409+
fixture.detectChanges();
410+
tick();
411+
412+
expect(fixture.componentInstance.startedSpy).toHaveBeenCalledTimes(1);
413+
expect(fixture.componentInstance.endedSpy).toHaveBeenCalledTimes(1);
414+
}));
415+
416+
});
392417

393418
describe('draggable with a handle', () => {
394419
it('should not be able to drag the entire element if it has a handle', fakeAsync(() => {

src/cdk/drag-drop/drag.ts

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,14 @@ export function CDK_DRAG_CONFIG_FACTORY(): CdkDragConfig {
7676
return {dragStartThreshold: 5, pointerDirectionChangeThreshold: 5};
7777
}
7878

79+
/**
80+
* Time in milliseconds for which to ignore mouse events, after
81+
* receiving a touch event. Used to avoid doing double work for
82+
* touch devices where the browser fires fake mouse events, in
83+
* addition to touch events.
84+
*/
85+
const MOUSE_EVENT_IGNORE_TIME = 800;
86+
7987
/** Element that can be moved inside a CdkDrop container. */
8088
@Directive({
8189
selector: '[cdkDrag]',
@@ -161,6 +169,12 @@ export class CdkDrag<T = any> implements AfterViewInit, OnDestroy {
161169

162170
/** Subscription to the event that is dispatched when the user lifts their pointer. */
163171
private _pointerUpSubscription = Subscription.EMPTY;
172+
/**
173+
* Time at which the last touch event occurred. Used to avoid firing the same
174+
* events multiple times on touch devices where the browser will fire a fake
175+
* mouse event for each touch event, after a certain time.
176+
*/
177+
private _lastTouchEventTime: number;
164178

165179
/** Elements that can be used to drag the draggable item. */
166180
@ContentChildren(CdkDragHandle) _handles: QueryList<CdkDragHandle>;
@@ -308,9 +322,13 @@ export class CdkDrag<T = any> implements AfterViewInit, OnDestroy {
308322
*/
309323
private _initializeDragSequence(referenceElement: HTMLElement, event: MouseEvent | TouchEvent) {
310324
const isDragging = this._isDragging();
325+
const isTouchEvent = this._isTouchEvent(event);
326+
const isAuxiliaryMouseButton = !isTouchEvent && (event as MouseEvent).button !== 0;
327+
const isSyntheticEvent = !isTouchEvent && this._lastTouchEventTime &&
328+
this._lastTouchEventTime + MOUSE_EVENT_IGNORE_TIME > Date.now();
311329

312330
// Abort if the user is already dragging or is using a mouse button other than the primary one.
313-
if (isDragging || (!this._isTouchEvent(event) && event.button !== 0)) {
331+
if (isDragging || isAuxiliaryMouseButton || isSyntheticEvent) {
314332
return;
315333
}
316334

@@ -331,10 +349,14 @@ export class CdkDrag<T = any> implements AfterViewInit, OnDestroy {
331349
}
332350

333351
/** Starts the dragging sequence. */
334-
private _startDragSequence() {
352+
private _startDragSequence(event: MouseEvent | TouchEvent) {
335353
// Emit the event on the item before the one on the container.
336354
this.started.emit({source: this});
337355

356+
if (this._isTouchEvent(event)) {
357+
this._lastTouchEventTime = Date.now();
358+
}
359+
338360
if (this.dropContainer) {
339361
const element = this._rootElement;
340362

@@ -370,7 +392,7 @@ export class CdkDrag<T = any> implements AfterViewInit, OnDestroy {
370392
// per pixel of movement (e.g. if the user moves their pointer quickly).
371393
if (distanceX + distanceY >= minimumDistance) {
372394
this._hasStartedDragging = true;
373-
this._ngZone.run(() => this._startDragSequence());
395+
this._ngZone.run(() => this._startDragSequence(event));
374396
}
375397

376398
return;
@@ -426,10 +448,14 @@ export class CdkDrag<T = any> implements AfterViewInit, OnDestroy {
426448
this._passiveTransform.x = this._activeTransform.x;
427449
this._passiveTransform.y = this._activeTransform.y;
428450
this._ngZone.run(() => this.ended.emit({source: this}));
451+
this._dragDropRegistry.stopDragging(this);
429452
return;
430453
}
431454

432-
this._animatePreviewToPlaceholder().then(() => this._cleanupDragArtifacts());
455+
this._animatePreviewToPlaceholder().then(() => {
456+
this._cleanupDragArtifacts();
457+
this._dragDropRegistry.stopDragging(this);
458+
});
433459
}
434460

435461
/** Cleans up the DOM artifacts that were added to facilitate the element being dragged. */

0 commit comments

Comments
 (0)