Skip to content

Commit

Permalink
fix(drag-drop): unable to return item to initial container within sam…
Browse files Browse the repository at this point in the history
…e 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.
  • Loading branch information
crisbeto authored and jelbourn committed Sep 25, 2018
1 parent 40fb5cb commit 0ac41a0
Show file tree
Hide file tree
Showing 4 changed files with 81 additions and 7 deletions.
43 changes: 43 additions & 0 deletions src/cdk/drag-drop/drag.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}));

});

});
Expand Down
15 changes: 12 additions & 3 deletions src/cdk/drag-drop/drag.ts
Original file line number Diff line number Diff line change
Expand Up @@ -430,16 +430,25 @@ export class CdkDrag<T = any> 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(() => {
// Notify the old container that the item has left.
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);
});
}
Expand Down
1 change: 1 addition & 0 deletions src/cdk/drag-drop/drop-container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ export interface CdkDropContainer<T = any> {
_sortItem(item: CdkDrag, pointerX: number, pointerY: number, delta: {x: number, y: number}): void;
_draggables: QueryList<CdkDrag>;
_getSiblingContainerFromPosition(item: CdkDrag, x: number, y: number): CdkDropContainer | null;
_canReturnItem(item: CdkDrag, x: number, y: number): boolean;
}

/**
Expand Down
29 changes: 25 additions & 4 deletions src/cdk/drag-drop/drop.ts
Original file line number Diff line number Diff line change
Expand Up @@ -304,14 +304,23 @@ export class CdkDrop<T = any> 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';
Expand Down Expand Up @@ -451,3 +460,15 @@ function findIndex<T>(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;
}

0 comments on commit 0ac41a0

Please sign in to comment.