Skip to content

Commit 21690bc

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 8b2dc82 commit 21690bc

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
@@ -443,7 +443,32 @@ describe('CdkDrag', () => {
443443
.not.toEqual('none', 'should not disable touchAction on when there is a drag handle');
444444
});
445445

446-
});
446+
it('should not dispatch multiple events for a mouse event right after a touch event',
447+
fakeAsync(() => {
448+
const fixture = createComponent(StandaloneDraggable);
449+
fixture.detectChanges();
450+
451+
const dragElement = fixture.componentInstance.dragElement.nativeElement;
452+
453+
// Dispatch a touch sequence.
454+
dispatchTouchEvent(dragElement, 'touchstart');
455+
fixture.detectChanges();
456+
dispatchTouchEvent(dragElement, 'touchend');
457+
fixture.detectChanges();
458+
tick();
459+
460+
// Immediately dispatch a mouse sequence to simulate a fake event.
461+
startDraggingViaMouse(fixture, dragElement);
462+
fixture.detectChanges();
463+
dispatchMouseEvent(dragElement, 'mouseup');
464+
fixture.detectChanges();
465+
tick();
466+
467+
expect(fixture.componentInstance.startedSpy).toHaveBeenCalledTimes(1);
468+
expect(fixture.componentInstance.endedSpy).toHaveBeenCalledTimes(1);
469+
}));
470+
471+
});
447472

448473
describe('draggable with a handle', () => {
449474
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
@@ -82,6 +82,14 @@ export function CDK_DRAG_CONFIG_FACTORY(): CdkDragConfig {
8282
/** Options that can be used to bind a passive event listener. */
8383
const passiveEventListenerOptions = normalizePassiveListenerOptions({passive: true});
8484

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

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

178192
/** Subscription to the stream that initializes the root element. */
179193
private _rootElementInitSubscription = Subscription.EMPTY;
@@ -347,8 +361,14 @@ export class CdkDrag<T = any> implements AfterViewInit, OnDestroy {
347361
// starting another sequence for a draggable parent somewhere up the DOM tree.
348362
event.stopPropagation();
349363

364+
const isDragging = this._isDragging();
365+
const isTouchEvent = this._isTouchEvent(event);
366+
const isAuxiliaryMouseButton = !isTouchEvent && (event as MouseEvent).button !== 0;
367+
const isSyntheticEvent = !isTouchEvent && this._lastTouchEventTime &&
368+
this._lastTouchEventTime + MOUSE_EVENT_IGNORE_TIME > Date.now();
369+
350370
// Abort if the user is already dragging or is using a mouse button other than the primary one.
351-
if (this._isDragging() || (!this._isTouchEvent(event) && event.button !== 0)) {
371+
if (isDragging || isAuxiliaryMouseButton || isSyntheticEvent) {
352372
return;
353373
}
354374

@@ -375,10 +395,14 @@ export class CdkDrag<T = any> implements AfterViewInit, OnDestroy {
375395
}
376396

377397
/** Starts the dragging sequence. */
378-
private _startDragSequence() {
398+
private _startDragSequence(event: MouseEvent | TouchEvent) {
379399
// Emit the event on the item before the one on the container.
380400
this.started.emit({source: this});
381401

402+
if (this._isTouchEvent(event)) {
403+
this._lastTouchEventTime = Date.now();
404+
}
405+
382406
if (this.dropContainer) {
383407
const element = this._rootElement;
384408

@@ -413,7 +437,7 @@ export class CdkDrag<T = any> implements AfterViewInit, OnDestroy {
413437
// per pixel of movement (e.g. if the user moves their pointer quickly).
414438
if (distanceX + distanceY >= this._config.dragStartThreshold) {
415439
this._hasStartedDragging = true;
416-
this._ngZone.run(() => this._startDragSequence());
440+
this._ngZone.run(() => this._startDragSequence(event));
417441
}
418442

419443
return;
@@ -473,10 +497,14 @@ export class CdkDrag<T = any> implements AfterViewInit, OnDestroy {
473497
this._passiveTransform.x = this._activeTransform.x;
474498
this._passiveTransform.y = this._activeTransform.y;
475499
this._ngZone.run(() => this.ended.emit({source: this}));
500+
this._dragDropRegistry.stopDragging(this);
476501
return;
477502
}
478503

479-
this._animatePreviewToPlaceholder().then(() => this._cleanupDragArtifacts());
504+
this._animatePreviewToPlaceholder().then(() => {
505+
this._cleanupDragArtifacts();
506+
this._dragDropRegistry.stopDragging(this);
507+
});
480508
}
481509

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

0 commit comments

Comments
 (0)