Skip to content

fix(drag-n-drop): ignore consecutive touchmove events on drag item on multitouch #15923

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jun 3, 2019
Merged
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
128 changes: 128 additions & 0 deletions src/cdk/drag-drop/directives/drag.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3233,6 +3233,60 @@ describe('CdkDrag', () => {
'Expected target to have dragging class once item has been moved over.');
}));
});

describe('with nested drags', () => {
it('should not move draggable container when dragging child (multitouch)', fakeAsync(() => {

const fixture = createComponent(NestedDragsComponent, [], 10);
fixture.detectChanges();

// First finger drags container (less then threshold)
startDraggingViaTouch(fixture, fixture.componentInstance.container.nativeElement);
continueDraggingViaTouch(fixture, 2, 0);

// Second finger drags container
startDraggingViaTouch(fixture, fixture.componentInstance.container.nativeElement);
continueDraggingViaTouch(fixture, 0, 10);
continueDraggingViaTouch(fixture, 0, 20);

// First finger releases
stopDraggingViaTouch(fixture, 0, 20);
// Second finger releases
stopDraggingViaTouch(fixture, 0, 10);

// Container spies worked
const containerDragStartedCount =
fixture.componentInstance.containerDragStartedSpy.calls.count();
const containerDragMovedCount =
fixture.componentInstance.containerDragMovedSpy.calls.count();
const containerDragReleasedCount =
fixture.componentInstance.containerDragReleasedSpy.calls.count();

expect(containerDragStartedCount).toBeGreaterThan(0);
expect(containerDragMovedCount).toBeGreaterThan(0);
expect(containerDragReleasedCount).toBeGreaterThan(0);

// Drag item
startDraggingViaTouch(fixture, fixture.componentInstance.item.nativeElement);
continueDraggingViaTouch(fixture, 20, 20);
continueDraggingViaTouch(fixture, 40, 40);
stopDraggingViaTouch(fixture, 60, 60);

// Spies on item worked
expect(fixture.componentInstance.itemDragStartedSpy).toHaveBeenCalledTimes(1);
expect(fixture.componentInstance.itemDragMovedSpy).toHaveBeenCalledTimes(1);
expect(fixture.componentInstance.itemDragReleasedSpy).toHaveBeenCalledTimes(1);

// Spies on container stay intact
expect(fixture.componentInstance.containerDragStartedSpy)
.toHaveBeenCalledTimes(containerDragStartedCount);
expect(fixture.componentInstance.containerDragMovedSpy)
.toHaveBeenCalledTimes(containerDragMovedCount);
expect(fixture.componentInstance.containerDragReleasedSpy)
.toHaveBeenCalledTimes(containerDragReleasedCount);

}));
});
});

