diff --git a/src/cdk/drag-drop/directives/drag.spec.ts b/src/cdk/drag-drop/directives/drag.spec.ts index ad79d4585b6c..eda4f332dbe1 100644 --- a/src/cdk/drag-drop/directives/drag.spec.ts +++ b/src/cdk/drag-drop/directives/drag.spec.ts @@ -318,6 +318,7 @@ describe('CdkDrag', () => { dispatchTouchEvent(fixture.componentInstance.dragElement.nativeElement, 'touchstart'); fixture.detectChanges(); + dispatchTouchEvent(document, 'touchmove'); expect(dispatchTouchEvent(document, 'touchmove').defaultPrevented).toBe(true); dispatchTouchEvent(document, 'touchend'); @@ -1059,6 +1060,24 @@ describe('CdkDrag', () => { 'Expected element to be dragged after all the time has passed.'); })); + it('should not prevent the default touch action before the delay has elapsed', fakeAsync(() => { + spyOn(Date, 'now').and.callFake(() => currentTime); + let currentTime = 0; + + const fixture = createComponent(StandaloneDraggable); + fixture.componentInstance.dragStartDelay = 500; + fixture.detectChanges(); + const dragElement = fixture.componentInstance.dragElement.nativeElement; + + expect(dragElement.style.transform).toBeFalsy('Expected element not to be moved by default.'); + + dispatchTouchEvent(dragElement, 'touchstart'); + fixture.detectChanges(); + currentTime += 250; + + expect(dispatchTouchEvent(document, 'touchmove', 50, 100).defaultPrevented).toBe(false); + })); + it('should handle the drag delay as a string', fakeAsync(() => { // We can't use Jasmine's `clock` because Zone.js interferes with it. spyOn(Date, 'now').and.callFake(() => currentTime); @@ -1206,34 +1225,6 @@ describe('CdkDrag', () => { subscription.unsubscribe(); })); - it('should prevent the default `mousemove` action even before the drag threshold has ' + - 'been reached', fakeAsync(() => { - const fixture = createComponent(StandaloneDraggable, [], 5); - fixture.detectChanges(); - const dragElement = fixture.componentInstance.dragElement.nativeElement; - - dispatchMouseEvent(dragElement, 'mousedown', 2, 2); - fixture.detectChanges(); - const mousemoveEvent = dispatchMouseEvent(document, 'mousemove', 2, 2); - fixture.detectChanges(); - - expect(mousemoveEvent.defaultPrevented).toBe(true); - })); - - it('should prevent the default `touchmove` action even before the drag threshold has ' + - 'been reached', fakeAsync(() => { - const fixture = createComponent(StandaloneDraggable, [], 5); - fixture.detectChanges(); - const dragElement = fixture.componentInstance.dragElement.nativeElement; - - dispatchTouchEvent(dragElement, 'touchstart', 2, 2); - fixture.detectChanges(); - const touchmoveEvent = dispatchTouchEvent(document, 'touchmove', 2, 2); - fixture.detectChanges(); - - expect(touchmoveEvent.defaultPrevented).toBe(true); - })); - it('should be able to configure the drag input defaults through a provider', fakeAsync(() => { const config: DragDropConfig = { draggingDisabled: true, diff --git a/src/cdk/drag-drop/drag-drop-registry.spec.ts b/src/cdk/drag-drop/drag-drop-registry.spec.ts index 27f7158d9416..9eca66fc68e9 100644 --- a/src/cdk/drag-drop/drag-drop-registry.spec.ts +++ b/src/cdk/drag-drop/drag-drop-registry.spec.ts @@ -1,4 +1,4 @@ -import {QueryList, ViewChildren, Component} from '@angular/core'; +import {Component} from '@angular/core'; import {fakeAsync, TestBed, ComponentFixture, inject} from '@angular/core/testing'; import { createMouseEvent, @@ -9,25 +9,21 @@ import { } from '@angular/cdk/testing/private'; import {DragDropRegistry} from './drag-drop-registry'; import {DragDropModule} from './drag-drop-module'; -import {CdkDrag} from './directives/drag'; -import {CdkDropList} from './directives/drop-list'; describe('DragDropRegistry', () => { - let fixture: ComponentFixture; - let testComponent: SimpleDropZone; - let registry: DragDropRegistry; + let fixture: ComponentFixture; + let registry: DragDropRegistry; beforeEach(fakeAsync(() => { TestBed.configureTestingModule({ imports: [DragDropModule], - declarations: [SimpleDropZone], + declarations: [BlankComponent], }).compileComponents(); - fixture = TestBed.createComponent(SimpleDropZone); - testComponent = fixture.componentInstance; + fixture = TestBed.createComponent(BlankComponent); fixture.detectChanges(); - inject([DragDropRegistry], (c: DragDropRegistry) => { + inject([DragDropRegistry], (c: DragDropRegistry) => { registry = c; })(); })); @@ -37,38 +33,38 @@ describe('DragDropRegistry', () => { }); it('should be able to start dragging an item', () => { - const firstItem = testComponent.dragItems.first; + const item = new DragItem(); - expect(registry.isDragging(firstItem)).toBe(false); - registry.startDragging(firstItem, createMouseEvent('mousedown')); - expect(registry.isDragging(firstItem)).toBe(true); + expect(registry.isDragging(item)).toBe(false); + registry.startDragging(item, createMouseEvent('mousedown')); + expect(registry.isDragging(item)).toBe(true); }); it('should be able to stop dragging an item', () => { - const firstItem = testComponent.dragItems.first; + const item = new DragItem(); - registry.startDragging(firstItem, createMouseEvent('mousedown')); - expect(registry.isDragging(firstItem)).toBe(true); + registry.startDragging(item, createMouseEvent('mousedown')); + expect(registry.isDragging(item)).toBe(true); - registry.stopDragging(firstItem); - expect(registry.isDragging(firstItem)).toBe(false); + registry.stopDragging(item); + expect(registry.isDragging(item)).toBe(false); }); it('should stop dragging an item if it is removed', () => { - const firstItem = testComponent.dragItems.first; + const item = new DragItem(); - registry.startDragging(firstItem, createMouseEvent('mousedown')); - expect(registry.isDragging(firstItem)).toBe(true); + registry.startDragging(item, createMouseEvent('mousedown')); + expect(registry.isDragging(item)).toBe(true); - registry.removeDragItem(firstItem); - expect(registry.isDragging(firstItem)).toBe(false); + registry.removeDragItem(item); + expect(registry.isDragging(item)).toBe(false); }); it('should dispatch `mousemove` events after starting to drag via the mouse', () => { const spy = jasmine.createSpy('pointerMove spy'); const subscription = registry.pointerMove.subscribe(spy); - - registry.startDragging(testComponent.dragItems.first, createMouseEvent('mousedown')); + const item = new DragItem(true); + registry.startDragging(item, createMouseEvent('mousedown')); dispatchMouseEvent(document, 'mousemove'); expect(spy).toHaveBeenCalled(); @@ -79,9 +75,8 @@ describe('DragDropRegistry', () => { it('should dispatch `touchmove` events after starting to drag via touch', () => { const spy = jasmine.createSpy('pointerMove spy'); const subscription = registry.pointerMove.subscribe(spy); - - registry.startDragging(testComponent.dragItems.first, - createTouchEvent('touchstart') as TouchEvent); + const item = new DragItem(true); + registry.startDragging(item, createTouchEvent('touchstart') as TouchEvent); dispatchTouchEvent(document, 'touchmove'); expect(spy).toHaveBeenCalled(); @@ -92,10 +87,10 @@ describe('DragDropRegistry', () => { it('should dispatch pointer move events if event propagation is stopped', () => { const spy = jasmine.createSpy('pointerMove spy'); const subscription = registry.pointerMove.subscribe(spy); - + const item = new DragItem(true); fixture.nativeElement.addEventListener('mousemove', (e: MouseEvent) => e.stopPropagation()); - registry.startDragging(testComponent.dragItems.first, createMouseEvent('mousedown')); - dispatchMouseEvent(fixture.nativeElement.querySelector('div'), 'mousemove'); + registry.startDragging(item, createMouseEvent('mousedown')); + dispatchMouseEvent(fixture.nativeElement, 'mousemove'); expect(spy).toHaveBeenCalled(); @@ -105,8 +100,9 @@ describe('DragDropRegistry', () => { it('should dispatch `mouseup` events after ending the drag via the mouse', () => { const spy = jasmine.createSpy('pointerUp spy'); const subscription = registry.pointerUp.subscribe(spy); + const item = new DragItem(); - registry.startDragging(testComponent.dragItems.first, createMouseEvent('mousedown')); + registry.startDragging(item, createMouseEvent('mousedown')); dispatchMouseEvent(document, 'mouseup'); expect(spy).toHaveBeenCalled(); @@ -117,9 +113,9 @@ describe('DragDropRegistry', () => { it('should dispatch `touchend` events after ending the drag via touch', () => { const spy = jasmine.createSpy('pointerUp spy'); const subscription = registry.pointerUp.subscribe(spy); + const item = new DragItem(); - registry.startDragging(testComponent.dragItems.first, - createTouchEvent('touchstart') as TouchEvent); + registry.startDragging(item, createTouchEvent('touchstart') as TouchEvent); dispatchTouchEvent(document, 'touchend'); expect(spy).toHaveBeenCalled(); @@ -130,10 +126,11 @@ describe('DragDropRegistry', () => { it('should dispatch pointer up events if event propagation is stopped', () => { const spy = jasmine.createSpy('pointerUp spy'); const subscription = registry.pointerUp.subscribe(spy); + const item = new DragItem(); fixture.nativeElement.addEventListener('mouseup', (e: MouseEvent) => e.stopPropagation()); - registry.startDragging(testComponent.dragItems.first, createMouseEvent('mousedown')); - dispatchMouseEvent(fixture.nativeElement.querySelector('div'), 'mouseup'); + registry.startDragging(item, createMouseEvent('mousedown')); + dispatchMouseEvent(fixture.nativeElement, 'mouseup'); expect(spy).toHaveBeenCalled(); @@ -156,17 +153,17 @@ describe('DragDropRegistry', () => { }); it('should not emit pointer events when dragging is over (multi touch)', () => { - const firstItem = testComponent.dragItems.first; + const item = new DragItem(); // First finger down - registry.startDragging(firstItem, createTouchEvent('touchstart') as TouchEvent); + registry.startDragging(item, createTouchEvent('touchstart') as TouchEvent); // Second finger down - registry.startDragging(firstItem, createTouchEvent('touchstart') as TouchEvent); + registry.startDragging(item, createTouchEvent('touchstart') as TouchEvent); // First finger up - registry.stopDragging(firstItem); + registry.stopDragging(item); // Ensure dragging is over - registry is empty - expect(registry.isDragging(firstItem)).toBe(false); + expect(registry.isDragging(item)).toBe(false); const pointerUpSpy = jasmine.createSpy('pointerUp spy'); const pointerMoveSpy = jasmine.createSpy('pointerMove spy'); @@ -191,19 +188,27 @@ describe('DragDropRegistry', () => { }); it('should prevent the default `touchmove` action when an item is being dragged', () => { - registry.startDragging(testComponent.dragItems.first, - createTouchEvent('touchstart') as TouchEvent); + const item = new DragItem(true); + registry.startDragging(item, createTouchEvent('touchstart') as TouchEvent); 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); + it('should prevent the default `touchmove` if the item does not consider itself as being ' + + 'dragged yet', () => { + const item = new DragItem(false); + registry.startDragging(item, createTouchEvent('touchstart') as TouchEvent); + expect(dispatchTouchEvent(document, 'touchmove').defaultPrevented).toBe(false); - fixture.nativeElement.addEventListener('touchmove', (e: TouchEvent) => e.stopPropagation()); + item.shouldBeDragging = true; + expect(dispatchTouchEvent(document, 'touchmove').defaultPrevented).toBe(true); + }); - const event = dispatchTouchEvent(fixture.nativeElement.querySelector('div'), 'touchmove'); + it('should prevent the default `touchmove` if event propagation is stopped', () => { + const item = new DragItem(true); + registry.startDragging(item, createTouchEvent('touchstart') as TouchEvent); + fixture.nativeElement.addEventListener('touchmove', (e: TouchEvent) => e.stopPropagation()); + const event = dispatchTouchEvent(fixture.nativeElement, 'touchmove'); expect(event.defaultPrevented).toBe(true); }); @@ -212,15 +217,17 @@ describe('DragDropRegistry', () => { }); it('should prevent the default `selectstart` action when an item is being dragged', () => { - registry.startDragging(testComponent.dragItems.first, createMouseEvent('mousedown')); + const item = new DragItem(true); + registry.startDragging(item, createMouseEvent('mousedown')); expect(dispatchFakeEvent(document, 'selectstart').defaultPrevented).toBe(true); }); it('should dispatch `scroll` events if the viewport is scrolled while dragging', () => { const spy = jasmine.createSpy('scroll spy'); const subscription = registry.scroll.subscribe(spy); + const item = new DragItem(); - registry.startDragging(testComponent.dragItems.first, createMouseEvent('mousedown')); + registry.startDragging(item, createMouseEvent('mousedown')); dispatchFakeEvent(document, 'scroll'); expect(spy).toHaveBeenCalled(); @@ -237,20 +244,19 @@ describe('DragDropRegistry', () => { subscription.unsubscribe(); }); + class DragItem { + isDragging() { return this.shouldBeDragging; } + constructor(public shouldBeDragging = false) { + registry.registerDragItem(this); + } + } + + class DragList { + constructor() { + registry.registerDropContainer(this); + } + } + + @Component({template: ``}) + class BlankComponent {} }); - -@Component({ - template: ` -
-
{{item}}
-
- -
- ` -}) -class SimpleDropZone { - @ViewChildren(CdkDrag) dragItems: QueryList; - @ViewChildren(CdkDropList) dropInstances: QueryList; - items = ['Zero', 'One', 'Two', 'Three']; - showDuplicateContainer = false; -} diff --git a/src/cdk/drag-drop/drag-drop-registry.ts b/src/cdk/drag-drop/drag-drop-registry.ts index 9eefd26fe157..2c7135374d48 100644 --- a/src/cdk/drag-drop/drag-drop-registry.ts +++ b/src/cdk/drag-drop/drag-drop-registry.ts @@ -26,7 +26,7 @@ const activeCapturingEventOptions = normalizePassiveListenerOptions({ // to avoid circular imports. If we were to reference them here, importing the registry into the // classes that are registering themselves will introduce a circular import. @Injectable({providedIn: 'root'}) -export class DragDropRegistry implements OnDestroy { +export class DragDropRegistry implements OnDestroy { private _document: Document; /** Registered drop container instances. */ @@ -36,7 +36,7 @@ export class DragDropRegistry implements OnDestroy { private _dragInstances = new Set(); /** Drag item instances that are currently being dragged. */ - private _activeDragInstances = new Set(); + private _activeDragInstances: I[] = []; /** Keeps track of the event listeners that we've bound to the `document`. */ private _globalListeners = new Map implements OnDestroy { options?: AddEventListenerOptions | boolean }>(); + /** + * Predicate function to check if an item is being dragged. Moved out into a property, + * because it'll be called a lot and we don't want to create a new function every time. + */ + private _draggingPredicate = (item: I) => item.isDragging(); + /** * Emits the `touchmove` or `mousemove` events that are dispatched * while the user is dragging a drag item instance. @@ -112,13 +118,13 @@ export class DragDropRegistry implements OnDestroy { */ 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)) { + if (this._activeDragInstances.indexOf(drag) > -1) { return; } - this._activeDragInstances.add(drag); + this._activeDragInstances.push(drag); - if (this._activeDragInstances.size === 1) { + if (this._activeDragInstances.length === 1) { const isTouchEvent = event.type.startsWith('touch'); // We explicitly bind __active__ listeners here, because newer browsers will default to @@ -163,16 +169,20 @@ export class DragDropRegistry implements OnDestroy { /** Stops dragging a drag item instance. */ stopDragging(drag: I) { - this._activeDragInstances.delete(drag); + const index = this._activeDragInstances.indexOf(drag); + + if (index > -1) { + this._activeDragInstances.splice(index, 1); - if (this._activeDragInstances.size === 0) { - this._clearGlobalListeners(); + if (this._activeDragInstances.length === 0) { + this._clearGlobalListeners(); + } } } /** Gets whether a drag item instance is currently being dragged. */ isDragging(drag: I) { - return this._activeDragInstances.has(drag); + return this._activeDragInstances.indexOf(drag) > -1; } ngOnDestroy() { @@ -188,15 +198,21 @@ export class DragDropRegistry implements OnDestroy { * @param event Event whose default action should be prevented. */ private _preventDefaultWhileDragging = (event: Event) => { - if (this._activeDragInstances.size) { + if (this._activeDragInstances.length > 0) { event.preventDefault(); } } /** Event listener for `touchmove` that is bound even if no dragging is happening. */ private _persistentTouchmoveListener = (event: TouchEvent) => { - if (this._activeDragInstances.size) { - event.preventDefault(); + if (this._activeDragInstances.length > 0) { + // Note that we only want to prevent the default action after dragging has actually started. + // Usually this is the same time at which the item is added to the `_activeDragInstances`, + // but it could be pushed back if the user has set up a drag delay or threshold. + if (this._activeDragInstances.some(this._draggingPredicate)) { + event.preventDefault(); + } + this.pointerMove.next(event); } } diff --git a/src/cdk/drag-drop/drag-ref.ts b/src/cdk/drag-drop/drag-ref.ts index da18f013d7dd..9b3888010f98 100644 --- a/src/cdk/drag-drop/drag-ref.ts +++ b/src/cdk/drag-drop/drag-ref.ts @@ -593,9 +593,6 @@ export class DragRef { /** Handler that is invoked when the user moves their pointer after they've initiated a drag. */ private _pointerMove = (event: MouseEvent | TouchEvent) => { - // Prevent the default action as early as possible in order to block - // native actions like dragging the selected text or images with the mouse. - event.preventDefault(); const pointerPosition = this._getPointerPositionOnPage(event); if (!this._hasStartedDragging) { @@ -637,6 +634,11 @@ export class DragRef { } } + // We prevent the default action down here so that we know that dragging has started. This is + // important for touch devices where doing this too early can unnecessarily block scrolling, + // if there's a dragging delay. + event.preventDefault(); + const constrainedPointerPosition = this._getConstrainedPointerPosition(pointerPosition); this._hasMoved = true; this._lastKnownPointerPosition = pointerPosition; diff --git a/tools/public_api_guard/cdk/drag-drop.d.ts b/tools/public_api_guard/cdk/drag-drop.d.ts index e955b38c1779..803c11f3d786 100644 --- a/tools/public_api_guard/cdk/drag-drop.d.ts +++ b/tools/public_api_guard/cdk/drag-drop.d.ts @@ -228,7 +228,9 @@ export declare class DragDropModule { static ɵmod: i0.ɵɵNgModuleDefWithMeta; } -export declare class DragDropRegistry implements OnDestroy { +export declare class DragDropRegistry implements OnDestroy { readonly pointerMove: Subject; readonly pointerUp: Subject; readonly scroll: Subject;