Skip to content

Commit 8b2dc82

Browse files
crisbetovivian-hu-zz
authored andcommitted
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.
1 parent 1e4ee0c commit 8b2dc82

File tree

2 files changed

+53
-7
lines changed

2 files changed

+53
-7
lines changed

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

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,19 @@ describe('DragDropRegistry', () => {
8888
subscription.unsubscribe();
8989
});
9090

91+
it('should dispatch pointer move events if event propagation is stopped', () => {
92+
const spy = jasmine.createSpy('pointerMove spy');
93+
const subscription = registry.pointerMove.subscribe(spy);
94+
95+
fixture.nativeElement.addEventListener('mousemove', (e: MouseEvent) => e.stopPropagation());
96+
registry.startDragging(testComponent.dragItems.first, createMouseEvent('mousedown'));
97+
dispatchMouseEvent(fixture.nativeElement.querySelector('div'), 'mousemove');
98+
99+
expect(spy).toHaveBeenCalled();
100+
101+
subscription.unsubscribe();
102+
});
103+
91104
it('should dispatch `mouseup` events after ending the drag via the mouse', () => {
92105
const spy = jasmine.createSpy('pointerUp spy');
93106
const subscription = registry.pointerUp.subscribe(spy);
@@ -113,6 +126,19 @@ describe('DragDropRegistry', () => {
113126
subscription.unsubscribe();
114127
});
115128

129+
it('should dispatch pointer up events if event propagation is stopped', () => {
130+
const spy = jasmine.createSpy('pointerUp spy');
131+
const subscription = registry.pointerUp.subscribe(spy);
132+
133+
fixture.nativeElement.addEventListener('mouseup', (e: MouseEvent) => e.stopPropagation());
134+
registry.startDragging(testComponent.dragItems.first, createMouseEvent('mousedown'));
135+
dispatchMouseEvent(fixture.nativeElement.querySelector('div'), 'mouseup');
136+
137+
expect(spy).toHaveBeenCalled();
138+
139+
subscription.unsubscribe();
140+
});
141+
116142
it('should complete the pointer event streams on destroy', () => {
117143
const pointerUpSpy = jasmine.createSpy('pointerUp complete spy');
118144
const pointerMoveSpy = jasmine.createSpy('pointerMove complete spy');
@@ -155,6 +181,17 @@ describe('DragDropRegistry', () => {
155181
expect(dispatchTouchEvent(document, 'touchmove').defaultPrevented).toBe(true);
156182
});
157183

184+
it('should prevent the default `touchmove` if event propagation is stopped', () => {
185+
registry.startDragging(testComponent.dragItems.first,
186+
createTouchEvent('touchstart') as TouchEvent);
187+
188+
fixture.nativeElement.addEventListener('touchmove', (e: TouchEvent) => e.stopPropagation());
189+
190+
const event = dispatchTouchEvent(fixture.nativeElement.querySelector('div'), 'touchmove');
191+
192+
expect(event.defaultPrevented).toBe(true);
193+
});
194+
158195
});
159196

160197
@Component({

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

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,11 @@ import {normalizePassiveListenerOptions} from '@angular/cdk/platform';
1212
import {Subject} from 'rxjs';
1313
import {toggleNativeDragInteractions} from './drag-styling';
1414

15-
/** Event options that can be used to bind an active event. */
16-
const activeEventOptions = normalizePassiveListenerOptions({passive: false});
15+
/** Event options that can be used to bind an active, capturing event. */
16+
const activeCapturingEventOptions = normalizePassiveListenerOptions({
17+
passive: false,
18+
capture: true
19+
});
1720

1821
/** Handler for a pointer event callback. */
1922
type PointerEventHandler = (event: TouchEvent | MouseEvent) => void;
@@ -42,7 +45,7 @@ export class DragDropRegistry<I, C extends {id: string}> implements OnDestroy {
4245
/** Keeps track of the event listeners that we've bound to the `document`. */
4346
private _globalListeners = new Map<'touchmove' | 'mousemove' | 'touchend' | 'mouseup', {
4447
handler: PointerEventHandler,
45-
options?: any
48+
options?: AddEventListenerOptions | boolean
4649
}>();
4750

4851
/**
@@ -83,7 +86,7 @@ export class DragDropRegistry<I, C extends {id: string}> implements OnDestroy {
8386
// The event handler has to be explicitly active, because
8487
// newer browsers make it passive by default.
8588
this._document.addEventListener('touchmove', this._preventScrollListener,
86-
activeEventOptions);
89+
activeCapturingEventOptions);
8790
});
8891
}
8992
}
@@ -100,7 +103,7 @@ export class DragDropRegistry<I, C extends {id: string}> implements OnDestroy {
100103

101104
if (this._dragInstances.size === 0) {
102105
this._document.removeEventListener('touchmove', this._preventScrollListener,
103-
activeEventOptions as any);
106+
activeCapturingEventOptions);
104107
}
105108
}
106109

@@ -125,8 +128,14 @@ export class DragDropRegistry<I, C extends {id: string}> implements OnDestroy {
125128
// passive ones for `mousemove` and `touchmove`. The events need to be active, because we
126129
// use `preventDefault` to prevent the page from scrolling while the user is dragging.
127130
this._globalListeners
128-
.set(moveEvent, {handler: e => this.pointerMove.next(e), options: activeEventOptions})
129-
.set(upEvent, {handler: e => this.pointerUp.next(e)})
131+
.set(moveEvent, {
132+
handler: e => this.pointerMove.next(e),
133+
options: activeCapturingEventOptions
134+
})
135+
.set(upEvent, {
136+
handler: e => this.pointerUp.next(e),
137+
options: true
138+
})
130139
.forEach((config, name) => {
131140
this._ngZone.runOutsideAngular(() => {
132141
this._document.addEventListener(name, config.handler, config.options);

0 commit comments

Comments
 (0)