Skip to content

fix(drag-drop): unable to drag last item back into initial container #12261

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 23, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 60 additions & 12 deletions src/cdk-experimental/drag-drop/drag.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -657,7 +657,8 @@ describe('CdkDrag', () => {
const initialRect = item.element.nativeElement.getBoundingClientRect();
const targetRect = groups[1][2].element.nativeElement.getBoundingClientRect();

expect(dropZones[0].contains(item.element.nativeElement)).toBe(true);
expect(dropZones[0].contains(item.element.nativeElement))
.toBe(true, 'Expected placeholder to be inside the first container.');
dispatchMouseEvent(item.element.nativeElement, 'mousedown');
fixture.detectChanges();

Expand All @@ -675,7 +676,7 @@ describe('CdkDrag', () => {
fixture.detectChanges();

expect(dropZones[0].contains(placeholder))
.toBe(true, 'Expected placeholder to be inside first container.');
.toBe(true, 'Expected placeholder to be back inside first container.');

dispatchMouseEvent(document, 'mouseup');
fixture.detectChanges();
Expand Down Expand Up @@ -746,6 +747,46 @@ describe('CdkDrag', () => {
assertDownwardSorting(fixture, Array.from(dropZones[1].querySelectorAll('.cdk-drag')));
}));

it('should be able to return the last item inside its initial container', fakeAsync(() => {
const fixture = createComponent(ConnectedDropZones);

// Make sure there's only one item in the first list.
fixture.componentInstance.todo = ['things'];
fixture.detectChanges();

const groups = fixture.componentInstance.groupedDragItems;
const dropZones = fixture.componentInstance.dropInstances.map(d => d.element.nativeElement);
const item = groups[0][0];
const initialRect = item.element.nativeElement.getBoundingClientRect();
const targetRect = groups[1][0].element.nativeElement.getBoundingClientRect();

expect(dropZones[0].contains(item.element.nativeElement))
.toBe(true, 'Expected placeholder to be inside the first container.');
dispatchMouseEvent(item.element.nativeElement, 'mousedown');
fixture.detectChanges();

const placeholder = dropZones[0].querySelector('.cdk-drag-placeholder')!;

expect(placeholder).toBeTruthy();

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 Expand Up @@ -916,29 +957,36 @@ export class DraggableInDropZoneWithCustomPlaceholder {


@Component({
encapsulation: ViewEncapsulation.None,
styles: [`
.cdk-drop {
display: block;
width: 100px;
min-height: ${ITEM_HEIGHT}px;
background: hotpink;
}

.cdk-drag {
display: block;
height: ${ITEM_HEIGHT}px;
background: red;
}
`],
template: `
<cdk-drop
#todoZone
style="display: block; width: 100px; background: pink;"
[data]="todo"
[connectedTo]="[doneZone]"
(dropped)="droppedSpy($event)">
<div
*ngFor="let item of todo"
cdkDrag
style="width: 100%; height: ${ITEM_HEIGHT}px; background: red;">{{item}}</div>
<div *ngFor="let item of todo" cdkDrag>{{item}}</div>
</cdk-drop>

<cdk-drop
#doneZone
style="display: block; width: 100px; background: purple;"
[data]="done"
[connectedTo]="[todoZone]"
(dropped)="droppedSpy($event)">
<div
*ngFor="let item of done"
cdkDrag
style="width: 100%; height: ${ITEM_HEIGHT}px; background: green;">{{item}}</div>
<div *ngFor="let item of done" cdkDrag>{{item}}</div>
</cdk-drop>
`
})
Expand Down
2 changes: 1 addition & 1 deletion src/cdk-experimental/drag-drop/drag.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ export class CdkDrag implements AfterContentInit, OnDestroy {
private _activeTransform: Point = {x: 0, y: 0};

/** Whether the element is being dragged. */
private _isDragging = false;
_isDragging = false;

/** Whether the element has moved since the user started dragging it. */
private _hasMoved = false;
Expand Down
9 changes: 7 additions & 2 deletions src/cdk-experimental/drag-drop/drop.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,9 @@ export class CdkDrop<T = any> {
const siblings = this._positionCache.items;
const newPosition = siblings.find(({drag, clientRect}) => {
if (drag === item) {
return false;
// If there's only one left item in the container, it must be
// the dragged item itself so we use it as a reference.
return siblings.length < 2;
}

return this.orientation === 'horizontal' ?
Expand All @@ -153,7 +155,10 @@ export class CdkDrop<T = any> {
return;
}

const element = newPosition ? newPosition.drag.element.nativeElement : null;
// Don't take the element of a dragged item as a reference,
// because it has been moved down to the end of the body.
const element = (newPosition && !newPosition.drag._isDragging) ?
newPosition.drag.element.nativeElement : null;
const next = element ? element!.nextSibling : null;
const parent = element ? element.parentElement! : this.element.nativeElement;
const placeholder = item.getPlaceholderElement();
Expand Down