fix(drag-drop): not working correctly inside transplanted views (#18499)

Currently the CDK drag&drop is set up so that it keeps track of the items inside each drop list via `ContentChildren`, however this doesn't work properly inside of a transplanted view (e.g. in the header of a `mat-table`). There are two main problems inside transplanted views:

1. On the first change detection the `ContentChildren` query may be empty, even though the items exist. Usually they're fine on the second run, but that may be too late, because the user could've started dragging already.
2. Even if we somehow ensured that the `ContentChildren` is up-to-date when dragging starts, Angular won't update the order of the transplanted view items in the query once they're shuffled around.

To work around these limitations and to make it possible to support more advanced usages like in `mat-table`, these changes switch to keeping track of the items using DI. Whenever an item is created or destroyed, it registers/deregisters itself with the closest drop list. Since the insertion order can be different from the DOM order, we use `compareDocumentPosition` to sort the items right before dragging starts.

Fixes #18482.

Author: crisbeto

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

@@ -1358,6 +1358,47 @@ describe('CdkDrag', () => {
       expect(fixture.componentInstance.dropInstance.data).toBe(fixture.componentInstance.items);
     });
 
+    it('should register an item with the drop container', () => {
+      const fixture = createComponent(DraggableInDropZone);
+      fixture.detectChanges();
+      const list = fixture.componentInstance.dropInstance;
+
+      spyOn(list, 'addItem').and.callThrough();
+
+      fixture.componentInstance.items.push({value: 'Extra', margin: 0, height: ITEM_HEIGHT});
+      fixture.detectChanges();
+
+      expect(list.addItem).toHaveBeenCalledTimes(1);
+    });
+
+    it('should remove an item from the drop container', () => {
+      const fixture = createComponent(DraggableInDropZone);
+      fixture.detectChanges();
+      const list = fixture.componentInstance.dropInstance;
+
+      spyOn(list, 'removeItem').and.callThrough();
+
+      fixture.componentInstance.items.pop();
+      fixture.detectChanges();
+
+      expect(list.removeItem).toHaveBeenCalledTimes(1);
+    });
+
+    it('should return the items sorted by their position in the DOM', () => {
+      const fixture = createComponent(DraggableInDropZone);
+      const items = fixture.componentInstance.items;
+      fixture.detectChanges();
+
+      // Insert a couple of items in the start and the middle so the list gets shifted around.
+      items.unshift({value: 'Extra 0', margin: 0, height: ITEM_HEIGHT});
+      items.splice(3, 0, {value: 'Extra 1', margin: 0, height: ITEM_HEIGHT});
+      fixture.detectChanges();
+
+      expect(fixture.componentInstance.dropInstance.getSortedItems().map(item => {
+        return item.element.nativeElement.textContent!.trim();
+      })).toEqual(['Extra 0', 'Zero', 'One', 'Extra 1', 'Two', 'Three']);
+    });
+
     it('should sync the drop list inputs with the drop list ref', () => {
       const fixture = createComponent(DraggableInDropZone);
       fixture.detectChanges();
@@ -5167,7 +5208,7 @@ class ConnectedDropZones implements AfterViewInit {
         this.groupedDragItems.push([]);
       }
 
-      this.groupedDragItems[index].push(...dropZone._draggables.toArray());
+      this.groupedDragItems[index].push(...dropZone.getSortedItems());
     });
   }
 }

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

