Skip to content

Commit 5ec58c0

Browse files
crisbetojosephperrott
authored andcommitted
fix(drag-drop): events fired multiple times for short drag sequences on touch devices (#13135)
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 1f806ea commit 5ec58c0

File tree

2 files changed

+58
-5
lines changed

2 files changed

+58
-5
lines changed

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

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -508,7 +508,32 @@ describe('CdkDrag', () => {
508508
expect(dragElement.style.transform).toBe('translate3d(25px, 50px, 0px)');
509509
}));
510510

511-
});
511+
it('should not dispatch multiple events for a mouse event right after a touch event',
512+
fakeAsync(() => {
513+
const fixture = createComponent(StandaloneDraggable);
514+
fixture.detectChanges();
515+
516+
const dragElement = fixture.componentInstance.dragElement.nativeElement;
517+
518+
// Dispatch a touch sequence.
519+
dispatchTouchEvent(dragElement, 'touchstart');
520+
fixture.detectChanges();
521+
dispatchTouchEvent(dragElement, 'touchend');
522+
fixture.detectChanges();
523+
tick();
524+
525+
// Immediately dispatch a mouse sequence to simulate a fake event.
526+
startDraggingViaMouse(fixture, dragElement);
527+
fixture.detectChanges();
528+
dispatchMouseEvent(dragElement, 'mouseup');
529+
fixture.detectChanges();
530+
tick();
531+
532+
expect(fixture.componentInstance.startedSpy).toHaveBeenCalledTimes(1);
533+
expect(fixture.componentInstance.endedSpy).toHaveBeenCalledTimes(1);
534+
}));
535+
536+
});
512537

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

src/cdk/drag-drop/drag.ts

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,14 @@ const passiveEventListenerOptions = normalizePassiveListenerOptions({passive: tr
8585
/** Options that can be used to bind an active event listener. */
8686
const activeEventListenerOptions = normalizePassiveListenerOptions({passive: false});
8787

88+
/**
89+
* Time in milliseconds for which to ignore mouse events, after
90+
* receiving a touch event. Used to avoid doing double work for
91+
* touch devices where the browser fires fake mouse events, in
92+
* addition to touch events.
93+
*/
94+
const MOUSE_EVENT_IGNORE_TIME = 800;
95+
8896
/** Element that can be moved inside a CdkDropList container. */
8997
@Directive({
9098
selector: '[cdkDrag]',
@@ -177,6 +185,12 @@ export class CdkDrag<T = any> implements AfterViewInit, OnDestroy {
177185

178186
/** Subscription to the event that is dispatched when the user lifts their pointer. */
179187
private _pointerUpSubscription = Subscription.EMPTY;
188+
/**
189+
* Time at which the last touch event occurred. Used to avoid firing the same
190+
* events multiple times on touch devices where the browser will fire a fake
191+
* mouse event for each touch event, after a certain time.
192+
*/
193+
private _lastTouchEventTime: number;
180194

181195
/** Subscription to the stream that initializes the root element. */
182196
private _rootElementInitSubscription = Subscription.EMPTY;
@@ -357,6 +371,12 @@ export class CdkDrag<T = any> implements AfterViewInit, OnDestroy {
357371
// starting another sequence for a draggable parent somewhere up the DOM tree.
358372
event.stopPropagation();
359373

374+
const isDragging = this._isDragging();
375+
const isTouchEvent = this._isTouchEvent(event);
376+
const isAuxiliaryMouseButton = !isTouchEvent && (event as MouseEvent).button !== 0;
377+
const isSyntheticEvent = !isTouchEvent && this._lastTouchEventTime &&
378+
this._lastTouchEventTime + MOUSE_EVENT_IGNORE_TIME > Date.now();
379+
360380
// If the event started from an element with the native HTML drag&drop, it'll interfere
361381
// with our own dragging (e.g. `img` tags do it by default). Prevent the default action
362382
// to stop it from happening. Note that preventing on `dragstart` also seems to work, but
@@ -368,7 +388,7 @@ export class CdkDrag<T = any> implements AfterViewInit, OnDestroy {
368388
}
369389

370390
// Abort if the user is already dragging or is using a mouse button other than the primary one.
371-
if (this._isDragging() || (!this._isTouchEvent(event) && event.button !== 0)) {
391+
if (isDragging || isAuxiliaryMouseButton || isSyntheticEvent) {
372392
return;
373393
}
374394

@@ -395,10 +415,14 @@ export class CdkDrag<T = any> implements AfterViewInit, OnDestroy {
395415
}
396416

397417
/** Starts the dragging sequence. */
398-
private _startDragSequence() {
418+
private _startDragSequence(event: MouseEvent | TouchEvent) {
399419
// Emit the event on the item before the one on the container.
400420
this.started.emit({source: this});
401421

422+
if (this._isTouchEvent(event)) {
423+
this._lastTouchEventTime = Date.now();
424+
}
425+
402426
if (this.dropContainer) {
403427
const element = this._rootElement;
404428

@@ -433,7 +457,7 @@ export class CdkDrag<T = any> implements AfterViewInit, OnDestroy {
433457
// per pixel of movement (e.g. if the user moves their pointer quickly).
434458
if (distanceX + distanceY >= this._config.dragStartThreshold) {
435459
this._hasStartedDragging = true;
436-
this._ngZone.run(() => this._startDragSequence());
460+
this._ngZone.run(() => this._startDragSequence(event));
437461
}
438462

439463
return;
@@ -493,10 +517,14 @@ export class CdkDrag<T = any> implements AfterViewInit, OnDestroy {
493517
this._passiveTransform.x = this._activeTransform.x;
494518
this._passiveTransform.y = this._activeTransform.y;
495519
this._ngZone.run(() => this.ended.emit({source: this}));
520+
this._dragDropRegistry.stopDragging(this);
496521
return;
497522
}
498523

499-
this._animatePreviewToPlaceholder().then(() => this._cleanupDragArtifacts());
524+
this._animatePreviewToPlaceholder().then(() => {
525+
this._cleanupDragArtifacts();
526+
this._dragDropRegistry.stopDragging(this);
527+
});
500528
}
501529

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

0 commit comments

Comments
 (0)