Skip to content

Commit 3b1ae05

Browse files
anotherpitjosephperrott
authored andcommitted
fix(drag-n-drop): ignore consecutive touchmove events on drag item on multitouch (#15923)
1 parent 15038e3 commit 3b1ae05

File tree

4 files changed

+169
-0
lines changed

4 files changed

+169
-0
lines changed

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

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3467,6 +3467,60 @@ describe('CdkDrag', () => {
34673467
}));
34683468

34693469
});
3470+
3471+
describe('with nested drags', () => {
3472+
it('should not move draggable container when dragging child (multitouch)', fakeAsync(() => {
3473+
3474+
const fixture = createComponent(NestedDragsComponent, [], 10);
3475+
fixture.detectChanges();
3476+
3477+
// First finger drags container (less then threshold)
3478+
startDraggingViaTouch(fixture, fixture.componentInstance.container.nativeElement);
3479+
continueDraggingViaTouch(fixture, 2, 0);
3480+
3481+
// Second finger drags container
3482+
startDraggingViaTouch(fixture, fixture.componentInstance.container.nativeElement);
3483+
continueDraggingViaTouch(fixture, 0, 10);
3484+
continueDraggingViaTouch(fixture, 0, 20);
3485+
3486+
// First finger releases
3487+
stopDraggingViaTouch(fixture, 0, 20);
3488+
// Second finger releases
3489+
stopDraggingViaTouch(fixture, 0, 10);
3490+
3491+
// Container spies worked
3492+
const containerDragStartedCount =
3493+
fixture.componentInstance.containerDragStartedSpy.calls.count();
3494+
const containerDragMovedCount =
3495+
fixture.componentInstance.containerDragMovedSpy.calls.count();
3496+
const containerDragReleasedCount =
3497+
fixture.componentInstance.containerDragReleasedSpy.calls.count();
3498+
3499+
expect(containerDragStartedCount).toBeGreaterThan(0);
3500+
expect(containerDragMovedCount).toBeGreaterThan(0);
3501+
expect(containerDragReleasedCount).toBeGreaterThan(0);
3502+
3503+
// Drag item
3504+
startDraggingViaTouch(fixture, fixture.componentInstance.item.nativeElement);
3505+
continueDraggingViaTouch(fixture, 20, 20);
3506+
continueDraggingViaTouch(fixture, 40, 40);
3507+
stopDraggingViaTouch(fixture, 60, 60);
3508+
3509+
// Spies on item worked
3510+
expect(fixture.componentInstance.itemDragStartedSpy).toHaveBeenCalledTimes(1);
3511+
expect(fixture.componentInstance.itemDragMovedSpy).toHaveBeenCalledTimes(1);
3512+
expect(fixture.componentInstance.itemDragReleasedSpy).toHaveBeenCalledTimes(1);
3513+
3514+
// Spies on container stay intact
3515+
expect(fixture.componentInstance.containerDragStartedSpy)
3516+
.toHaveBeenCalledTimes(containerDragStartedCount);
3517+
expect(fixture.componentInstance.containerDragMovedSpy)
3518+
.toHaveBeenCalledTimes(containerDragMovedCount);
3519+
expect(fixture.componentInstance.containerDragReleasedSpy)
3520+
.toHaveBeenCalledTimes(containerDragReleasedCount);
3521+
3522+
}));
3523+
});
34703524
});
34713525

