Skip to content

Commit c6b687b

Browse files
committed
fix(drag-drop): unable to start dragging in list if dragged item is destroyed
We currently have some logic which cleans up the dragging state if the dragged item is destroyed, but nothing notifies the parent drop list which leaves it in the dragging state. Next time the user tries to drag, they won't be able to, because the list still thinks that its dragging. These changes add some logic to abort the dragging if the dragged item is removed from the list. Fixes #18628.
1 parent 695dde6 commit c6b687b

File tree

2 files changed

+60
-2
lines changed

2 files changed

+60
-2
lines changed

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

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3801,6 +3801,49 @@ describe('CdkDrag', () => {
38013801
expect(styles.scrollSnapType || styles.msScrollSnapType).toBe('block');
38023802
}));
38033803

3804+
it('should be able to start dragging again if the dragged item is destroyed', fakeAsync(() => {
3805+
// We have some behavior where we move the dragged element out to the bottom of the `body`,
3806+
// in order to work around a browser issue. We restore the element when dragging stops, but
3807+
// the problem is that if it's destroyed before we've had a chance to return it, ViewEngine
3808+
// will throw an error since the element isn't in its original parent. Skip this test if the
3809+
// component hasn't been compiled with Ivy since the assertions depend on the element being
3810+
// removed while dragging.
3811+
// TODO(crisbeto): remove this check once ViewEngine has been dropped.
3812+
if (!DraggableInDropZone.hasOwnProperty('ɵcmp')) {
3813+
return;
3814+
}
3815+
3816+
const fixture = createComponent(DraggableInDropZone);
3817+
fixture.detectChanges();
3818+
3819+
let item = fixture.componentInstance.dragItems.first;
3820+
startDraggingViaMouse(fixture, item.element.nativeElement);
3821+
expect(document.querySelector('.cdk-drop-list-dragging'))
3822+
.toBeTruthy('Expected to drag initially.');
3823+
3824+
fixture.componentInstance.items = [
3825+
{value: 'Five', height: ITEM_HEIGHT, margin: 0},
3826+
{value: 'Six', height: ITEM_HEIGHT, margin: 0}
3827+
];
3828+
fixture.detectChanges();
3829+
3830+
expect(fixture.componentInstance.droppedSpy).not.toHaveBeenCalled();
3831+
expect(document.querySelector('.cdk-drop-list-dragging'))
3832+
.toBeFalsy('Expected not to be dragging after item is destroyed.');
3833+
3834+
item = fixture.componentInstance.dragItems.first;
3835+
startDraggingViaMouse(fixture, item.element.nativeElement);
3836+
3837+
expect(document.querySelector('.cdk-drop-list-dragging'))
3838+
.toBeTruthy('Expected to be able to start a new drag sequence.');
3839+
3840+
dispatchMouseEvent(document, 'mouseup');
3841+
fixture.detectChanges();
3842+
flush();
3843+
3844+
expect(fixture.componentInstance.droppedSpy).toHaveBeenCalledTimes(1);
3845+
}));
3846+
38043847
});
38053848

38063849
describe('in a connected drop container', () => {

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

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -367,11 +367,20 @@ export class DropListRef<T = any> {
367367
* @param items Items that are a part of this list.
368368
*/
369369
withItems(items: DragRef[]): this {
370+
const previousItems = this._draggables;
370371
this._draggables = items;
371372
items.forEach(item => item._withDropContainer(this));
372373

373374
if (this.isDragging()) {
374-
this._cacheItems();
375+
const draggedItems = previousItems.filter(item => item.isDragging());
376+
377+
// If all of the items being dragged were removed
378+
// from the list, abort the current drag sequence.
379+
if (draggedItems.every(item => items.indexOf(item) === -1)) {
380+
this._reset();
381+
} else {
382+
this._cacheItems();
383+
}
375384
}
376385

377386
return this;
@@ -631,7 +640,13 @@ export class DropListRef<T = any> {
631640
(styles as any).scrollSnapType = styles.msScrollSnapType = this._initialScrollSnap;
632641

633642
// TODO(crisbeto): may have to wait for the animations to finish.
634-
this._activeDraggables.forEach(item => item.getRootElement().style.transform = '');
643+
this._activeDraggables.forEach(item => {
644+
const rootElement = item.getRootElement();
645+
646+
if (rootElement) {
647+
rootElement.style.transform = '';
648+
}
649+
});
635650
this._siblings.forEach(sibling => sibling._stopReceiving(this));
636651
this._activeDraggables = [];
637652
this._itemPositions = [];

0 commit comments

Comments
 (0)