Skip to content

Commit

Permalink
fix(drag-drop): ignore enter predicate when returning item to its ini…
Browse files Browse the repository at this point in the history
…tial container (#13972)

Something I noticed while putting together an example for the `enterPredicate` a while ago. Currently we allow an item to be returned to its initial container, even if the initial container isn't connected to the new one, however we don't do the same for the `enterPredicate`. This means that the user can get into the situation where they drag an item out, but then they're not allowed to return it, which seems weird. These changes will ignore the `enterPredicate`, if an item is being returned to the initial container.
  • Loading branch information
crisbeto authored and Vivian Hu committed Nov 12, 2018
1 parent 1a7173d commit 3fb5522
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 5 deletions.
40 changes: 40 additions & 0 deletions src/cdk/drag-drop/drag.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1609,6 +1609,46 @@ describe('CdkDrag', () => {
expect(fixture.componentInstance.droppedSpy).not.toHaveBeenCalled();
}));

it('should be able to move the element over a new container and return it to the initial ' +
'one, even if it no longer matches the enterPredicate', fakeAsync(() => {
const fixture = createComponent(ConnectedDropZones);
fixture.detectChanges();

const groups = fixture.componentInstance.groupedDragItems;
const dropZones = fixture.componentInstance.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();

fixture.componentInstance.dropInstances.first.enterPredicate = () => false;
fixture.detectChanges();

startDraggingViaMouse(fixture, item.element.nativeElement);

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();
}));

it('should transfer the DOM element from one drop zone to another', fakeAsync(() => {
const fixture = createComponent(ConnectedDropZones);
fixture.detectChanges();
Expand Down
2 changes: 1 addition & 1 deletion src/cdk/drag-drop/drag.ts
Original file line number Diff line number Diff line change
Expand Up @@ -521,7 +521,7 @@ export class CdkDrag<T = any> implements AfterViewInit, OnDestroy {
// 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)) {
this._initialContainer._canReturnItem(x, y)) {
newContainer = this._initialContainer;
}

Expand Down
2 changes: 1 addition & 1 deletion src/cdk/drag-drop/drop-list-container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ export interface CdkDropListContainer<T = any> {
_draggables: QueryList<CdkDrag>;
_getSiblingContainerFromPosition(item: CdkDrag, x: number, y: number):
CdkDropListContainer | null;
_canReturnItem(item: CdkDrag, x: number, y: number): boolean;
_canReturnItem(x: number, y: number): boolean;
}

/**
Expand Down
5 changes: 2 additions & 3 deletions src/cdk/drag-drop/drop-list.ts
Original file line number Diff line number Diff line change
Expand Up @@ -356,12 +356,11 @@ export class CdkDropList<T = any> implements OnInit, OnDestroy {
/**
* 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);
_canReturnItem(x: number, y: number): boolean {
return isInsideClientRect(this._positionCache.self, x, y);
}

/** Refreshes the position cache of the items and sibling containers. */
Expand Down

0 comments on commit 3fb5522

Please sign in to comment.