Skip to content

Commit 72fe1e4

Browse files
crisbetojelbourn
authored andcommitted
fix(drag-drop): unable to start dragging in list if dragged item is destroyed (#19055)
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. (cherry picked from commit 7e66037)
1 parent a65872d commit 72fe1e4

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
@@ -3818,6 +3818,49 @@ describe('CdkDrag', () => {
38183818
expect(styles.scrollSnapType || styles.msScrollSnapType).toBe('block');
38193819
}));
38203820

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

38233866
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)