From 0ac41a0896ae4cb4bd66f282e45bd596b6767d65 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Tue, 25 Sep 2018 16:08:11 +0200 Subject: [PATCH] fix(drag-drop): unable to return item to initial container within same drag sequence, if not connected to current drag container (#13247) Handles the case where the consumer has two drop containers where only one is connected to the other. Currently once the user drags an element outside the first one, they won't be able to return it. With these changes we allow the element to be returned, as long as it hasn't been dropped into the new container. Fixes #13246. --- src/cdk/drag-drop/drag.spec.ts | 43 +++++++++++++++++++++++++++++ src/cdk/drag-drop/drag.ts | 15 ++++++++-- src/cdk/drag-drop/drop-container.ts | 1 + src/cdk/drag-drop/drop.ts | 29 ++++++++++++++++--- 4 files changed, 81 insertions(+), 7 deletions(-) diff --git a/src/cdk/drag-drop/drag.spec.ts b/src/cdk/drag-drop/drag.spec.ts index 05a1b811b92..eda825c4fb9 100644 --- a/src/cdk/drag-drop/drag.spec.ts +++ b/src/cdk/drag-drop/drag.spec.ts @@ -1507,6 +1507,49 @@ describe('CdkDrag', () => { 'Expected CdkDrag to be returned to first container in memory'); })); + it('should be able to return an element to its initial container in the same sequence, ' + + 'even if it is not connected to the current container', fakeAsync(() => { + const fixture = createComponent(ConnectedDropZones); + fixture.detectChanges(); + + const groups = fixture.componentInstance.groupedDragItems; + const dropInstances = fixture.componentInstance.dropInstances.toArray(); + const dropZones = dropInstances.map(d => d.element.nativeElement); + const item = groups[0][1]; + const initialRect = item.element.nativeElement.getBoundingClientRect(); + const targetRect = groups[1][2].element.nativeElement.getBoundingClientRect(); + + // Change the `connectedTo` so the containers are only connected one-way. + dropInstances[0].connectedTo = dropInstances[1]; + dropInstances[1].connectedTo = []; + + dispatchMouseEvent(item.element.nativeElement, 'mousedown'); + fixture.detectChanges(); + + const placeholder = dropZones[0].querySelector('.cdk-drag-placeholder')!; + + expect(placeholder).toBeTruthy(); + expect(dropZones[0].contains(placeholder)) + .toBe(true, 'Expected placeholder to be inside the first container.'); + + dispatchMouseEvent(document, 'mousemove', targetRect.left + 1, targetRect.top + 1); + fixture.detectChanges(); + + expect(dropZones[1].contains(placeholder)) + .toBe(true, 'Expected placeholder to be inside second container.'); + + dispatchMouseEvent(document, 'mousemove', initialRect.left + 1, initialRect.top + 1); + fixture.detectChanges(); + + expect(dropZones[0].contains(placeholder)) + .toBe(true, 'Expected placeholder to be back inside first container.'); + + dispatchMouseEvent(document, 'mouseup'); + fixture.detectChanges(); + + expect(fixture.componentInstance.droppedSpy).not.toHaveBeenCalled(); + })); + }); }); diff --git a/src/cdk/drag-drop/drag.ts b/src/cdk/drag-drop/drag.ts index a3bf0c813e3..ee4fed6013b 100644 --- a/src/cdk/drag-drop/drag.ts +++ b/src/cdk/drag-drop/drag.ts @@ -430,7 +430,16 @@ export class CdkDrag implements AfterViewInit, OnDestroy { */ private _updateActiveDropContainer({x, y}) { // Drop container that draggable has been moved into. - const newContainer = this.dropContainer._getSiblingContainerFromPosition(this, x, y); + let newContainer = this.dropContainer._getSiblingContainerFromPosition(this, x, y); + + // If we couldn't find a new container to move the item into, and the item has left it's + // initial container, check whether the it's allowed to return into its original container. + // This handles the case where two containers are connected one way and the user tries to + // undo dragging an item into a new container. + if (!newContainer && this.dropContainer !== this._initialContainer && + this._initialContainer._canReturnItem(this, x, y)) { + newContainer = this._initialContainer; + } if (newContainer) { this._ngZone.run(() => { @@ -438,8 +447,8 @@ export class CdkDrag implements AfterViewInit, OnDestroy { this.exited.emit({item: this, container: this.dropContainer}); this.dropContainer.exit(this); // Notify the new container that the item has entered. - this.entered.emit({item: this, container: newContainer}); - this.dropContainer = newContainer; + this.entered.emit({item: this, container: newContainer!}); + this.dropContainer = newContainer!; this.dropContainer.enter(this, x, y); }); } diff --git a/src/cdk/drag-drop/drop-container.ts b/src/cdk/drag-drop/drop-container.ts index 7cf9823e058..0891e56c960 100644 --- a/src/cdk/drag-drop/drop-container.ts +++ b/src/cdk/drag-drop/drop-container.ts @@ -59,6 +59,7 @@ export interface CdkDropContainer { _sortItem(item: CdkDrag, pointerX: number, pointerY: number, delta: {x: number, y: number}): void; _draggables: QueryList; _getSiblingContainerFromPosition(item: CdkDrag, x: number, y: number): CdkDropContainer | null; + _canReturnItem(item: CdkDrag, x: number, y: number): boolean; } /** diff --git a/src/cdk/drag-drop/drop.ts b/src/cdk/drag-drop/drop.ts index b86073a3f3d..a6b5307b9a7 100644 --- a/src/cdk/drag-drop/drop.ts +++ b/src/cdk/drag-drop/drop.ts @@ -304,14 +304,23 @@ export class CdkDrop implements OnInit, OnDestroy { * @param y Position of the item along the Y axis. */ _getSiblingContainerFromPosition(item: CdkDrag, x: number, y: number): CdkDrop | null { - const result = this._positionCache.siblings.find(({clientRect}) => { - const {top, bottom, left, right} = clientRect; - return y >= top && y <= bottom && x >= left && x <= right; - }); + const result = this._positionCache.siblings + .find(sibling => isInsideClientRect(sibling.clientRect, x, y)); return result && result.drop.enterPredicate(item, this) ? result.drop : null; } + /** + * Checks whether an item that started in this container can be returned to it, + * after it was moved out into another container. + * @param item Item that is being checked. + * @param x Position of the item along the X axis. + * @param y Position of the item along the Y axis. + */ + _canReturnItem(item: CdkDrag, x: number, y: number): boolean { + return isInsideClientRect(this._positionCache.self, x, y) && this.enterPredicate(item, this); + } + /** Refreshes the position cache of the items and sibling containers. */ private _cachePositions() { const isHorizontal = this.orientation === 'horizontal'; @@ -451,3 +460,15 @@ function findIndex(array: T[], return -1; } + + +/** + * Checks whether some coordinates are within a `ClientRect`. + * @param clientRect ClientRect that is being checked. + * @param x Coordinates along the X axis. + * @param y Coordinates along the Y axis. + */ +function isInsideClientRect(clientRect: ClientRect, x: number, y: number) { + const {top, bottom, left, right} = clientRect; + return y >= top && y <= bottom && x >= left && x <= right; +}