Skip to content

Commit

Permalink
fix(drag-drop): events fired multiple times for short drag sequences …
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
crisbeto authored and andrewseguin committed Nov 14, 2018
1 parent d7a8892 commit dc0b51a
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 5 deletions.
27 changes: 26 additions & 1 deletion src/cdk/drag-drop/drag.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -508,7 +508,32 @@ describe('CdkDrag', () => {
expect(dragElement.style.transform).toBe('translate3d(25px, 50px, 0px)');
}));

});
it('should not dispatch multiple events for a mouse event right after a touch event',
fakeAsync(() => {
const fixture = createComponent(StandaloneDraggable);
fixture.detectChanges();

const dragElement = fixture.componentInstance.dragElement.nativeElement;

// Dispatch a touch sequence.
dispatchTouchEvent(dragElement, 'touchstart');
fixture.detectChanges();
dispatchTouchEvent(dragElement, 'touchend');
fixture.detectChanges();
tick();

// Immediately dispatch a mouse sequence to simulate a fake event.
startDraggingViaMouse(fixture, dragElement);
fixture.detectChanges();
dispatchMouseEvent(dragElement, 'mouseup');
fixture.detectChanges();
tick();

expect(fixture.componentInstance.startedSpy).toHaveBeenCalledTimes(1);
expect(fixture.componentInstance.endedSpy).toHaveBeenCalledTimes(1);
}));

});

describe('draggable with a handle', () => {
it('should not be able to drag the entire element if it has a handle', fakeAsync(() => {
Expand Down
36 changes: 32 additions & 4 deletions src/cdk/drag-drop/drag.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,14 @@ const passiveEventListenerOptions = normalizePassiveListenerOptions({passive: tr
/** Options that can be used to bind an active event listener. */
const activeEventListenerOptions = normalizePassiveListenerOptions({passive: false});

/**
* Time in milliseconds for which to ignore mouse events, after
* receiving a touch event. Used to avoid doing double work for
* touch devices where the browser fires fake mouse events, in
* addition to touch events.
*/
const MOUSE_EVENT_IGNORE_TIME = 800;

/** Element that can be moved inside a CdkDropList container. */
@Directive({
selector: '[cdkDrag]',
Expand Down Expand Up @@ -177,6 +185,12 @@ export class CdkDrag<T = any> implements AfterViewInit, OnDestroy {

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

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

const isDragging = this._isDragging();
const isTouchEvent = this._isTouchEvent(event);
const isAuxiliaryMouseButton = !isTouchEvent && (event as MouseEvent).button !== 0;
const isSyntheticEvent = !isTouchEvent && this._lastTouchEventTime &&
this._lastTouchEventTime + MOUSE_EVENT_IGNORE_TIME > Date.now();

// If the event started from an element with the native HTML drag&drop, it'll interfere
// with our own dragging (e.g. `img` tags do it by default). Prevent the default action
// to stop it from happening. Note that preventing on `dragstart` also seems to work, but
Expand All @@ -368,7 +388,7 @@ export class CdkDrag<T = any> implements AfterViewInit, OnDestroy {
}

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

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

/** Starts the dragging sequence. */
private _startDragSequence() {
private _startDragSequence(event: MouseEvent | TouchEvent) {
// Emit the event on the item before the one on the container.
this.started.emit({source: this});

if (this._isTouchEvent(event)) {
this._lastTouchEventTime = Date.now();
}

if (this.dropContainer) {
const element = this._rootElement;

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

return;
Expand Down Expand Up @@ -493,10 +517,14 @@ export class CdkDrag<T = any> implements AfterViewInit, OnDestroy {
this._passiveTransform.x = this._activeTransform.x;
this._passiveTransform.y = this._activeTransform.y;
this._ngZone.run(() => this.ended.emit({source: this}));
this._dragDropRegistry.stopDragging(this);
return;
}

this._animatePreviewToPlaceholder().then(() => this._cleanupDragArtifacts());
this._animatePreviewToPlaceholder().then(() => {
this._cleanupDragArtifacts();
this._dragDropRegistry.stopDragging(this);
});
}

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

0 comments on commit dc0b51a

Please sign in to comment.