From 7e660371de649e866d05388a5355b087923b73e3 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Mon, 20 Apr 2020 16:33:06 +0200 Subject: [PATCH] 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. --- 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 57302017ea1a..81e161776eed 100644 --- a/src/cdk/drag-drop/directives/drag.spec.ts +++ b/src/cdk/drag-drop/directives/drag.spec.ts @@ -3818,6 +3818,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 = [];