Skip to content

Commit 7bc095b

Browse files
crisbetojelbourn
authored andcommitted
fix(drag-drop): error when item enters from the top and last has an intermediate child (#19361)
In #19116 some logic was added so that we use `insertBefore` when an item is entering from the top, rather than `appendChild`. The problem is that an error will be thrown if the first item in the drop list isn't a sibling of the first draggable element. Fixes #19359.
1 parent 27c64a1 commit 7bc095b

File tree

2 files changed

+69
-8
lines changed

2 files changed

+69
-8
lines changed

src/cdk/drag-drop/directives/drag.spec.ts

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4352,6 +4352,29 @@ describe('CdkDrag', () => {
43524352
dispatchMouseEvent(document, 'mouseup');
43534353
}));
43544354

4355+
it('should not throw when entering from the top with an intermediate sibling present',
4356+
fakeAsync(() => {
4357+
const fixture = createComponent(ConnectedDropZonesWithIntermediateSibling);
4358+
4359+
// Make sure there's only one item in the first list.
4360+
fixture.componentInstance.todo = ['things'];
4361+
fixture.detectChanges();
4362+
4363+
const groups = fixture.componentInstance.groupedDragItems;
4364+
const dropZones = fixture.componentInstance.dropInstances.map(d => d.element.nativeElement);
4365+
const item = groups[0][0];
4366+
4367+
// Add some initial padding as the target drop zone
4368+
dropZones[1].style.paddingTop = '10px';
4369+
const targetRect = dropZones[1].getBoundingClientRect();
4370+
4371+
expect(() => {
4372+
dragElementViaMouse(fixture, item.element.nativeElement, targetRect.left, targetRect.top);
4373+
flush();
4374+
fixture.detectChanges();
4375+
}).not.toThrow();
4376+
}));
4377+
43554378
it('should assign a default id on each drop zone', fakeAsync(() => {
43564379
const fixture = createComponent(ConnectedDropZones);
43574380
fixture.detectChanges();
@@ -6072,6 +6095,46 @@ class DraggableInHorizontalFlexDropZoneWithMatchSizePreview {
60726095
}
60736096

60746097

6098+
@Component({
6099+
styles: CONNECTED_DROP_ZONES_STYLES,
6100+
template: `
6101+
<div
6102+
cdkDropList
6103+
#todoZone="cdkDropList"
6104+
[cdkDropListData]="todo"
6105+
[cdkDropListConnectedTo]="[doneZone]"
6106+
(cdkDropListDropped)="droppedSpy($event)"
6107+
(cdkDropListEntered)="enteredSpy($event)">
6108+
<div
6109+
[cdkDragData]="item"
6110+
(cdkDragEntered)="itemEnteredSpy($event)"
6111+
*ngFor="let item of todo"
6112+
cdkDrag>{{item}}</div>
6113+
</div>
6114+
6115+
<div
6116+
cdkDropList
6117+
#doneZone="cdkDropList"
6118+
[cdkDropListData]="done"
6119+
[cdkDropListConnectedTo]="[todoZone]"
6120+
(cdkDropListDropped)="droppedSpy($event)"
6121+
(cdkDropListEntered)="enteredSpy($event)">
6122+
6123+
<div>Hello there</div>
6124+
<div>
6125+
<div
6126+
[cdkDragData]="item"
6127+
(cdkDragEntered)="itemEnteredSpy($event)"
6128+
*ngFor="let item of done"
6129+
cdkDrag>{{item}}</div>
6130+
</div>
6131+
</div>
6132+
`
6133+
})
6134+
class ConnectedDropZonesWithIntermediateSibling extends ConnectedDropZones {
6135+
}
6136+
6137+
60756138
/**
60766139
* Drags an element to a position on the page using the mouse.
60776140
* @param fixture Fixture on which to run change detection.

src/cdk/drag-drop/drop-list-ref.ts

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -301,15 +301,13 @@ export class DropListRef<T = any> {
301301
const element = newPositionReference.getRootElement();
302302
element.parentElement!.insertBefore(placeholder, element);
303303
activeDraggables.splice(newIndex, 0, item);
304+
} else if (this._shouldEnterAsFirstChild(pointerX, pointerY)) {
305+
const reference = activeDraggables[0].getRootElement();
306+
reference.parentNode!.insertBefore(placeholder, reference);
307+
activeDraggables.unshift(item);
304308
} else {
305-
const element = coerceElement(this.element);
306-
if (this._shouldEnterAsFirstChild(pointerX, pointerY)) {
307-
element.insertBefore(placeholder, activeDraggables[0].getRootElement());
308-
activeDraggables.unshift(item);
309-
} else {
310-
element.appendChild(placeholder);
311-
activeDraggables.push(item);
312-
}
309+
coerceElement(this.element).appendChild(placeholder);
310+
activeDraggables.push(item);
313311
}
314312

315313
// The transform needs to be cleared so it doesn't throw off the measurements.

0 commit comments

Comments
 (0)