From bbf51b1fe56b2b0182da5a6bc2989d356bff70c4 Mon Sep 17 00:00:00 2001 From: crisbeto Date: Sat, 25 Aug 2018 19:26:02 +0200 Subject: [PATCH] fix(drag-drop): don't move item in list if pointer is too far away Currently the drop container will move an item in a list, no matter how far away the pointer is, as long as it overlaps another item along the proper axis. This can be annoying to users, because it can lead to unwanted re-arrangements of the list. These changes rework the logic so that the list is only re-arranged if the pointer is inside the drop container or within a buffer around it. --- src/cdk/drag-drop/drag.spec.ts | 70 +++++++++++++++++++++++++++++ src/cdk/drag-drop/drop-container.ts | 8 ++-- src/cdk/drag-drop/drop.ts | 56 +++++++++++++++++------ 3 files changed, 116 insertions(+), 18 deletions(-) diff --git a/src/cdk/drag-drop/drag.spec.ts b/src/cdk/drag-drop/drag.spec.ts index 3a1f89d3d2fb..34240908f218 100644 --- a/src/cdk/drag-drop/drag.spec.ts +++ b/src/cdk/drag-drop/drag.spec.ts @@ -461,6 +461,41 @@ describe('CdkDrag', () => { .toEqual(['One', 'Two', 'Zero', 'Three']); })); + it('should not move items in a vertical list if the pointer is too far away', fakeAsync(() => { + const fixture = createComponent(DraggableInDropZone); + fixture.detectChanges(); + const dragItems = fixture.componentInstance.dragItems; + + expect(dragItems.map(drag => drag.element.nativeElement.textContent!.trim())) + .toEqual(['Zero', 'One', 'Two', 'Three']); + + const firstItem = dragItems.first; + const thirdItemRect = dragItems.toArray()[2].element.nativeElement.getBoundingClientRect(); + + // Move the cursor all the way to the right so it doesn't intersect along the x axis. + dragElementViaMouse(fixture, firstItem.element.nativeElement, + thirdItemRect.right + 1000, thirdItemRect.top + 1); + flush(); + fixture.detectChanges(); + + expect(fixture.componentInstance.droppedSpy).toHaveBeenCalledTimes(1); + + const event = fixture.componentInstance.droppedSpy.calls.mostRecent().args[0]; + + // Assert the event like this, rather than `toHaveBeenCalledWith`, because Jasmine will + // go into an infinite loop trying to stringify the event, if the test fails. + expect(event).toEqual({ + previousIndex: 0, + currentIndex: 0, + item: firstItem, + container: fixture.componentInstance.dropInstance, + previousContainer: fixture.componentInstance.dropInstance + }); + + expect(dragItems.map(drag => drag.element.nativeElement.textContent!.trim())) + .toEqual(['Zero', 'One', 'Two', 'Three']); + })); + it('should not move the original element from its initial DOM position', fakeAsync(() => { const fixture = createComponent(DraggableInDropZone); fixture.detectChanges(); @@ -518,6 +553,41 @@ describe('CdkDrag', () => { .toEqual(['One', 'Two', 'Zero', 'Three']); })); + it('should not move items in a horizontal list if pointer is too far away', fakeAsync(() => { + const fixture = createComponent(DraggableInHorizontalDropZone); + fixture.detectChanges(); + const dragItems = fixture.componentInstance.dragItems; + + expect(dragItems.map(drag => drag.element.nativeElement.textContent!.trim())) + .toEqual(['Zero', 'One', 'Two', 'Three']); + + const firstItem = dragItems.first; + const thirdItemRect = dragItems.toArray()[2].element.nativeElement.getBoundingClientRect(); + + // Move the cursor all the way to the bottom so it doesn't intersect along the y axis. + dragElementViaMouse(fixture, firstItem.element.nativeElement, + thirdItemRect.left + 1, thirdItemRect.bottom + 1000); + flush(); + fixture.detectChanges(); + + expect(fixture.componentInstance.droppedSpy).toHaveBeenCalledTimes(1); + + const event = fixture.componentInstance.droppedSpy.calls.mostRecent().args[0]; + + // Assert the event like this, rather than `toHaveBeenCalledWith`, because Jasmine will + // go into an infinite loop trying to stringify the event, if the test fails. + expect(event).toEqual({ + previousIndex: 0, + currentIndex: 0, + item: firstItem, + container: fixture.componentInstance.dropInstance, + previousContainer: fixture.componentInstance.dropInstance + }); + + expect(dragItems.map(drag => drag.element.nativeElement.textContent!.trim())) + .toEqual(['Zero', 'One', 'Two', 'Three']); + })); + it('should create a preview element while the item is dragged', fakeAsync(() => { const fixture = createComponent(DraggableInDropZone); fixture.detectChanges(); diff --git a/src/cdk/drag-drop/drop-container.ts b/src/cdk/drag-drop/drop-container.ts index 4bbed64e63d6..b489a8bbb2e8 100644 --- a/src/cdk/drag-drop/drop-container.ts +++ b/src/cdk/drag-drop/drop-container.ts @@ -36,10 +36,10 @@ export interface CdkDropContainer { /** * Emits an event to indicate that the user moved an item into the container. * @param item Item that was moved into the container. - * @param xOffset Position of the item along the X axis. - * @param yOffset Position of the item along the Y axis. + * @param pointerX Position of the item along the X axis. + * @param pointerY Position of the item along the Y axis. */ - enter(item: CdkDrag, xOffset: number, yOffset: number): void; + enter(item: CdkDrag, pointerX: number, pointerY: number): void; /** * Removes an item from the container after it was dragged into another container by the user. @@ -52,7 +52,7 @@ export interface CdkDropContainer { * @param item Item whose index should be determined. */ getItemIndex(item: CdkDrag): number; - _sortItem(item: CdkDrag, xOffset: number, yOffset: number): void; + _sortItem(item: CdkDrag, pointerX: number, pointerY: number): void; _draggables: QueryList; _getSiblingContainerFromPosition(item: CdkDrag, x: number, y: number): CdkDropContainer | null; } diff --git a/src/cdk/drag-drop/drop.ts b/src/cdk/drag-drop/drop.ts index 7a2fdacea322..a83e6dfbae1c 100644 --- a/src/cdk/drag-drop/drop.ts +++ b/src/cdk/drag-drop/drop.ts @@ -30,6 +30,12 @@ import {moveItemInArray} from './drag-utils'; /** Counter used to generate unique ids for drop zones. */ let _uniqueIdCounter = 0; +/** + * Proximity, as a ratio to width/height, at which a + * dragged item will affect the drop container. + */ +const DROP_PROXIMITY_THRESHOLD = 0.05; + /** Container that wraps a set of draggable items. */ @Component({ moduleId: module.id, @@ -112,7 +118,8 @@ export class CdkDrop implements OnInit, OnDestroy { /** Cache of the dimensions of all the items and the sibling containers. */ private _positionCache = { items: [] as {drag: CdkDrag, clientRect: ClientRect, offset: number}[], - siblings: [] as {drop: CdkDrop, clientRect: ClientRect}[] + siblings: [] as {drop: CdkDrop, clientRect: ClientRect}[], + self: {} as ClientRect }; /** @@ -150,16 +157,16 @@ export class CdkDrop implements OnInit, OnDestroy { /** * Emits an event to indicate that the user moved an item into the container. * @param item Item that was moved into the container. - * @param xOffset Position of the item along the X axis. - * @param yOffset Position of the item along the Y axis. + * @param pointerX Position of the item along the X axis. + * @param pointerY Position of the item along the Y axis. */ - enter(item: CdkDrag, xOffset: number, yOffset: number): void { + enter(item: CdkDrag, pointerX: number, pointerY: number): void { this.entered.emit({item, container: this}); this.start(); // We use the coordinates of where the item entered the drop // zone to figure out at which index it should be inserted. - const newIndex = this._getItemIndexFromPointerPosition(item, xOffset, yOffset); + const newIndex = this._getItemIndexFromPointerPosition(item, pointerX, pointerY); const currentIndex = this._activeDraggables.indexOf(item); const newPositionReference = this._activeDraggables[newIndex]; const placeholder = item.getPlaceholderElement(); @@ -211,12 +218,17 @@ export class CdkDrop implements OnInit, OnDestroy { /** * Sorts an item inside the container based on its position. * @param item Item to be sorted. - * @param xOffset Position of the item along the X axis. - * @param yOffset Position of the item along the Y axis. + * @param pointerX Position of the item along the X axis. + * @param pointerY Position of the item along the Y axis. */ - _sortItem(item: CdkDrag, xOffset: number, yOffset: number): void { + _sortItem(item: CdkDrag, pointerX: number, pointerY: number): void { + // Don't sort the item if it's out of range. + if (!this._isPointerNearDropContainer(pointerX, pointerY)) { + return; + } + const siblings = this._positionCache.items; - const newIndex = this._getItemIndexFromPointerPosition(item, xOffset, yOffset); + const newIndex = this._getItemIndexFromPointerPosition(item, pointerX, pointerY); if (newIndex === -1 && siblings.length > 0) { return; @@ -321,6 +333,8 @@ export class CdkDrop implements OnInit, OnDestroy { .map(drop => typeof drop === 'string' ? this._dragDropRegistry.getDropContainer(drop)! : drop) .filter(drop => drop && drop !== this) .map(drop => ({drop, clientRect: drop.element.nativeElement.getBoundingClientRect()})); + + this._positionCache.self = this.element.nativeElement.getBoundingClientRect(); } /** Resets the container to its initial state. */ @@ -351,10 +365,10 @@ export class CdkDrop implements OnInit, OnDestroy { /** * Gets the index of an item in the drop container, based on the position of the user's pointer. * @param item Item that is being sorted. - * @param xOffset Position of the user's pointer along the X axis. - * @param yOffset Position of the user's pointer along the Y axis. + * @param pointerX Position of the user's pointer along the X axis. + * @param pointerY Position of the user's pointer along the Y axis. */ - private _getItemIndexFromPointerPosition(item: CdkDrag, xOffset: number, yOffset: number) { + private _getItemIndexFromPointerPosition(item: CdkDrag, pointerX: number, pointerY: number) { return this._positionCache.items.findIndex(({drag, clientRect}, _, array) => { if (drag === item) { // If there's only one item left in the container, it must be @@ -365,8 +379,22 @@ export class CdkDrop implements OnInit, OnDestroy { return this.orientation === 'horizontal' ? // Round these down since most browsers report client rects with // sub-pixel precision, whereas the mouse coordinates are rounded to pixels. - xOffset >= Math.floor(clientRect.left) && xOffset <= Math.floor(clientRect.right) : - yOffset >= Math.floor(clientRect.top) && yOffset <= Math.floor(clientRect.bottom); + pointerX >= Math.floor(clientRect.left) && pointerX <= Math.floor(clientRect.right) : + pointerY >= Math.floor(clientRect.top) && pointerY <= Math.floor(clientRect.bottom); }); } + + /** + * Checks whether the pointer coordinates are close to the drop container. + * @param pointerX Coordinates along the X axis. + * @param pointerY Coordinates along the Y axis. + */ + private _isPointerNearDropContainer(pointerX: number, pointerY: number): boolean { + const {top, right, bottom, left, width, height} = this._positionCache.self; + const xThreshold = width * DROP_PROXIMITY_THRESHOLD; + const yThreshold = height * DROP_PROXIMITY_THRESHOLD; + + return pointerY > top - yThreshold && pointerY < bottom + yThreshold && + pointerX > left - xThreshold && pointerX < right + xThreshold; + } }