From c6b687b06492fadf45ef8a064317cab4a65751ff Mon Sep 17 00:00:00 2001 From: crisbeto Date: Sun, 12 Apr 2020 16:56:18 +0200 Subject: [PATCH] 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. --- src/cdk/drag-drop/directives/drag.spec.ts | 43 +++++++++++++++++++++++ src/cdk/drag-drop/drop-list-ref.ts | 19 ++++++++-- 2 files changed, 60 insertions(+), 2 deletions(-) diff --git a/src/cdk/drag-drop/directives/drag.spec.ts b/src/cdk/drag-drop/directives/drag.spec.ts index 942e9599fb35..2e98a642285a 100644 --- a/src/cdk/drag-drop/directives/drag.spec.ts +++ b/src/cdk/drag-drop/directives/drag.spec.ts @@ -3801,6 +3801,49 @@ describe('CdkDrag', () => { expect(styles.scrollSnapType || styles.msScrollSnapType).toBe('block'); })); + it('should be able to start dragging again if the dragged item is destroyed', fakeAsync(() => { + // We have some behavior where we move the dragged element out to the bottom of the `body`, + // in order to work around a browser issue. We restore the element when dragging stops, but + // the problem is that if it's destroyed before we've had a chance to return it, ViewEngine + // will throw an error since the element isn't in its original parent. Skip this test if the + // component hasn't been compiled with Ivy since the assertions depend on the element being + // removed while dragging. + // TODO(crisbeto): remove this check once ViewEngine has been dropped. + if (!DraggableInDropZone.hasOwnProperty('ɵcmp')) { + return; + } + + const fixture = createComponent(DraggableInDropZone); + fixture.detectChanges(); + + let item = fixture.componentInstance.dragItems.first; + startDraggingViaMouse(fixture, item.element.nativeElement); + expect(document.querySelector('.cdk-drop-list-dragging')) + .toBeTruthy('Expected to drag initially.'); + + fixture.componentInstance.items = [ + {value: 'Five', height: ITEM_HEIGHT, margin: 0}, + {value: 'Six', height: ITEM_HEIGHT, margin: 0} + ]; + fixture.detectChanges(); + + expect(fixture.componentInstance.droppedSpy).not.toHaveBeenCalled(); + expect(document.querySelector('.cdk-drop-list-dragging')) + .toBeFalsy('Expected not to be dragging after item is destroyed.'); + + item = fixture.componentInstance.dragItems.first; + startDraggingViaMouse(fixture, item.element.nativeElement); + + expect(document.querySelector('.cdk-drop-list-dragging')) + .toBeTruthy('Expected to be able to start a new drag sequence.'); + + dispatchMouseEvent(document, 'mouseup'); + fixture.detectChanges(); + flush(); + + expect(fixture.componentInstance.droppedSpy).toHaveBeenCalledTimes(1); + })); + }); describe('in a connected drop container', () => { diff --git a/src/cdk/drag-drop/drop-list-ref.ts b/src/cdk/drag-drop/drop-list-ref.ts index b0722d34a9ba..9e8ef001d610 100644 --- a/src/cdk/drag-drop/drop-list-ref.ts +++ b/src/cdk/drag-drop/drop-list-ref.ts @@ -367,11 +367,20 @@ export class DropListRef { * @param items Items that are a part of this list. */ withItems(items: DragRef[]): this { + const previousItems = this._draggables; this._draggables = items; items.forEach(item => item._withDropContainer(this)); if (this.isDragging()) { - this._cacheItems(); + const draggedItems = previousItems.filter(item => item.isDragging()); + + // If all of the items being dragged were removed + // from the list, abort the current drag sequence. + if (draggedItems.every(item => items.indexOf(item) === -1)) { + this._reset(); + } else { + this._cacheItems(); + } } return this; @@ -631,7 +640,13 @@ export class DropListRef { (styles as any).scrollSnapType = styles.msScrollSnapType = this._initialScrollSnap; // TODO(crisbeto): may have to wait for the animations to finish. - this._activeDraggables.forEach(item => item.getRootElement().style.transform = ''); + this._activeDraggables.forEach(item => { + const rootElement = item.getRootElement(); + + if (rootElement) { + rootElement.style.transform = ''; + } + }); this._siblings.forEach(sibling => sibling._stopReceiving(this)); this._activeDraggables = []; this._itemPositions = [];