34723526
@Component({
@@ -4063,6 +4117,56 @@ class WrappedDropContainerComponent {
40634117
@Input() items: string[];
40644118
}
40654119

4120+
@Component({
4121+
styles: [`
4122+
:host {
4123+
height: 400px;
4124+
width: 400px;
4125+
position: absolute;
4126+
}
4127+
.container {
4128+
height: 200px;
4129+
width: 200px;
4130+
position: absolute;
4131+
}
4132+
.item {
4133+
height: 50px;
4134+
width: 50px;
4135+
position: absolute;
4136+
}
4137+
`],
4138+
template: `
4139+
<div
4140+
cdkDrag
4141+
#container
4142+
class="container"
4143+
(cdkDragStarted)="containerDragStartedSpy($event)"
4144+
(cdkDragMoved)="containerDragMovedSpy($event)"
4145+
(cdkDragReleased)="containerDragReleasedSpy($event)"
4146+
>
4147+
<div
4148+
cdkDrag
4149+
class="item"
4150+
#item
4151+
(cdkDragStarted)="itemDragStartedSpy($event)"
4152+
(cdkDragMoved)="itemDragMovedSpy($event)"
4153+
(cdkDragReleased)="itemDragReleasedSpy($event)"
4154+
>
4155+
</div>
4156+
</div>`
4157+
})
4158+
class NestedDragsComponent {
4159+
@ViewChild('container', {static: false}) container: ElementRef;
4160+
@ViewChild('item', {static: false}) item: ElementRef;
4161+
4162+
containerDragStartedSpy = jasmine.createSpy('container drag started spy');
4163+
containerDragMovedSpy = jasmine.createSpy('container drag moved spy');
4164+
containerDragReleasedSpy = jasmine.createSpy('container drag released spy');
4165+
itemDragStartedSpy = jasmine.createSpy('item drag started spy');
4166+
itemDragMovedSpy = jasmine.createSpy('item drag moved spy');
4167+
itemDragReleasedSpy = jasmine.createSpy('item drag released spy');
4168+
}
4169+
40664170
/**
40674171
* Drags an element to a position on the page using the mouse.
40684172
* @param fixture Fixture on which to run change detection.
@@ -4107,16 +4211,40 @@ function startDraggingViaMouse(fixture: ComponentFixture<any>,
41074211
*/
41084212
function dragElementViaTouch(fixture: ComponentFixture<any>,
41094213
element: Element, x: number, y: number) {
4214+
startDraggingViaTouch(fixture, element);
4215+
continueDraggingViaTouch(fixture, x, y);
4216+
stopDraggingViaTouch(fixture, x, y);
4217+
}
41104218

4219+
/**
4220+
* @param fixture Fixture on which to run change detection.
4221+
* @param element Element which is being dragged.
4222+
*/
4223+
function startDraggingViaTouch(fixture: ComponentFixture<any>,
4224+
element: Element) {
41114225
dispatchTouchEvent(element, 'touchstart');
41124226
fixture.detectChanges();
41134227

41144228
dispatchTouchEvent(document, 'touchmove');
41154229
fixture.detectChanges();
4230+
}
41164231

4232+
/**
4233+
* @param fixture Fixture on which to run change detection.
4234+
* @param x Position along the x axis to which to drag the element.
4235+
* @param y Position along the y axis to which to drag the element.
4236+
*/
4237+
function continueDraggingViaTouch(fixture: ComponentFixture<any>, x: number, y: number) {
41174238
dispatchTouchEvent(document, 'touchmove', x, y);
41184239
fixture.detectChanges();
4240+
}
41194241

4242+
/**
4243+
* @param fixture Fixture on which to run change detection.
4244+
* @param x Position along the x axis to which to drag the element.
4245+
* @param y Position along the y axis to which to drag the element.
4246+
*/
4247+
function stopDraggingViaTouch(fixture: ComponentFixture<any>, x: number, y: number) {
41204248
dispatchTouchEvent(document, 'touchend', x, y);
41214249
fixture.detectChanges();
41224250
}

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

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,37 @@ describe('DragDropRegistry', () => {
156156
pointerMoveSubscription.unsubscribe();
157157
});
158158

159+
it('should not emit pointer events when dragging is over (mutli touch)', () => {
160+
const firstItem = testComponent.dragItems.first;
161+
162+
// First finger down
163+
registry.startDragging(firstItem, createTouchEvent('touchstart') as TouchEvent);
164+
// Second finger down
165+
registry.startDragging(firstItem, createTouchEvent('touchstart') as TouchEvent);
166+
// First finger up
167+
registry.stopDragging(firstItem);
168+
169+
// Ensure dragging is over - registry is empty
170+
expect(registry.isDragging(firstItem)).toBe(false);
171+
172+
const pointerUpSpy = jasmine.createSpy('pointerUp spy');
173+
const pointerMoveSpy = jasmine.createSpy('pointerMove spy');
174+
175+
const pointerUpSubscription = registry.pointerUp.subscribe(pointerUpSpy);
176+
const pointerMoveSubscription = registry.pointerMove.subscribe(pointerMoveSpy);
177+
178+
// Second finger keeps moving
179+
dispatchTouchEvent(document, 'touchmove');
180+
expect(pointerMoveSpy).not.toHaveBeenCalled();
181+
182+
// Second finger up
183+
dispatchTouchEvent(document, 'touchend');
184+
expect(pointerUpSpy).not.toHaveBeenCalled();
185+
186+
pointerUpSubscription.unsubscribe();
187+
pointerMoveSubscription.unsubscribe();
188+
});
189+
159190
it('should not throw when trying to register the same container again', () => {
160191
expect(() => registry.registerDropContainer(testComponent.dropInstances.first)).not.toThrow();
161192
});

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,11 @@ export class DragDropRegistry<I, C extends {id: string}> implements OnDestroy {
112112
* @param event Event that initiated the dragging.
113113
*/
114114
startDragging(drag: I, event: TouchEvent | MouseEvent) {
115+
// Do not process the same drag twice to avoid memory leaks and redundant listeners
116+
if (this._activeDragInstances.has(drag)) {
117+
return;
118+
}
119+
115120
this._activeDragInstances.add(drag);
116121

117122
if (this._activeDragInstances.size === 1) {

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -689,8 +689,13 @@ export class DragRef<T = any> {
689689
this._toggleNativeDragInteractions();
690690
this._hasStartedDragging = this._hasMoved = false;
691691
this._initialContainer = this._dropContainer!;
692+
693+
// Avoid multiple subscriptions and memory leaks when multi touch
694+
// (isDragging check above isn't enough because of possible temporal and/or dimensional delays)
695+
this._removeSubscriptions();
692696
this._pointerMoveSubscription = this._dragDropRegistry.pointerMove.subscribe(this._pointerMove);
693697
this._pointerUpSubscription = this._dragDropRegistry.pointerUp.subscribe(this._pointerUp);
698+
694699
this._scrollPosition = this._viewportRuler.getViewportScrollPosition();
695700

696701
if (this._boundaryElement) {

0 commit comments

Comments
 (0)