@@ -215,6 +215,7 @@ export class CdkDrag<T = any> implements AfterViewInit, OnChanges, OnDestroy {
     // assigning the drop container both from here and the list.
     if (dropContainer) {
       this._dragRef._withDropContainer(dropContainer._dropListRef);
+      dropContainer.addItem(this);
     }
 
     this._syncInputs(this._dragRef);
@@ -303,6 +304,10 @@ export class CdkDrag<T = any> implements AfterViewInit, OnChanges, OnDestroy {
   }
 
   ngOnDestroy() {
+    if (this.dropContainer) {
+      this.dropContainer.removeItem(this);
+    }
+
     this._destroyed.next();
     this._destroyed.complete();
     this._dragRef.dispose();

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

@@ -8,13 +8,11 @@
 
 import {BooleanInput, coerceArray, coerceBooleanProperty} from '@angular/cdk/coercion';
 import {
-  ContentChildren,
   ElementRef,
   EventEmitter,
   Input,
   OnDestroy,
   Output,
-  QueryList,
   Optional,
   Directive,
   ChangeDetectorRef,
@@ -71,9 +69,6 @@ export class CdkDropList<T = any> implements AfterContentInit, OnDestroy {
   /** Reference to the underlying drop list instance. */
   _dropListRef: DropListRef<CdkDropList<T>>;
 
-  /** Draggable items in the container. */
-  @ContentChildren(CdkDrag, {descendants: true}) _draggables: QueryList<CdkDrag>;
-
   /**
    * Other draggable containers that this container is connected to and into which the
    * container's items can be transferred. Can either be references to other drop containers,
@@ -147,6 +142,15 @@ export class CdkDropList<T = any> implements AfterContentInit, OnDestroy {
   @Output('cdkDropListSorted')
   sorted: EventEmitter<CdkDragSortEvent<T>> = new EventEmitter<CdkDragSortEvent<T>>();
 
+  /**
+   * Keeps track of the items that are registered with this container. Historically we used to
+   * do this with a `ContentChildren` query, however queries don't handle transplanted views very
+   * well which means that we can't handle cases like dragging the headers of a `mat-table`
+   * correctly. What we do instead is to have the items register themselves with the container
+   * and then we sort them based on their position in the DOM.
+   */
+  private _unsortedItems = new Set<CdkDrag>();
+
   constructor(
       /** Element that the drop list is attached to. */
       public element: ElementRef<HTMLElement>, dragDrop: DragDrop,
@@ -187,18 +191,37 @@ export class CdkDropList<T = any> implements AfterContentInit, OnDestroy {
         .map(scrollable => scrollable.getElementRef().nativeElement);
       this._dropListRef.withScrollableParents(scrollableParents);
     }
+  }
 
-    this._draggables.changes
-      .pipe(startWith(this._draggables), takeUntil(this._destroyed))
-      .subscribe((items: QueryList<CdkDrag>) => {
-        this._dropListRef.withItems(items.reduce((filteredItems, drag) => {
-          if (drag.dropContainer === this) {
-            filteredItems.push(drag._dragRef);
-          }
+  /** Registers an items with the drop list. */
+  addItem(item: CdkDrag): void {
+    this._unsortedItems.add(item);
 
-          return filteredItems;
-        }, [] as DragRef[]));
-      });
+    if (this._dropListRef.isDragging()) {
+      this._syncItemsWithRef();
+    }
+  }
+
+  /** Removes an item from the drop list. */
+  removeItem(item: CdkDrag): void {
+    this._unsortedItems.delete(item);
+
+    if (this._dropListRef.isDragging()) {
+      this._syncItemsWithRef();
+    }
+  }
+
+  /** Gets the registered items in the list, sorted by their position in the DOM. */
+  getSortedItems(): CdkDrag[] {
+    return Array.from(this._unsortedItems).sort((a: CdkDrag, b: CdkDrag) => {
+      const documentPosition =
+          a._dragRef.getVisibleElement().compareDocumentPosition(b._dragRef.getVisibleElement());
+
+      // `compareDocumentPosition` returns a bitmask so we have to use a bitwise operator.
+      // https://developer.mozilla.org/en-US/docs/Web/API/Node/compareDocumentPosition
+      // tslint:disable-next-line:no-bitwise
+      return documentPosition & Node.DOCUMENT_POSITION_FOLLOWING ? -1 : 1;
+    });
   }
 
   ngOnDestroy() {
@@ -212,6 +235,7 @@ export class CdkDropList<T = any> implements AfterContentInit, OnDestroy {
       this._group._items.delete(this);
     }
 
+    this._unsortedItems.clear();
     this._dropListRef.dispose();
     this._destroyed.next();
     this._destroyed.complete();
@@ -310,6 +334,7 @@ export class CdkDropList<T = any> implements AfterContentInit, OnDestroy {
   /** Handles events from the underlying DropListRef. */
   private _handleEvents(ref: DropListRef<CdkDropList>) {
     ref.beforeStarted.subscribe(() => {
+      this._syncItemsWithRef();
       this._changeDetectorRef.markForCheck();
     });
 
@@ -371,6 +396,11 @@ export class CdkDropList<T = any> implements AfterContentInit, OnDestroy {
     }
   }
 
+  /** Syncs up the registered drag items with underlying drop list ref. */
+  private _syncItemsWithRef() {
+    this._dropListRef.withItems(this.getSortedItems().map(item => item._dragRef));
+  }
+
   static ngAcceptInputType_disabled: BooleanInput;
   static ngAcceptInputType_sortingDisabled: BooleanInput;
   static ngAcceptInputType_autoScrollDisabled: BooleanInput;

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

@@ -124,6 +124,9 @@ export class DragRef<T = any> {
   /** Drop container in which the DragRef resided when dragging began. */
   private _initialContainer: DropListRef;
 
+  /** Index at which the item started in its initial container. */
+  private _initialIndex: number;
+
   /** Cached scroll position on the page when the element was picked up. */
   private _scrollPosition: {top: number, left: number};
 
@@ -309,6 +312,14 @@ export class DragRef<T = any> {
     return this._rootElement;
   }
 
+  /**
+   * Gets the currently-visible element that represents the drag item.
+   * While dragging this is the placeholder, otherwise it's the root element.
+   */
+  getVisibleElement(): HTMLElement {
+    return this.isDragging() ? this.getPlaceholderElement() : this.getRootElement();
+  }
+
   /** Registers the handles that can be used to drag the element. */
   withHandles(handles: (HTMLElement | ElementRef<HTMLElement>)[]): this {
     this._handles = handles.map(handle => coerceElement(handle));
@@ -697,6 +708,10 @@ export class DragRef<T = any> {
       this._document.body.appendChild(parent.replaceChild(placeholder, element));
       getPreviewInsertionPoint(this._document).appendChild(preview);
       this._dropContainer.start();
+      this._initialContainer = this._dropContainer;
+      this._initialIndex = this._dropContainer.getItemIndex(this);
+    } else {
+      this._initialContainer = this._initialIndex = undefined!;
     }
   }
 
@@ -743,7 +758,6 @@ export class DragRef<T = any> {
     }
 
     this._hasStartedDragging = this._hasMoved = false;
-    this._initialContainer = this._dropContainer!;
 
     // Avoid multiple subscriptions and memory leaks when multi touch
     // (isDragging check above isn't enough because of possible temporal and/or dimensional delays)
@@ -796,13 +810,14 @@ export class DragRef<T = any> {
       this.dropped.next({
         item: this,
         currentIndex,
-        previousIndex: this._initialContainer.getItemIndex(this),
+        previousIndex: this._initialIndex,
         container: container,
         previousContainer: this._initialContainer,
         isPointerOverContainer,
         distance
       });
-      container.drop(this, currentIndex, this._initialContainer, isPointerOverContainer, distance);
+      container.drop(this, currentIndex, this._initialContainer, isPointerOverContainer, distance,
+          this._initialIndex);
       this._dropContainer = this._initialContainer;
     });
   }
@@ -831,7 +846,10 @@ export class DragRef<T = any> {
         this._dropContainer!.exit(this);
         // Notify the new container that the item has entered.
         this._dropContainer = newContainer!;
-        this._dropContainer.enter(this, x, y);
+        this._dropContainer.enter(this, x, y,
+            // If we're re-entering the initial container,
+            // put item the into its starting index to begin with.
+            newContainer === this._initialContainer ? this._initialIndex : undefined);
         this.entered.next({
           item: this,
           container: newContainer!,

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

@@ -256,18 +256,26 @@ export class DropListRef<T = any> {
    * @param item Item that was moved into the container.
    * @param pointerX Position of the item along the X axis.
    * @param pointerY Position of the item along the Y axis.
+   * @param index Index at which the item entered. If omitted, the container will try to figure it
+   *   out automatically.
    */
-  enter(item: DragRef, pointerX: number, pointerY: number): void {
+  enter(item: DragRef, pointerX: number, pointerY: number, index?: number): void {
     this.start();
 
     // If sorting is disabled, we want the item to return to its starting
     // position if the user is returning it to its initial container.
-    let newIndex = this.sortingDisabled ? this._draggables.indexOf(item) : -1;
+    let newIndex: number;
 
-    if (newIndex === -1) {
-      // We use the coordinates of where the item entered the drop
-      // zone to figure out at which index it should be inserted.
-      newIndex = this._getItemIndexFromPointerPosition(item, pointerX, pointerY);
+    if (index == null) {
+      newIndex = this.sortingDisabled ? this._draggables.indexOf(item) : -1;
+
+      if (newIndex === -1) {
+        // We use the coordinates of where the item entered the drop
+        // zone to figure out at which index it should be inserted.
+        newIndex = this._getItemIndexFromPointerPosition(item, pointerX, pointerY);
+      }
+    } else {
+      newIndex = index;
     }
 
     const activeDraggables = this._activeDraggables;
@@ -325,14 +333,22 @@ export class DropListRef<T = any> {
    * @param isPointerOverContainer Whether the user's pointer was over the
    *    container when the item was dropped.
    * @param distance Distance the user has dragged since the start of the dragging sequence.
+   * @param previousIndex Index of the item when dragging started.
+   *
+   * @breaking-change 11.0.0 `previousIndex` parameter to become required.
    */
   drop(item: DragRef, currentIndex: number, previousContainer: DropListRef,
-    isPointerOverContainer: boolean, distance: Point): void {
+    isPointerOverContainer: boolean, distance: Point, previousIndex?: number): void {
     this._reset();
-    this.dropped.next({
-      item,
+
+    // @breaking-change 11.0.0 Remove this fallback logic once `previousIndex` is a required param.
+    if (previousIndex == null) {
+      previousIndex = previousContainer.getItemIndex(item);
+    }
+
+    this.dropped.next({item,
       currentIndex,
-      previousIndex: previousContainer.getItemIndex(item),
+      previousIndex,
       container: this,
       previousContainer,
       isPointerOverContainer,
@@ -591,11 +607,7 @@ export class DropListRef<T = any> {
     const isHorizontal = this._orientation === 'horizontal';
 
     this._itemPositions = this._activeDraggables.map(drag => {
-      const elementToMeasure = this._dragDropRegistry.isDragging(drag) ?
-          // If the element is being dragged, we have to measure the
-          // placeholder, because the element is hidden.
-          drag.getPlaceholderElement() :
-          drag.getRootElement();
+      const elementToMeasure = drag.getVisibleElement();
       return {drag, offset: 0, clientRect: getMutableClientRect(elementToMeasure)};
     }).sort((a, b) => {
       return isHorizontal ? a.clientRect.left - b.clientRect.left :

tools/public_api_guard/cdk/drag-drop.d.ts

@@ -143,7 +143,6 @@ export interface CdkDragStart<T = any> {
 }
 
 export declare class CdkDropList<T = any> implements AfterContentInit, OnDestroy {
-    _draggables: QueryList<CdkDrag>;
     _dropListRef: DropListRef<CdkDropList<T>>;
     autoScrollDisabled: boolean;
     connectedTo: (CdkDropList | string)[] | CdkDropList | string;
@@ -163,17 +162,20 @@ export declare class CdkDropList<T = any> implements AfterContentInit, OnDestroy
     constructor(
     element: ElementRef<HTMLElement>, dragDrop: DragDrop, _changeDetectorRef: ChangeDetectorRef, _dir?: Directionality | undefined, _group?: CdkDropListGroup<CdkDropList<any>> | undefined,
     _scrollDispatcher?: ScrollDispatcher | undefined, config?: DragDropConfig);
+    addItem(item: CdkDrag): void;
     drop(item: CdkDrag, currentIndex: number, previousContainer: CdkDropList, isPointerOverContainer: boolean): void;
     enter(item: CdkDrag, pointerX: number, pointerY: number): void;
     exit(item: CdkDrag): void;
     getItemIndex(item: CdkDrag): number;
+    getSortedItems(): CdkDrag[];
     ngAfterContentInit(): void;
     ngOnDestroy(): void;
+    removeItem(item: CdkDrag): void;
     start(): void;
     static ngAcceptInputType_autoScrollDisabled: BooleanInput;
     static ngAcceptInputType_disabled: BooleanInput;
     static ngAcceptInputType_sortingDisabled: BooleanInput;
-    static ɵdir: i0.ɵɵDirectiveDefWithMeta<CdkDropList<any>, "[cdkDropList], cdk-drop-list", ["cdkDropList"], { "connectedTo": "cdkDropListConnectedTo"; "data": "cdkDropListData"; "orientation": "cdkDropListOrientation"; "id": "id"; "lockAxis": "cdkDropListLockAxis"; "disabled": "cdkDropListDisabled"; "sortingDisabled": "cdkDropListSortingDisabled"; "enterPredicate": "cdkDropListEnterPredicate"; "autoScrollDisabled": "cdkDropListAutoScrollDisabled"; }, { "dropped": "cdkDropListDropped"; "entered": "cdkDropListEntered"; "exited": "cdkDropListExited"; "sorted": "cdkDropListSorted"; }, ["_draggables"]>;
+    static ɵdir: i0.ɵɵDirectiveDefWithMeta<CdkDropList<any>, "[cdkDropList], cdk-drop-list", ["cdkDropList"], { "connectedTo": "cdkDropListConnectedTo"; "data": "cdkDropListData"; "orientation": "cdkDropListOrientation"; "id": "id"; "lockAxis": "cdkDropListLockAxis"; "disabled": "cdkDropListDisabled"; "sortingDisabled": "cdkDropListSortingDisabled"; "enterPredicate": "cdkDropListEnterPredicate"; "autoScrollDisabled": "cdkDropListAutoScrollDisabled"; }, { "dropped": "cdkDropListDropped"; "entered": "cdkDropListEntered"; "exited": "cdkDropListExited"; "sorted": "cdkDropListSorted"; }, never>;
     static ɵfac: i0.ɵɵFactoryDef<CdkDropList<any>>;
 }
 
@@ -298,6 +300,7 @@ export declare class DragRef<T = any> {
     getFreeDragPosition(): Readonly<Point>;
     getPlaceholderElement(): HTMLElement;
     getRootElement(): HTMLElement;
+    getVisibleElement(): HTMLElement;
     isDragging(): boolean;
     reset(): void;
     setFreeDragPosition(value: Point): this;
@@ -368,8 +371,8 @@ export declare class DropListRef<T = any> {
     _stopScrolling(): void;
     connectedTo(connectedTo: DropListRef[]): this;
     dispose(): void;
-    drop(item: DragRef, currentIndex: number, previousContainer: DropListRef, isPointerOverContainer: boolean, distance: Point): void;
-    enter(item: DragRef, pointerX: number, pointerY: number): void;
+    drop(item: DragRef, currentIndex: number, previousContainer: DropListRef, isPointerOverContainer: boolean, distance: Point, previousIndex?: number): void;
+    enter(item: DragRef, pointerX: number, pointerY: number, index?: number): void;
     exit(item: DragRef): void;
     getItemIndex(item: DragRef): number;
     isDragging(): boolean;