From e59c3021940a43357ea53db78a89cbde0ce31528 Mon Sep 17 00:00:00 2001 From: Antoine Boisier-Michaud Date: Tue, 16 Jul 2019 14:32:38 -0400 Subject: [PATCH] fix(drag-drop): fix drag start delay behavior to allow scrolling (#16224) The current implementation of the drag start delay does not allow scrolling on iOS devices. Also, once the element has been scrolled once, the scrolling is never reenabled. In order to fix this issue, we prevent default behaviour of pointer events fired after the drag sequence has been started instead of initialized. Also, we toggle native drag interactions when the drag sequence is ended. --- src/cdk/drag-drop/directives/drag.spec.ts | 18 ++++++++ src/cdk/drag-drop/drag-drop-registry.spec.ts | 44 +++++++++++++------- src/cdk/drag-drop/drag-drop-registry.ts | 31 ++++++++++---- src/cdk/drag-drop/drag-ref.ts | 11 ++++- tools/public_api_guard/cdk/drag-drop.d.ts | 3 +- 5 files changed, 81 insertions(+), 26 deletions(-) diff --git a/src/cdk/drag-drop/directives/drag.spec.ts b/src/cdk/drag-drop/directives/drag.spec.ts index e20ad712d793..03f80fe8217f 100644 --- a/src/cdk/drag-drop/directives/drag.spec.ts +++ b/src/cdk/drag-drop/directives/drag.spec.ts @@ -636,6 +636,24 @@ describe('CdkDrag', () => { expect(styles.touchAction || (styles as any).webkitUserDrag).toBe('none'); })); + it('should reenable native drag interactions after dragging', fakeAsync(() => { + const fixture = createComponent(StandaloneDraggable); + fixture.detectChanges(); + const dragElement = fixture.componentInstance.dragElement.nativeElement; + const styles = dragElement.style; + + expect(styles.touchAction || (styles as any).webkitUserDrag).toBeFalsy(); + + startDraggingViaMouse(fixture, dragElement); + dispatchMouseEvent(document, 'mousemove', 50, 100); + fixture.detectChanges(); + expect(styles.touchAction || (styles as any).webkitUserDrag).toBe('none'); + + dispatchMouseEvent(document, 'mouseup', 50, 100); + fixture.detectChanges(); + expect(styles.touchAction || (styles as any).webkitUserDrag).toBeFalsy(); + })); + it('should stop propagation for the drag sequence start event', fakeAsync(() => { const fixture = createComponent(StandaloneDraggable); fixture.detectChanges(); diff --git a/src/cdk/drag-drop/drag-drop-registry.spec.ts b/src/cdk/drag-drop/drag-drop-registry.spec.ts index 8af6d8407ae3..018273f6eccb 100644 --- a/src/cdk/drag-drop/drag-drop-registry.spec.ts +++ b/src/cdk/drag-drop/drag-drop-registry.spec.ts @@ -40,14 +40,14 @@ describe('DragDropRegistry', () => { const firstItem = testComponent.dragItems.first; expect(registry.isDragging(firstItem)).toBe(false); - registry.startDragging(firstItem, createMouseEvent('mousedown')); + registry.initializeDragging(firstItem, createMouseEvent('mousedown')); expect(registry.isDragging(firstItem)).toBe(true); }); it('should be able to stop dragging an item', () => { const firstItem = testComponent.dragItems.first; - registry.startDragging(firstItem, createMouseEvent('mousedown')); + registry.initializeDragging(firstItem, createMouseEvent('mousedown')); expect(registry.isDragging(firstItem)).toBe(true); registry.stopDragging(firstItem); @@ -57,7 +57,7 @@ describe('DragDropRegistry', () => { it('should stop dragging an item if it is removed', () => { const firstItem = testComponent.dragItems.first; - registry.startDragging(firstItem, createMouseEvent('mousedown')); + registry.initializeDragging(firstItem, createMouseEvent('mousedown')); expect(registry.isDragging(firstItem)).toBe(true); registry.removeDragItem(firstItem); @@ -68,7 +68,7 @@ describe('DragDropRegistry', () => { const spy = jasmine.createSpy('pointerMove spy'); const subscription = registry.pointerMove.subscribe(spy); - registry.startDragging(testComponent.dragItems.first, createMouseEvent('mousedown')); + registry.initializeDragging(testComponent.dragItems.first, createMouseEvent('mousedown')); dispatchMouseEvent(document, 'mousemove'); expect(spy).toHaveBeenCalled(); @@ -80,7 +80,7 @@ describe('DragDropRegistry', () => { const spy = jasmine.createSpy('pointerMove spy'); const subscription = registry.pointerMove.subscribe(spy); - registry.startDragging(testComponent.dragItems.first, + registry.initializeDragging(testComponent.dragItems.first, createTouchEvent('touchstart') as TouchEvent); dispatchTouchEvent(document, 'touchmove'); @@ -94,7 +94,7 @@ describe('DragDropRegistry', () => { const subscription = registry.pointerMove.subscribe(spy); fixture.nativeElement.addEventListener('mousemove', (e: MouseEvent) => e.stopPropagation()); - registry.startDragging(testComponent.dragItems.first, createMouseEvent('mousedown')); + registry.initializeDragging(testComponent.dragItems.first, createMouseEvent('mousedown')); dispatchMouseEvent(fixture.nativeElement.querySelector('div'), 'mousemove'); expect(spy).toHaveBeenCalled(); @@ -106,7 +106,7 @@ describe('DragDropRegistry', () => { const spy = jasmine.createSpy('pointerUp spy'); const subscription = registry.pointerUp.subscribe(spy); - registry.startDragging(testComponent.dragItems.first, createMouseEvent('mousedown')); + registry.initializeDragging(testComponent.dragItems.first, createMouseEvent('mousedown')); dispatchMouseEvent(document, 'mouseup'); expect(spy).toHaveBeenCalled(); @@ -118,7 +118,7 @@ describe('DragDropRegistry', () => { const spy = jasmine.createSpy('pointerUp spy'); const subscription = registry.pointerUp.subscribe(spy); - registry.startDragging(testComponent.dragItems.first, + registry.initializeDragging(testComponent.dragItems.first, createTouchEvent('touchstart') as TouchEvent); dispatchTouchEvent(document, 'touchend'); @@ -132,7 +132,7 @@ describe('DragDropRegistry', () => { const subscription = registry.pointerUp.subscribe(spy); fixture.nativeElement.addEventListener('mouseup', (e: MouseEvent) => e.stopPropagation()); - registry.startDragging(testComponent.dragItems.first, createMouseEvent('mousedown')); + registry.initializeDragging(testComponent.dragItems.first, createMouseEvent('mousedown')); dispatchMouseEvent(fixture.nativeElement.querySelector('div'), 'mouseup'); expect(spy).toHaveBeenCalled(); @@ -159,9 +159,9 @@ describe('DragDropRegistry', () => { const firstItem = testComponent.dragItems.first; // First finger down - registry.startDragging(firstItem, createTouchEvent('touchstart') as TouchEvent); + registry.initializeDragging(firstItem, createTouchEvent('touchstart') as TouchEvent); // Second finger down - registry.startDragging(firstItem, createTouchEvent('touchstart') as TouchEvent); + registry.initializeDragging(firstItem, createTouchEvent('touchstart') as TouchEvent); // First finger up registry.stopDragging(firstItem); @@ -194,15 +194,23 @@ describe('DragDropRegistry', () => { expect(dispatchTouchEvent(document, 'touchmove').defaultPrevented).toBe(false); }); + it('should not revent the default `touchmove` action when an item is only initialized', () => { + registry.initializeDragging(testComponent.dragItems.first, + createTouchEvent('touchstart') as TouchEvent); + expect(dispatchTouchEvent(document, 'touchmove').defaultPrevented).toBe(false); + }); + it('should prevent the default `touchmove` action when an item is being dragged', () => { - registry.startDragging(testComponent.dragItems.first, + registry.initializeDragging(testComponent.dragItems.first, createTouchEvent('touchstart') as TouchEvent); + registry.startDragging(testComponent.dragItems.first); expect(dispatchTouchEvent(document, 'touchmove').defaultPrevented).toBe(true); }); it('should prevent the default `touchmove` if event propagation is stopped', () => { - registry.startDragging(testComponent.dragItems.first, + registry.initializeDragging(testComponent.dragItems.first, createTouchEvent('touchstart') as TouchEvent); + registry.startDragging(testComponent.dragItems.first); fixture.nativeElement.addEventListener('touchmove', (e: TouchEvent) => e.stopPropagation()); @@ -215,8 +223,14 @@ describe('DragDropRegistry', () => { expect(dispatchFakeEvent(document, 'selectstart').defaultPrevented).toBe(false); }); + it('should not prevent the default `selectstart` action when an item is initialized', () => { + registry.initializeDragging(testComponent.dragItems.first, createMouseEvent('mousedown')); + expect(dispatchFakeEvent(document, 'selectstart').defaultPrevented).toBe(false); + }); + it('should prevent the default `selectstart` action when an item is being dragged', () => { - registry.startDragging(testComponent.dragItems.first, createMouseEvent('mousedown')); + registry.initializeDragging(testComponent.dragItems.first, createMouseEvent('mousedown')); + registry.startDragging(testComponent.dragItems.first); expect(dispatchFakeEvent(document, 'selectstart').defaultPrevented).toBe(true); }); @@ -224,7 +238,7 @@ describe('DragDropRegistry', () => { const spy = jasmine.createSpy('scroll spy'); const subscription = registry.scroll.subscribe(spy); - registry.startDragging(testComponent.dragItems.first, createMouseEvent('mousedown')); + registry.initializeDragging(testComponent.dragItems.first, createMouseEvent('mousedown')); dispatchFakeEvent(document, 'scroll'); expect(spy).toHaveBeenCalled(); diff --git a/src/cdk/drag-drop/drag-drop-registry.ts b/src/cdk/drag-drop/drag-drop-registry.ts index 7a46f667b696..30fb83cfe99a 100644 --- a/src/cdk/drag-drop/drag-drop-registry.ts +++ b/src/cdk/drag-drop/drag-drop-registry.ts @@ -35,8 +35,11 @@ export class DragDropRegistry implements OnDestroy { /** Registered drag item instances. */ private _dragInstances = new Set(); + /** Drag item instances for which a drag sequence has been initialized. */ + private _initializedDragSequences = new Set(); + /** Drag item instances that are currently being dragged. */ - private _activeDragInstances = new Set(); + private _startedDragSequences = new Set(); /** Keeps track of the event listeners that we've bound to the `document`. */ private _globalListeners = new Map implements OnDestroy { * @param drag Drag instance which is being dragged. * @param event Event that initiated the dragging. */ - startDragging(drag: I, event: TouchEvent | MouseEvent) { + initializeDragging(drag: I, event: TouchEvent | MouseEvent) { // Do not process the same drag twice to avoid memory leaks and redundant listeners - if (this._activeDragInstances.has(drag)) { + if (this._initializedDragSequences.has(drag)) { return; } - this._activeDragInstances.add(drag); + this._initializedDragSequences.add(drag); - if (this._activeDragInstances.size === 1) { + if (this._initializedDragSequences.size === 1) { const isTouchEvent = event.type.startsWith('touch'); const moveEvent = isTouchEvent ? 'touchmove' : 'mousemove'; const upEvent = isTouchEvent ? 'touchend' : 'mouseup'; @@ -159,18 +162,28 @@ export class DragDropRegistry implements OnDestroy { } } + startDragging(drag: I) { + if (this._startedDragSequences.has(drag)) { + return; + } + + this._startedDragSequences.add(drag); + } + /** Stops dragging a drag item instance. */ stopDragging(drag: I) { - this._activeDragInstances.delete(drag); - if (this._activeDragInstances.size === 0) { + this._startedDragSequences.delete(drag); + this._initializedDragSequences.delete(drag); + + if (this._initializedDragSequences.size === 0) { this._clearGlobalListeners(); } } /** Gets whether a drag item instance is currently being dragged. */ isDragging(drag: I) { - return this._activeDragInstances.has(drag); + return this._initializedDragSequences.has(drag); } /** @@ -195,7 +208,7 @@ export class DragDropRegistry implements OnDestroy { * @param event Event whose default action should be prevented. */ private _preventDefaultWhileDragging = (event: Event) => { - if (this._activeDragInstances.size) { + if (this._startedDragSequences.size) { event.preventDefault(); } } diff --git a/src/cdk/drag-drop/drag-ref.ts b/src/cdk/drag-drop/drag-ref.ts index 699cc138156a..06e97076f925 100644 --- a/src/cdk/drag-drop/drag-ref.ts +++ b/src/cdk/drag-drop/drag-ref.ts @@ -530,6 +530,11 @@ export class DragRef { return; } + // We need to prevent default here in case the pointer move starts a scroll sequence. + // If we do not prevent the scroll from starting, then we won't be able to prevent future + // touchemove events. + event.preventDefault(); + // Prevent other drag sequences from starting while something in the container is still // being dragged. This can happen while we're waiting for the drop animation to finish // and can cause errors, because some elements might still be moving around. @@ -600,6 +605,8 @@ export class DragRef { * @param event Browser event object that ended the sequence. */ private _endDragSequence(event: MouseEvent | TouchEvent) { + toggleNativeDragInteractions(this._rootElement, true); + // Note that here we use `isDragging` from the service, rather than from `this`. // The difference is that the one from the service reflects whether a dragging sequence // has been initiated, whereas the one on `this` includes whether the user has passed @@ -646,6 +653,8 @@ export class DragRef { /** Starts the dragging sequence. */ private _startDragSequence(event: MouseEvent | TouchEvent) { + this._dragDropRegistry.startDragging(this); + // Emit the event on the item before the one on the container. this.started.next({source: this}); @@ -742,7 +751,7 @@ export class DragRef { this._pointerDirectionDelta = {x: 0, y: 0}; this._pointerPositionAtLastDirectionChange = {x: pointerPosition.x, y: pointerPosition.y}; this._dragStartTime = Date.now(); - this._dragDropRegistry.startDragging(this, event); + this._dragDropRegistry.initializeDragging(this, event); } /** Cleans up the DOM artifacts that were added to facilitate the element being dragged. */ diff --git a/tools/public_api_guard/cdk/drag-drop.d.ts b/tools/public_api_guard/cdk/drag-drop.d.ts index 8cf666859fb5..c955cf010338 100644 --- a/tools/public_api_guard/cdk/drag-drop.d.ts +++ b/tools/public_api_guard/cdk/drag-drop.d.ts @@ -215,13 +215,14 @@ export declare class DragDropRegistry; constructor(_ngZone: NgZone, _document: any); getDropContainer(id: string): C | undefined; + initializeDragging(drag: I, event: TouchEvent | MouseEvent): void; isDragging(drag: I): boolean; ngOnDestroy(): void; registerDragItem(drag: I): void; registerDropContainer(drop: C): void; removeDragItem(drag: I): void; removeDropContainer(drop: C): void; - startDragging(drag: I, event: TouchEvent | MouseEvent): void; + startDragging(drag: I): void; stopDragging(drag: I): void; }