From 8b2dc82592b52504cc48f248c9606996a4944f61 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Thu, 8 Nov 2018 00:06:45 +0000 Subject: [PATCH] fix(drag-drop): avoid disrupting drag sequence if event propagation is stopped (#13841) Since we only listen for events at the `document` level, the dragging sequence can get broken if the consumer stopped event propagation somewhere along the DOM tree. These changes switch to using event capturing in order to ensure that all the correct events fire. --- src/cdk/drag-drop/drag-drop-registry.spec.ts | 37 ++++++++++++++++++++ src/cdk/drag-drop/drag-drop-registry.ts | 23 ++++++++---- 2 files changed, 53 insertions(+), 7 deletions(-) diff --git a/src/cdk/drag-drop/drag-drop-registry.spec.ts b/src/cdk/drag-drop/drag-drop-registry.spec.ts index 66a9ad9b4011..4a9d24bf23e5 100644 --- a/src/cdk/drag-drop/drag-drop-registry.spec.ts +++ b/src/cdk/drag-drop/drag-drop-registry.spec.ts @@ -88,6 +88,19 @@ describe('DragDropRegistry', () => { subscription.unsubscribe(); }); + it('should dispatch pointer move events if event propagation is stopped', () => { + const spy = jasmine.createSpy('pointerMove spy'); + const subscription = registry.pointerMove.subscribe(spy); + + fixture.nativeElement.addEventListener('mousemove', (e: MouseEvent) => e.stopPropagation()); + registry.startDragging(testComponent.dragItems.first, createMouseEvent('mousedown')); + dispatchMouseEvent(fixture.nativeElement.querySelector('div'), 'mousemove'); + + expect(spy).toHaveBeenCalled(); + + subscription.unsubscribe(); + }); + it('should dispatch `mouseup` events after ending the drag via the mouse', () => { const spy = jasmine.createSpy('pointerUp spy'); const subscription = registry.pointerUp.subscribe(spy); @@ -113,6 +126,19 @@ describe('DragDropRegistry', () => { subscription.unsubscribe(); }); + it('should dispatch pointer up events if event propagation is stopped', () => { + const spy = jasmine.createSpy('pointerUp spy'); + const subscription = registry.pointerUp.subscribe(spy); + + fixture.nativeElement.addEventListener('mouseup', (e: MouseEvent) => e.stopPropagation()); + registry.startDragging(testComponent.dragItems.first, createMouseEvent('mousedown')); + dispatchMouseEvent(fixture.nativeElement.querySelector('div'), 'mouseup'); + + expect(spy).toHaveBeenCalled(); + + subscription.unsubscribe(); + }); + it('should complete the pointer event streams on destroy', () => { const pointerUpSpy = jasmine.createSpy('pointerUp complete spy'); const pointerMoveSpy = jasmine.createSpy('pointerMove complete spy'); @@ -155,6 +181,17 @@ describe('DragDropRegistry', () => { expect(dispatchTouchEvent(document, 'touchmove').defaultPrevented).toBe(true); }); + it('should prevent the default `touchmove` if event propagation is stopped', () => { + registry.startDragging(testComponent.dragItems.first, + createTouchEvent('touchstart') as TouchEvent); + + fixture.nativeElement.addEventListener('touchmove', (e: TouchEvent) => e.stopPropagation()); + + const event = dispatchTouchEvent(fixture.nativeElement.querySelector('div'), 'touchmove'); + + expect(event.defaultPrevented).toBe(true); + }); + }); @Component({ diff --git a/src/cdk/drag-drop/drag-drop-registry.ts b/src/cdk/drag-drop/drag-drop-registry.ts index 156128b6eabe..9dedfed0fd37 100644 --- a/src/cdk/drag-drop/drag-drop-registry.ts +++ b/src/cdk/drag-drop/drag-drop-registry.ts @@ -12,8 +12,11 @@ import {normalizePassiveListenerOptions} from '@angular/cdk/platform'; import {Subject} from 'rxjs'; import {toggleNativeDragInteractions} from './drag-styling'; -/** Event options that can be used to bind an active event. */ -const activeEventOptions = normalizePassiveListenerOptions({passive: false}); +/** Event options that can be used to bind an active, capturing event. */ +const activeCapturingEventOptions = normalizePassiveListenerOptions({ + passive: false, + capture: true +}); /** Handler for a pointer event callback. */ type PointerEventHandler = (event: TouchEvent | MouseEvent) => void; @@ -42,7 +45,7 @@ export class DragDropRegistry implements OnDestroy { /** Keeps track of the event listeners that we've bound to the `document`. */ private _globalListeners = new Map<'touchmove' | 'mousemove' | 'touchend' | 'mouseup', { handler: PointerEventHandler, - options?: any + options?: AddEventListenerOptions | boolean }>(); /** @@ -83,7 +86,7 @@ export class DragDropRegistry implements OnDestroy { // The event handler has to be explicitly active, because // newer browsers make it passive by default. this._document.addEventListener('touchmove', this._preventScrollListener, - activeEventOptions); + activeCapturingEventOptions); }); } } @@ -100,7 +103,7 @@ export class DragDropRegistry implements OnDestroy { if (this._dragInstances.size === 0) { this._document.removeEventListener('touchmove', this._preventScrollListener, - activeEventOptions as any); + activeCapturingEventOptions); } } @@ -125,8 +128,14 @@ export class DragDropRegistry implements OnDestroy { // passive ones for `mousemove` and `touchmove`. The events need to be active, because we // use `preventDefault` to prevent the page from scrolling while the user is dragging. this._globalListeners - .set(moveEvent, {handler: e => this.pointerMove.next(e), options: activeEventOptions}) - .set(upEvent, {handler: e => this.pointerUp.next(e)}) + .set(moveEvent, { + handler: e => this.pointerMove.next(e), + options: activeCapturingEventOptions + }) + .set(upEvent, { + handler: e => this.pointerUp.next(e), + options: true + }) .forEach((config, name) => { this._ngZone.runOutsideAngular(() => { this._document.addEventListener(name, config.handler, config.options);