From 32c55de8444051eb9722948e5542177739d87f65 Mon Sep 17 00:00:00 2001 From: crisbeto Date: Tue, 18 Sep 2018 23:16:33 +0200 Subject: [PATCH] fix(drag-drop): avoid interfering with element clicks Resolves a long-standing TODO about starting the drag&drop sequence after the user has dragged over a certain amount of pixels, rather than starting it immediately on `mousedown`/`touchstart`. This prevents random clicks and taps on the element from being interpreted as dragging and avoids potentially interrupting the native click events on children of `CdkDrag`. --- src/cdk/drag-drop/drag.spec.ts | 206 ++++++++++++++++------- src/cdk/drag-drop/drag.ts | 186 ++++++++++---------- src/cdk/drag-drop/transition-duration.ts | 41 +++++ 3 files changed, 282 insertions(+), 151 deletions(-) create mode 100644 src/cdk/drag-drop/transition-duration.ts diff --git a/src/cdk/drag-drop/drag.spec.ts b/src/cdk/drag-drop/drag.spec.ts index 05a1b811b922..1b492937912f 100644 --- a/src/cdk/drag-drop/drag.spec.ts +++ b/src/cdk/drag-drop/drag.spec.ts @@ -19,7 +19,7 @@ import { dispatchTouchEvent, } from '@angular/cdk/testing'; import {Directionality} from '@angular/cdk/bidi'; -import {CdkDrag} from './drag'; +import {CdkDrag, CDK_DRAG_CONFIG, CdkDragConfig} from './drag'; import {CdkDragDrop} from './drag-events'; import {moveItemInArray} from './drag-utils'; import {CdkDrop} from './drop'; @@ -29,12 +29,24 @@ const ITEM_HEIGHT = 25; const ITEM_WIDTH = 75; describe('CdkDrag', () => { - function createComponent(componentType: Type, providers: Provider[] = []): + function createComponent(componentType: Type, providers: Provider[] = [], dragDistance = 0): ComponentFixture { TestBed.configureTestingModule({ imports: [DragDropModule], declarations: [componentType], - providers, + providers: [ + { + provide: CDK_DRAG_CONFIG, + useValue: { + // We default the `dragDistance` to zero, because the majority of the tests + // don't care about it and drags are a lot easier to simulate when we don't + // have to deal with thresholds. + dragStartThreshold: dragDistance, + pointerDirectionChangeThreshold: 5 + } as CdkDragConfig + }, + ...providers + ], }).compileComponents(); return TestBed.createComponent(componentType); @@ -120,6 +132,18 @@ describe('CdkDrag', () => { expect(dragElement.style.transform).toBeFalsy(); })); + + it('should not drag the element if it was not moved more than the minimum distance', + fakeAsync(() => { + const fixture = createComponent(StandaloneDraggable, [], 5); + fixture.detectChanges(); + const dragElement = fixture.componentInstance.dragElement.nativeElement; + + expect(dragElement.style.transform).toBeFalsy(); + dragElementViaMouse(fixture, dragElement, 2, 2); + expect(dragElement.style.transform).toBeFalsy(); + })); + }); describe('touch dragging', () => { @@ -201,8 +225,7 @@ describe('CdkDrag', () => { const fixture = createComponent(StandaloneDraggable); fixture.detectChanges(); - dispatchMouseEvent(fixture.componentInstance.dragElement.nativeElement, 'mousedown'); - fixture.detectChanges(); + startDraggingViaMouse(fixture, fixture.componentInstance.dragElement.nativeElement); expect(fixture.componentInstance.startedSpy).toHaveBeenCalled(); @@ -244,6 +267,21 @@ describe('CdkDrag', () => { subscription.unsubscribe(); }); + it('should not emit events if it was not moved more than the minimum distance', () => { + const fixture = createComponent(StandaloneDraggable, [], 5); + fixture.detectChanges(); + + const moveSpy = jasmine.createSpy('move spy'); + const subscription = fixture.componentInstance.dragInstance.moved.subscribe(moveSpy); + + dragElementViaMouse(fixture, fixture.componentInstance.dragElement.nativeElement, 2, 2); + + expect(fixture.componentInstance.startedSpy).not.toHaveBeenCalled(); + expect(fixture.componentInstance.endedSpy).not.toHaveBeenCalled(); + expect(moveSpy).not.toHaveBeenCalled(); + subscription.unsubscribe(); + }); + it('should emit to `moved` inside the NgZone', () => { const fixture = createComponent(StandaloneDraggable); fixture.detectChanges(); @@ -311,8 +349,7 @@ describe('CdkDrag', () => { expect(element.classList).not.toContain('cdk-drag-dragging'); - dispatchMouseEvent(element, 'mousedown'); - fixture.detectChanges(); + startDraggingViaMouse(fixture, element); expect(element.classList).toContain('cdk-drag-dragging'); @@ -322,6 +359,19 @@ describe('CdkDrag', () => { expect(element.classList).not.toContain('cdk-drag-dragging'); })); + it('should not add a class if item was not dragged more than the threshold', fakeAsync(() => { + const fixture = createComponent(StandaloneDraggable, [], 5); + fixture.detectChanges(); + + const element = fixture.componentInstance.dragElement.nativeElement; + + expect(element.classList).not.toContain('cdk-drag-dragging'); + + startDraggingViaMouse(fixture, element); + + expect(element.classList).not.toContain('cdk-drag-dragging'); + })); + it('should be able to set an alternate drag root element', fakeAsync(() => { const fixture = createComponent(DraggableWithAlternateRoot); fixture.detectChanges(); @@ -430,8 +480,7 @@ describe('CdkDrag', () => { expect(dropZone.element.nativeElement.classList).not.toContain('cdk-drop-dragging'); - dispatchMouseEvent(item, 'mousedown'); - fixture.detectChanges(); + startDraggingViaMouse(fixture, item); expect(dropZone.element.nativeElement.classList).toContain('cdk-drop-dragging'); @@ -443,6 +492,20 @@ describe('CdkDrag', () => { expect(dropZone.element.nativeElement.classList).not.toContain('cdk-drop-dragging'); })); + it('should not toggle dragging class if the element was not dragged more than the threshold', + fakeAsync(() => { + const fixture = createComponent(DraggableInDropZone, [], 5); + fixture.detectChanges(); + const item = fixture.componentInstance.dragItems.toArray()[1].element.nativeElement; + const dropZone = fixture.componentInstance.dropInstance; + + expect(dropZone.element.nativeElement.classList).not.toContain('cdk-drop-dragging'); + + startDraggingViaMouse(fixture, item); + + expect(dropZone.element.nativeElement.classList).not.toContain('cdk-drop-dragging'); + })); + it('should dispatch the `dropped` event when an item has been dropped', fakeAsync(() => { const fixture = createComponent(DraggableInDropZone); fixture.detectChanges(); @@ -611,8 +674,7 @@ describe('CdkDrag', () => { const itemRect = item.getBoundingClientRect(); const initialParent = item.parentNode; - dispatchMouseEvent(item, 'mousedown'); - fixture.detectChanges(); + startDraggingViaMouse(fixture, item); const preview = document.querySelector('.cdk-drag-preview')! as HTMLElement; const previewRect = preview.getBoundingClientRect(); @@ -637,6 +699,16 @@ describe('CdkDrag', () => { expect(preview.parentNode).toBeFalsy('Expected preview to be removed from the DOM'); })); + it('should not create a preview if the element was not dragged far enough', fakeAsync(() => { + const fixture = createComponent(DraggableInDropZone, [], 5); + fixture.detectChanges(); + const item = fixture.componentInstance.dragItems.toArray()[1].element.nativeElement; + + startDraggingViaMouse(fixture, item); + + expect(document.querySelector('.cdk-drag-preview')).toBeFalsy(); + })); + it('should pass the proper direction to the preview in rtl', fakeAsync(() => { const fixture = createComponent(DraggableInDropZone, [{ provide: Directionality, @@ -646,8 +718,7 @@ describe('CdkDrag', () => { fixture.detectChanges(); const item = fixture.componentInstance.dragItems.toArray()[1].element.nativeElement; - dispatchMouseEvent(item, 'mousedown'); - fixture.detectChanges(); + startDraggingViaMouse(fixture, item); expect(document.querySelector('.cdk-drag-preview')!.getAttribute('dir')) .toBe('rtl', 'Expected preview element to inherit the directionality.'); @@ -658,8 +729,7 @@ describe('CdkDrag', () => { fixture.detectChanges(); const item = fixture.componentInstance.dragItems.toArray()[1].element.nativeElement; - dispatchMouseEvent(item, 'mousedown'); - fixture.detectChanges(); + startDraggingViaMouse(fixture, item); const preview = document.querySelector('.cdk-drag-preview')! as HTMLElement; @@ -699,8 +769,7 @@ describe('CdkDrag', () => { expect(spy).toHaveBeenCalledTimes(1); // Start another drag. - dispatchMouseEvent(item, 'mousedown'); - fixture.detectChanges(); + startDraggingViaMouse(fixture, item); // Add a duration since the tests won't include one. const preview = document.querySelector('.cdk-drag-preview')! as HTMLElement; @@ -721,8 +790,7 @@ describe('CdkDrag', () => { fixture.detectChanges(); const item = fixture.componentInstance.dragItems.toArray()[1].element.nativeElement; - dispatchMouseEvent(item, 'mousedown'); - fixture.detectChanges(); + startDraggingViaMouse(fixture, item); const preview = document.querySelector('.cdk-drag-preview')! as HTMLElement; preview.style.transition = 'opacity 500ms ease'; @@ -744,8 +812,7 @@ describe('CdkDrag', () => { fixture.detectChanges(); const item = fixture.componentInstance.dragItems.toArray()[1].element.nativeElement; - dispatchMouseEvent(item, 'mousedown'); - fixture.detectChanges(); + startDraggingViaMouse(fixture, item); const preview = document.querySelector('.cdk-drag-preview')! as HTMLElement; preview.style.transition = 'opacity 500ms ease, transform 1000ms ease'; @@ -772,8 +839,7 @@ describe('CdkDrag', () => { const item = fixture.componentInstance.dragItems.toArray()[1].element.nativeElement; const initialParent = item.parentNode; - dispatchMouseEvent(item, 'mousedown'); - fixture.detectChanges(); + startDraggingViaMouse(fixture, item); const placeholder = document.querySelector('.cdk-drag-placeholder')! as HTMLElement; @@ -790,6 +856,16 @@ describe('CdkDrag', () => { expect(placeholder.parentNode).toBeFalsy('Expected placeholder to be removed from the DOM'); })); + it('should not create placeholder if the element was not dragged far enough', fakeAsync(() => { + const fixture = createComponent(DraggableInDropZone, [], 5); + fixture.detectChanges(); + const item = fixture.componentInstance.dragItems.toArray()[1].element.nativeElement; + + startDraggingViaMouse(fixture, item); + + expect(document.querySelector('.cdk-drag-placeholder')).toBeFalsy(); + })); + it('should move the placeholder as an item is being sorted down', fakeAsync(() => { const fixture = createComponent(DraggableInDropZone); fixture.detectChanges(); @@ -840,8 +916,7 @@ describe('CdkDrag', () => { const draggedItem = items[0].element.nativeElement; const {top, left} = draggedItem.getBoundingClientRect(); - dispatchMouseEvent(draggedItem, 'mousedown', left, top); - fixture.detectChanges(); + startDraggingViaMouse(fixture, draggedItem, left, top); const placeholder = document.querySelector('.cdk-drag-placeholder')! as HTMLElement; @@ -868,8 +943,7 @@ describe('CdkDrag', () => { const draggedItem = items[items.length - 1].element.nativeElement; const {top, left} = draggedItem.getBoundingClientRect(); - dispatchMouseEvent(draggedItem, 'mousedown', left, top); - fixture.detectChanges(); + startDraggingViaMouse(fixture, draggedItem, left, top); const placeholder = document.querySelector('.cdk-drag-placeholder')! as HTMLElement; @@ -897,8 +971,7 @@ describe('CdkDrag', () => { const draggedItem = items[0]; const {top, left} = draggedItem.getBoundingClientRect(); - dispatchMouseEvent(draggedItem, 'mousedown', left, top); - fixture.detectChanges(); + startDraggingViaMouse(fixture, draggedItem, left, top); const placeholder = document.querySelector('.cdk-drag-placeholder')! as HTMLElement; const targetRect = items[items.length - 1].getBoundingClientRect(); @@ -924,8 +997,7 @@ describe('CdkDrag', () => { const draggedItem = items[0]; const {top, left} = draggedItem.getBoundingClientRect(); - dispatchMouseEvent(draggedItem, 'mousedown', left, top); - fixture.detectChanges(); + startDraggingViaMouse(fixture, draggedItem, left, top); const placeholder = document.querySelector('.cdk-drag-placeholder')! as HTMLElement; const targetRect = items[items.length - 1].getBoundingClientRect(); @@ -954,8 +1026,7 @@ describe('CdkDrag', () => { // Bump the height so the pointer doesn't leave after swapping. target.style.height = `${ITEM_HEIGHT * 3}px`; - dispatchMouseEvent(draggedItem, 'mousedown', left, top); - fixture.detectChanges(); + startDraggingViaMouse(fixture, draggedItem, left, top); const placeholder = document.querySelector('.cdk-drag-placeholder')! as HTMLElement; @@ -994,8 +1065,7 @@ describe('CdkDrag', () => { // Bump the height so the pointer doesn't leave after swapping. target.style.height = `${ITEM_HEIGHT * 3}px`; - dispatchMouseEvent(draggedItem, 'mousedown', left, top); - fixture.detectChanges(); + startDraggingViaMouse(fixture, draggedItem, left, top); const placeholder = document.querySelector('.cdk-drag-placeholder')! as HTMLElement; @@ -1027,8 +1097,7 @@ describe('CdkDrag', () => { fixture.detectChanges(); const item = fixture.componentInstance.dragItems.toArray()[1].element.nativeElement; - dispatchMouseEvent(item, 'mousedown'); - fixture.detectChanges(); + startDraggingViaMouse(fixture, item); const preview = document.querySelector('.cdk-drag-preview')! as HTMLElement; @@ -1046,8 +1115,7 @@ describe('CdkDrag', () => { fixture.detectChanges(); const item = fixture.componentInstance.dragItems.toArray()[1].element.nativeElement; - dispatchMouseEvent(item, 'mousedown'); - fixture.detectChanges(); + startDraggingViaMouse(fixture, item); const preview = document.querySelector('.cdk-drag-preview')! as HTMLElement; @@ -1080,8 +1148,7 @@ describe('CdkDrag', () => { fixture.detectChanges(); const item = fixture.componentInstance.dragItems.toArray()[1].element.nativeElement; - dispatchMouseEvent(item, 'mousedown', 50, 50); - fixture.detectChanges(); + startDraggingViaMouse(fixture, item, 50, 50); const preview = document.querySelector('.cdk-drag-preview')! as HTMLElement; @@ -1097,8 +1164,7 @@ describe('CdkDrag', () => { item.lockAxis = 'x'; - dispatchMouseEvent(element, 'mousedown', 50, 50); - fixture.detectChanges(); + startDraggingViaMouse(fixture, element, 50, 50); dispatchMouseEvent(element, 'mousemove', 100, 100); fixture.detectChanges(); @@ -1117,8 +1183,7 @@ describe('CdkDrag', () => { item.lockAxis = 'y'; - dispatchMouseEvent(element, 'mousedown', 50, 50); - fixture.detectChanges(); + startDraggingViaMouse(fixture, element, 50, 50); dispatchMouseEvent(element, 'mousemove', 100, 100); fixture.detectChanges(); @@ -1136,8 +1201,7 @@ describe('CdkDrag', () => { fixture.componentInstance.dropInstance.lockAxis = 'x'; - dispatchMouseEvent(element, 'mousedown', 50, 50); - fixture.detectChanges(); + startDraggingViaMouse(fixture, element, 50, 50); dispatchMouseEvent(element, 'mousemove', 100, 100); fixture.detectChanges(); @@ -1152,8 +1216,7 @@ describe('CdkDrag', () => { fixture.detectChanges(); const item = fixture.componentInstance.dragItems.toArray()[1].element.nativeElement; - dispatchMouseEvent(item, 'mousedown'); - fixture.detectChanges(); + startDraggingViaMouse(fixture, item); const placeholder = document.querySelector('.cdk-drag-placeholder')! as HTMLElement; @@ -1171,8 +1234,7 @@ describe('CdkDrag', () => { const thirdItem = dragItems.toArray()[2].element.nativeElement; const thirdItemRect = thirdItem.getBoundingClientRect(); - dispatchMouseEvent(firstItem.element.nativeElement, 'mousedown'); - fixture.detectChanges(); + startDraggingViaMouse(fixture, firstItem.element.nativeElement); dispatchMouseEvent(document, 'mousemove', thirdItemRect.left + 1, thirdItemRect.top + 1); fixture.detectChanges(); @@ -1227,8 +1289,7 @@ describe('CdkDrag', () => { const initialRect = item.element.nativeElement.getBoundingClientRect(); const targetRect = groups[1][2].element.nativeElement.getBoundingClientRect(); - dispatchMouseEvent(item.element.nativeElement, 'mousedown'); - fixture.detectChanges(); + startDraggingViaMouse(fixture, item.element.nativeElement); const placeholder = dropZones[0].querySelector('.cdk-drag-placeholder')!; @@ -1369,8 +1430,7 @@ describe('CdkDrag', () => { const initialRect = item.element.nativeElement.getBoundingClientRect(); const targetRect = groups[1][0].element.nativeElement.getBoundingClientRect(); - dispatchMouseEvent(item.element.nativeElement, 'mousedown'); - fixture.detectChanges(); + startDraggingViaMouse(fixture, item.element.nativeElement); const placeholder = dropZones[0].querySelector('.cdk-drag-placeholder')!; @@ -1794,10 +1854,9 @@ class ConnectedDropZonesWithSingleItems { * @param y Position along the y axis to which to drag the element. */ function dragElementViaMouse(fixture: ComponentFixture, - element: HTMLElement, x: number, y: number) { + element: Element, x: number, y: number) { - dispatchMouseEvent(element, 'mousedown'); - fixture.detectChanges(); + startDraggingViaMouse(fixture, element); dispatchMouseEvent(document, 'mousemove', x, y); fixture.detectChanges(); @@ -1806,6 +1865,22 @@ function dragElementViaMouse(fixture: ComponentFixture, fixture.detectChanges(); } +/** + * Dispatches the events for starting a drag sequence. + * @param fixture Fixture on which to run change detection. + * @param element Element on which to dispatch the events. + * @param x Position along the x axis to which to drag the element. + * @param y Position along the y axis to which to drag the element. + */ +function startDraggingViaMouse(fixture: ComponentFixture, + element: Element, x?: number, y?: number) { + dispatchMouseEvent(element, 'mousedown', x, y); + fixture.detectChanges(); + + dispatchMouseEvent(document, 'mousemove', x, y); + fixture.detectChanges(); +} + /** * Drags an element to a position on the page using a touch device. * @param fixture Fixture on which to run change detection. @@ -1814,11 +1889,14 @@ function dragElementViaMouse(fixture: ComponentFixture, * @param y Position along the y axis to which to drag the element. */ function dragElementViaTouch(fixture: ComponentFixture, - element: HTMLElement, x: number, y: number) { + element: Element, x: number, y: number) { dispatchTouchEvent(element, 'touchstart'); fixture.detectChanges(); + dispatchTouchEvent(document, 'touchmove'); + fixture.detectChanges(); + dispatchTouchEvent(document, 'touchmove', x, y); fixture.detectChanges(); @@ -1827,12 +1905,12 @@ function dragElementViaTouch(fixture: ComponentFixture, } /** Gets the index of an element among its siblings, based on their position on the page. */ -function getElementIndexByPosition(element: HTMLElement, direction: 'top' | 'left') { +function getElementIndexByPosition(element: Element, direction: 'top' | 'left') { return getElementSibligsByPosition(element, direction).indexOf(element); } /** Gets the siblings of an element, sorted by their position on the page. */ -function getElementSibligsByPosition(element: HTMLElement, direction: 'top' | 'left') { +function getElementSibligsByPosition(element: Element, direction: 'top' | 'left') { return element.parentElement ? Array.from(element.parentElement.children).sort((a, b) => { return a.getBoundingClientRect()[direction] - b.getBoundingClientRect()[direction]; }) : []; @@ -1863,8 +1941,7 @@ function assertDownwardSorting(fixture: ComponentFixture, items: Element[]) const draggedItem = items[0]; const {top, left} = draggedItem.getBoundingClientRect(); - dispatchMouseEvent(draggedItem, 'mousedown', left, top); - fixture.detectChanges(); + startDraggingViaMouse(fixture, draggedItem, left, top); const placeholder = document.querySelector('.cdk-drag-placeholder')! as HTMLElement; @@ -1892,8 +1969,7 @@ function assertUpwardSorting(fixture: ComponentFixture, items: Element[]) { const draggedItem = items[items.length - 1]; const {top, left} = draggedItem.getBoundingClientRect(); - dispatchMouseEvent(draggedItem, 'mousedown', left, top); - fixture.detectChanges(); + startDraggingViaMouse(fixture, draggedItem, left, top); const placeholder = document.querySelector('.cdk-drag-placeholder')! as HTMLElement; diff --git a/src/cdk/drag-drop/drag.ts b/src/cdk/drag-drop/drag.ts index 9b96fc065bf1..6e3be3733d56 100644 --- a/src/cdk/drag-drop/drag.ts +++ b/src/cdk/drag-drop/drag.ts @@ -18,6 +18,7 @@ import { EmbeddedViewRef, EventEmitter, Inject, + InjectionToken, Input, NgZone, OnDestroy, @@ -27,8 +28,8 @@ import { SkipSelf, ViewContainerRef, } from '@angular/core'; -import {merge, Observable, Subject} from 'rxjs'; -import {takeUntil, take} from 'rxjs/operators'; +import {Observable, Subject, Subscription} from 'rxjs'; +import {take} from 'rxjs/operators'; import {DragDropRegistry} from './drag-drop-registry'; import { CdkDragDrop, @@ -42,17 +43,38 @@ import {CdkDragHandle} from './drag-handle'; import {CdkDragPlaceholder} from './drag-placeholder'; import {CdkDragPreview} from './drag-preview'; import {CDK_DROP_CONTAINER, CdkDropContainer} from './drop-container'; +import {getTransformTransitionDurationInMs} from './transition-duration'; // TODO(crisbeto): add auto-scrolling functionality. // TODO(crisbeto): add an API for moving a draggable up/down the // list programmatically. Useful for keyboard controls. -/** - * Amount the pixels the user should drag before we - * consider them to have changed the drag direction. - */ -const POINTER_DIRECTION_CHANGE_THRESHOLD = 5; +/** Object that can be used to configure the behavior of CdkDrag. */ +export interface CdkDragConfig { + /** + * Minimum amount of pixels that the user should + * drag, before the CDK initiates a drag sequence. + */ + dragStartThreshold: number; + + /** + * Amount the pixels the user should drag before the CDK + * considers them to have changed the drag direction. + */ + pointerDirectionChangeThreshold: number; +} + +/** Injection token that can be used to configure the behavior of `CdkDrag`. */ +export const CDK_DRAG_CONFIG = new InjectionToken('CDK_DRAG_CONFIG', { + providedIn: 'root', + factory: CDK_DRAG_CONFIG_FACTORY +}); + +/** @docs-private */ +export function CDK_DRAG_CONFIG_FACTORY(): CdkDragConfig { + return {dragStartThreshold: 5, pointerDirectionChangeThreshold: 5}; +} /** Element that can be moved inside a CdkDrop container. */ @Directive({ @@ -60,12 +82,11 @@ const POINTER_DIRECTION_CHANGE_THRESHOLD = 5; exportAs: 'cdkDrag', host: { 'class': 'cdk-drag', - '[class.cdk-drag-dragging]': '_isDragging()', + '[class.cdk-drag-dragging]': '_hasStartedDragging && _isDragging()', } }) export class CdkDrag implements AfterViewInit, OnDestroy { private _document: Document; - private _destroyed = new Subject(); /** Element displayed next to the user's pointer while the element is dragged. */ private _preview: HTMLElement; @@ -102,6 +123,12 @@ export class CdkDrag implements AfterViewInit, OnDestroy { /** CSS `transform` that is applied to the element while it's being dragged. */ private _activeTransform: Point = {x: 0, y: 0}; + /** + * Whether the dragging sequence has been started. Doesn't + * necessarily mean that the element has been moved. + */ + _hasStartedDragging: boolean; + /** Whether the element has moved since the user started dragging it. */ private _hasMoved: boolean; @@ -129,6 +156,12 @@ export class CdkDrag implements AfterViewInit, OnDestroy { /** Root element that will be dragged by the user. */ private _rootElement: HTMLElement; + /** Subscription to pointer movement events. */ + private _pointerMoveSubscription = Subscription.EMPTY; + + /** Subscription to the event that is dispatched when the user lifts their pointer. */ + private _pointerUpSubscription = Subscription.EMPTY; + /** Elements that can be used to drag the draggable item. */ @ContentChildren(CdkDragHandle) _handles: QueryList; @@ -193,6 +226,7 @@ export class CdkDrag implements AfterViewInit, OnDestroy { private _viewContainerRef: ViewContainerRef, private _viewportRuler: ViewportRuler, private _dragDropRegistry: DragDropRegistry, CdkDropContainer>, + @Inject(CDK_DRAG_CONFIG) private _config: CdkDragConfig, @Optional() private _dir: Directionality) { this._document = document; _dragDropRegistry.registerDragItem(this); @@ -218,18 +252,14 @@ export class CdkDrag implements AfterViewInit, OnDestroy { // their original DOM position and then they get transferred to the portal. this._ngZone.onStable.asObservable().pipe(take(1)).subscribe(() => { const rootElement = this._rootElement = this._getRootElement(); - - // We need to bring the events back into the `NgZone`, because of the `onStable` call. - this._ngZone.run(() => { - rootElement.addEventListener('mousedown', this._startDragging); - rootElement.addEventListener('touchstart', this._startDragging); - }); + rootElement.addEventListener('mousedown', this._pointerDown); + rootElement.addEventListener('touchstart', this._pointerDown); }); } ngOnDestroy() { - this._rootElement.removeEventListener('mousedown', this._startDragging); - this._rootElement.removeEventListener('touchstart', this._startDragging); + this._rootElement.removeEventListener('mousedown', this._pointerDown); + this._rootElement.removeEventListener('touchstart', this._pointerDown); this._destroyPreview(); this._destroyPlaceholder(); @@ -243,13 +273,17 @@ export class CdkDrag implements AfterViewInit, OnDestroy { this._nextSibling = null; this._dragDropRegistry.removeDragItem(this); + this._removeSubscriptions(); this._moveEvents.complete(); - this._destroyed.next(); - this._destroyed.complete(); } - /** Starts the dragging sequence. */ - _startDragging = (event: MouseEvent | TouchEvent) => { + /** Checks whether the element is currently being dragged. */ + _isDragging() { + return this._dragDropRegistry.isDragging(this); + } + + /** Handler for the `mousedown`/`touchstart` events. */ + _pointerDown = (event: MouseEvent | TouchEvent) => { // Delegate the event based on whether it started from a handle or the element itself. if (this._handles.length) { const targetHandle = this._handles.find(handle => { @@ -259,22 +293,20 @@ export class CdkDrag implements AfterViewInit, OnDestroy { }); if (targetHandle) { - this._pointerDown(targetHandle.element.nativeElement, event); + this._initializeDragSequence(targetHandle.element.nativeElement, event); } } else { - this._pointerDown(this._rootElement, event); + this._initializeDragSequence(this._rootElement, event); } } - /** Checks whether the element is currently being dragged. */ - _isDragging() { - return this._dragDropRegistry.isDragging(this); - } - - /** Handler for when the pointer is pressed down on the element or the handle. */ - private _pointerDown = (referenceElement: HTMLElement, - event: MouseEvent | TouchEvent) => { - + /** + * Sets up the different variables and subscriptions + * that will be necessary for the dragging sequence. + * @param referenceElement Element that started the drag sequence. + * @param event Browser event object that started the sequence. + */ + private _initializeDragSequence(referenceElement: HTMLElement, event: MouseEvent | TouchEvent) { const isDragging = this._isDragging(); // Abort if the user is already dragging or is using a mouse button other than the primary one. @@ -282,19 +314,10 @@ export class CdkDrag implements AfterViewInit, OnDestroy { return; } - const endedOrDestroyed = merge(this.ended, this._destroyed); - - this._hasMoved = false; - this._dragDropRegistry.pointerMove - .pipe(takeUntil(endedOrDestroyed)) - .subscribe(this._pointerMove); - - this._dragDropRegistry.pointerUp - .pipe(takeUntil(endedOrDestroyed)) - .subscribe(this._pointerUp); - - this._dragDropRegistry.startDragging(this, event); + this._hasStartedDragging = this._hasMoved = false; this._initialContainer = this.dropContainer; + this._pointerMoveSubscription = this._dragDropRegistry.pointerMove.subscribe(this._pointerMove); + this._pointerUpSubscription = this._dragDropRegistry.pointerUp.subscribe(this._pointerUp); this._scrollPosition = this._viewportRuler.getViewportScrollPosition(); // If we have a custom preview template, the element won't be visible anyway so we avoid the @@ -302,10 +325,13 @@ export class CdkDrag implements AfterViewInit, OnDestroy { this._pickupPositionInElement = this._previewTemplate ? {x: 0, y: 0} : this._getPointerPositionInElement(referenceElement, event); const pointerPosition = this._pickupPositionOnPage = this._getPointerPositionOnPage(event); - this._pointerDirectionDelta = {x: 0, y: 0}; this._pointerPositionAtLastDirectionChange = {x: pointerPosition.x, y: pointerPosition.y}; + this._dragDropRegistry.startDragging(this, event); + } + /** Starts the dragging sequence. */ + private _startDragSequence() { // Emit the event on the item before the one on the container. this.started.emit({source: this}); @@ -331,17 +357,28 @@ export class CdkDrag implements AfterViewInit, OnDestroy { /** Handler that is invoked when the user moves their pointer after they've initiated a drag. */ private _pointerMove = (event: MouseEvent | TouchEvent) => { - // TODO(crisbeto): this should start dragging after a certain threshold, - // otherwise we risk interfering with clicks on the element. - if (!this._isDragging()) { + const pointerPosition = this._getConstrainedPointerPosition(event); + + if (!this._hasStartedDragging) { + const distanceX = Math.abs(pointerPosition.x - this._pickupPositionOnPage.x); + const distanceY = Math.abs(pointerPosition.y - this._pickupPositionOnPage.y); + const minimumDistance = this._config.dragStartThreshold; + + // Only start dragging after the user has moved more than the minimum distance in either + // direction. Note that this is preferrable over doing something like `skip(minimumDistance)` + // in the `pointerMove` subscription, because we're not guaranteed to have one move event + // per pixel of movement (e.g. if the user moves their pointer quickly). + if (distanceX + distanceY >= minimumDistance) { + this._hasStartedDragging = true; + this._ngZone.run(() => this._startDragSequence()); + } + return; } this._hasMoved = true; event.preventDefault(); - - const pointerPosition = this._getConstrainedPointerPosition(event); - const delta = this._updatePointerDirectionDelta(pointerPosition); + this._updatePointerDirectionDelta(pointerPosition); if (this.dropContainer) { this._updateActiveDropContainer(pointerPosition); @@ -363,7 +400,7 @@ export class CdkDrag implements AfterViewInit, OnDestroy { source: this, pointerPosition, event, - delta + delta: this._pointerDirectionDelta }); }); } @@ -375,8 +412,13 @@ export class CdkDrag implements AfterViewInit, OnDestroy { return; } + this._removeSubscriptions(); this._dragDropRegistry.stopDragging(this); + if (!this._hasStartedDragging) { + return; + } + if (!this.dropContainer) { // Convert the active transform into a passive one. This means that next time // the user starts dragging the item, its position will be calculated relatively @@ -654,12 +696,12 @@ export class CdkDrag implements AfterViewInit, OnDestroy { // to change for every pixel, otherwise anything that depends on it can look erratic. // To make the delta more consistent, we track how much the user has moved since the last // delta change and we only update it after it has reached a certain threshold. - if (changeX > POINTER_DIRECTION_CHANGE_THRESHOLD) { + if (changeX > this._config.pointerDirectionChangeThreshold) { delta.x = x > positionSinceLastChange.x ? 1 : -1; positionSinceLastChange.x = x; } - if (changeY > POINTER_DIRECTION_CHANGE_THRESHOLD) { + if (changeY > this._config.pointerDirectionChangeThreshold) { delta.y = y > positionSinceLastChange.y ? 1 : -1; positionSinceLastChange.y = y; } @@ -683,40 +725,12 @@ export class CdkDrag implements AfterViewInit, OnDestroy { return this.element.nativeElement; } -} -/** Parses a CSS time value to milliseconds. */ -function parseCssTimeUnitsToMs(value: string): number { - // Some browsers will return it in seconds, whereas others will return milliseconds. - const multiplier = value.toLowerCase().indexOf('ms') > -1 ? 1 : 1000; - return parseFloat(value) * multiplier; -} - -/** Gets the transform transition duration, including the delay, of an element in milliseconds. */ -function getTransformTransitionDurationInMs(element: HTMLElement): number { - const computedStyle = getComputedStyle(element); - const transitionedProperties = parseCssPropertyValue(computedStyle, 'transition-property'); - const property = transitionedProperties.find(prop => prop === 'transform' || prop === 'all'); - - // If there's no transition for `all` or `transform`, we shouldn't do anything. - if (!property) { - return 0; + /** Unsubscribes from the global subscriptions. */ + private _removeSubscriptions() { + this._pointerMoveSubscription.unsubscribe(); + this._pointerUpSubscription.unsubscribe(); } - - // Get the index of the property that we're interested in and match - // it up to the same index in `transition-delay` and `transition-duration`. - const propertyIndex = transitionedProperties.indexOf(property); - const rawDurations = parseCssPropertyValue(computedStyle, 'transition-duration'); - const rawDelays = parseCssPropertyValue(computedStyle, 'transition-delay'); - - return parseCssTimeUnitsToMs(rawDurations[propertyIndex]) + - parseCssTimeUnitsToMs(rawDelays[propertyIndex]); -} - -/** Parses out multiple values from a computed style into an array. */ -function parseCssPropertyValue(computedStyle: CSSStyleDeclaration, name: string): string[] { - const value = computedStyle.getPropertyValue(name); - return value.split(',').map(part => part.trim()); } /** Point on the page or within an element. */ diff --git a/src/cdk/drag-drop/transition-duration.ts b/src/cdk/drag-drop/transition-duration.ts new file mode 100644 index 000000000000..21a188c677bb --- /dev/null +++ b/src/cdk/drag-drop/transition-duration.ts @@ -0,0 +1,41 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +/** Parses a CSS time value to milliseconds. */ +function parseCssTimeUnitsToMs(value: string): number { + // Some browsers will return it in seconds, whereas others will return milliseconds. + const multiplier = value.toLowerCase().indexOf('ms') > -1 ? 1 : 1000; + return parseFloat(value) * multiplier; +} + +/** Gets the transform transition duration, including the delay, of an element in milliseconds. */ +export function getTransformTransitionDurationInMs(element: HTMLElement): number { + const computedStyle = getComputedStyle(element); + const transitionedProperties = parseCssPropertyValue(computedStyle, 'transition-property'); + const property = transitionedProperties.find(prop => prop === 'transform' || prop === 'all'); + + // If there's no transition for `all` or `transform`, we shouldn't do anything. + if (!property) { + return 0; + } + + // Get the index of the property that we're interested in and match + // it up to the same index in `transition-delay` and `transition-duration`. + const propertyIndex = transitionedProperties.indexOf(property); + const rawDurations = parseCssPropertyValue(computedStyle, 'transition-duration'); + const rawDelays = parseCssPropertyValue(computedStyle, 'transition-delay'); + + return parseCssTimeUnitsToMs(rawDurations[propertyIndex]) + + parseCssTimeUnitsToMs(rawDelays[propertyIndex]); +} + +/** Parses out multiple values from a computed style into an array. */ +function parseCssPropertyValue(computedStyle: CSSStyleDeclaration, name: string): string[] { + const value = computedStyle.getPropertyValue(name); + return value.split(',').map(part => part.trim()); +}