Skip to content

Commit

Permalink
fix(drag-drop): unable to start dragging in list if dragged item is d…
Browse files Browse the repository at this point in the history
…estroyed (angular#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 angular#18628.
  • Loading branch information
crisbeto authored Apr 20, 2020
1 parent cd75979 commit 7e66037
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 2 deletions.
43 changes: 43 additions & 0 deletions src/cdk/drag-drop/directives/drag.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
19 changes: 17 additions & 2 deletions src/cdk/drag-drop/drop-list-ref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -367,11 +367,20 @@ export class DropListRef<T = any> {
* @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;
Expand Down Expand Up @@ -631,7 +640,13 @@ export class DropListRef<T = any> {
(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 = [];
Expand Down

0 comments on commit 7e66037

Please sign in to comment.