@Component({
Expand Down Expand Up @@ -3771,6 +3825,56 @@ class WrappedDropContainerComponent {
@Input() items: string[];
}

@Component({
styles: [`
:host {
height: 400px;
width: 400px;
position: absolute;
}
.container {
height: 200px;
width: 200px;
position: absolute;
}
.item {
height: 50px;
width: 50px;
position: absolute;
}
`],
template: `
<div
cdkDrag
#container
class="container"
(cdkDragStarted)="containerDragStartedSpy($event)"
(cdkDragMoved)="containerDragMovedSpy($event)"
(cdkDragReleased)="containerDragReleasedSpy($event)"
>
<div
cdkDrag
class="item"
#item
(cdkDragStarted)="itemDragStartedSpy($event)"
(cdkDragMoved)="itemDragMovedSpy($event)"
(cdkDragReleased)="itemDragReleasedSpy($event)"
>
</div>
</div>`
})
class NestedDragsComponent {
@ViewChild('container', {static: false}) container: ElementRef;
@ViewChild('item', {static: false}) item: ElementRef;

containerDragStartedSpy = jasmine.createSpy('container drag started spy');
containerDragMovedSpy = jasmine.createSpy('container drag moved spy');
containerDragReleasedSpy = jasmine.createSpy('container drag released spy');
itemDragStartedSpy = jasmine.createSpy('item drag started spy');
itemDragMovedSpy = jasmine.createSpy('item drag moved spy');
itemDragReleasedSpy = jasmine.createSpy('item drag released spy');
}

/**
* Drags an element to a position on the page using the mouse.
* @param fixture Fixture on which to run change detection.
Expand Down Expand Up @@ -3815,16 +3919,40 @@ function startDraggingViaMouse(fixture: ComponentFixture<any>,
*/
function dragElementViaTouch(fixture: ComponentFixture<any>,
element: Element, x: number, y: number) {
startDraggingViaTouch(fixture, element);
continueDraggingViaTouch(fixture, x, y);
stopDraggingViaTouch(fixture, x, y);
}

/**
* @param fixture Fixture on which to run change detection.
* @param element Element which is being dragged.
*/
function startDraggingViaTouch(fixture: ComponentFixture<any>,
element: Element) {
dispatchTouchEvent(element, 'touchstart');
fixture.detectChanges();

dispatchTouchEvent(document, 'touchmove');
fixture.detectChanges();
}

/**
* @param fixture Fixture on which to run change detection.
* @param x Position along the x axis to which to drag the element.
* @param y Position along the y axis to which to drag the element.
*/
function continueDraggingViaTouch(fixture: ComponentFixture<any>, x: number, y: number) {
dispatchTouchEvent(document, 'touchmove', x, y);
fixture.detectChanges();
}

/**
* @param fixture Fixture on which to run change detection.
* @param x Position along the x axis to which to drag the element.
* @param y Position along the y axis to which to drag the element.
*/
function stopDraggingViaTouch(fixture: ComponentFixture<any>, x: number, y: number) {
dispatchTouchEvent(document, 'touchend', x, y);
fixture.detectChanges();
}
Expand Down
31 changes: 31 additions & 0 deletions src/cdk/drag-drop/drag-drop-registry.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,37 @@ describe('DragDropRegistry', () => {
pointerMoveSubscription.unsubscribe();
});

it('should not emit pointer events when dragging is over (mutli touch)', () => {
const firstItem = testComponent.dragItems.first;

// First finger down
registry.startDragging(firstItem, createTouchEvent('touchstart') as TouchEvent);
// Second finger down
registry.startDragging(firstItem, createTouchEvent('touchstart') as TouchEvent);
// First finger up
registry.stopDragging(firstItem);

// Ensure dragging is over - registry is empty
expect(registry.isDragging(firstItem)).toBe(false);

const pointerUpSpy = jasmine.createSpy('pointerUp spy');
const pointerMoveSpy = jasmine.createSpy('pointerMove spy');

const pointerUpSubscription = registry.pointerUp.subscribe(pointerUpSpy);
const pointerMoveSubscription = registry.pointerMove.subscribe(pointerMoveSpy);

// Second finger keeps moving
dispatchTouchEvent(document, 'touchmove');
expect(pointerMoveSpy).not.toHaveBeenCalled();

// Second finger up
dispatchTouchEvent(document, 'touchend');
expect(pointerUpSpy).not.toHaveBeenCalled();

pointerUpSubscription.unsubscribe();
pointerMoveSubscription.unsubscribe();
});

it('should not throw when trying to register the same container again', () => {
expect(() => registry.registerDropContainer(testComponent.dropInstances.first)).not.toThrow();
});
Expand Down
5 changes: 5 additions & 0 deletions src/cdk/drag-drop/drag-drop-registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,11 @@ export class DragDropRegistry<I, C extends {id: string}> implements OnDestroy {
* @param event Event that initiated the dragging.
*/
startDragging(drag: I, event: TouchEvent | MouseEvent) {
// Do not process the same drag twice to avoid memory leaks and redundant listeners
if (this._activeDragInstances.has(drag)) {
return;
}

this._activeDragInstances.add(drag);

if (this._activeDragInstances.size === 1) {
Expand Down
5 changes: 5 additions & 0 deletions src/cdk/drag-drop/drag-ref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -689,8 +689,13 @@ export class DragRef<T = any> {
this._toggleNativeDragInteractions();
this._hasStartedDragging = this._hasMoved = false;
this._initialContainer = this._dropContainer!;

// Avoid multiple subscriptions and memory leaks when multi touch
// (isDragging check above isn't enough because of possible temporal and/or dimensional delays)
this._removeSubscriptions();
this._pointerMoveSubscription = this._dragDropRegistry.pointerMove.subscribe(this._pointerMove);
this._pointerUpSubscription = this._dragDropRegistry.pointerUp.subscribe(this._pointerUp);

this._scrollPosition = this._viewportRuler.getViewportScrollPosition();

if (this._boundaryElement) {
Expand Down