fix(drag-drop): unable to move item into connected container by passing through another container (#14651)

crisbeto · vivian-hu-zz · commit 0bd93dd64e8e · 2019-01-16T10:39:35.000-08:00
Fixes not being able to move an item from one container into another by passing it through an intermediate container that isn't connected to the final one. The issue comes from the fact that the way things are set up at the moment, the container from which the item started the sequence knows which containers it can go into, however all that knowledge is reset once the item enters into a different container. These changes rework the logic to have the individual containers know whether the item can enter into them and have the source container "ask" each of its siblings whether the item can enter. Fixes #14645.
diff --git a/src/cdk/drag-drop/directives/drag.spec.ts b/src/cdk/drag-drop/directives/drag.spec.ts
@@ -2736,6 +2736,63 @@ describe('CdkDrag', () => {
             'Expected new container not to have the receiving class after entering.');
       }));
 
+    it('should be able to move the item over an intermediate container before ' +
+      'dropping it into the final one', fakeAsync(() => {
+        const fixture = createComponent(ConnectedDropZones);
+        fixture.detectChanges();
+
+        const dropInstances = fixture.componentInstance.dropInstances.toArray();
+        dropInstances[0].connectedTo = [dropInstances[1], dropInstances[2]];
+        dropInstances[1].connectedTo = [];
+        dropInstances[2].connectedTo = [];
+        fixture.detectChanges();
+
+        const groups = fixture.componentInstance.groupedDragItems;
+        const dropZones = dropInstances.map(d => d.element.nativeElement);
+        const item = groups[0][1];
+        const intermediateRect = dropZones[1].getBoundingClientRect();
+        const finalRect = dropZones[2].getBoundingClientRect();
+
+        startDraggingViaMouse(fixture, item.element.nativeElement);
+
+        const placeholder = dropZones[0].querySelector('.cdk-drag-placeholder')!;
+
+        expect(placeholder).toBeTruthy();
+        expect(dropZones[0].contains(placeholder))
+            .toBe(true, 'Expected placeholder to be inside the first container.');
+
+        dispatchMouseEvent(document, 'mousemove',
+            intermediateRect.left + 1, intermediateRect.top + 1);
+        fixture.detectChanges();
+
+        expect(dropZones[1].contains(placeholder))
+            .toBe(true, 'Expected placeholder to be inside second container.');
+
+        dispatchMouseEvent(document, 'mousemove', finalRect.left + 1, finalRect.top + 1);
+        fixture.detectChanges();
+
+        expect(dropZones[2].contains(placeholder))
+            .toBe(true, 'Expected placeholder to be inside third container.');
+
+        dispatchMouseEvent(document, 'mouseup');
+        fixture.detectChanges();
+        flush();
+        fixture.detectChanges();
+
+        const event = fixture.componentInstance.droppedSpy.calls.mostRecent().args[0];
+
+        expect(event).toBeTruthy();
+        expect(event).toEqual(jasmine.objectContaining({
+          previousIndex: 1,
+          currentIndex: 0,
+          item: groups[0][1],
+          container: dropInstances[2],
+          previousContainer: dropInstances[0],
+          isPointerOverContainer: false
+        }));
+
+      }));
+
   });
 
 });
@@ -3025,6 +3082,14 @@ class DraggableInDropZoneWithCustomPlaceholder {
       (cdkDropListDropped)="droppedSpy($event)">
       <div [cdkDragData]="item" *ngFor="let item of done" cdkDrag>{{item}}</div>
     </div>
+
+    <div
+      cdkDropList
+      #extraZone="cdkDropList"
+      [cdkDropListData]="extra"
+      (cdkDropListDropped)="droppedSpy($event)">
+      <div [cdkDragData]="item" *ngFor="let item of extra" cdkDrag>{{item}}</div>
+    </div>
   `
 })
 class ConnectedDropZones implements AfterViewInit {
@@ -3034,6 +3099,7 @@ class ConnectedDropZones implements AfterViewInit {
   groupedDragItems: CdkDrag[][] = [];
   todo = ['Zero', 'One', 'Two', 'Three'];
   done = ['Four', 'Five', 'Six'];
+  extra = [];
   droppedSpy = jasmine.createSpy('dropped spy');
 
   ngAfterViewInit() {
diff --git a/src/cdk/drag-drop/drag-ref.ts b/src/cdk/drag-drop/drag-ref.ts
@@ -704,7 +704,8 @@ export class DragRef<T = any> {
    */
   private _updateActiveDropContainer({x, y}: Point) {
     // Drop container that draggable has been moved into.
-    let newContainer = this.dropContainer!._getSiblingContainerFromPosition(this, x, y);
+    let newContainer = this.dropContainer!._getSiblingContainerFromPosition(this, x, y) ||
+        this._initialContainer._getSiblingContainerFromPosition(this, x, y);
 
     // If we couldn't find a new container to move the item into, and the item has left it's
     // initial container, check whether the it's over the initial container. This handles the
@@ -715,7 +716,7 @@ export class DragRef<T = any> {
       newContainer = this._initialContainer;
     }
 
-    if (newContainer) {
+    if (newContainer && newContainer !== this.dropContainer) {
       this._ngZone.run(() => {
         // Notify the old container that the item has left.
         this.exited.next({item: this, container: this.dropContainer!});
diff --git a/src/cdk/drag-drop/drop-list-ref.ts b/src/cdk/drag-drop/drop-list-ref.ts

-Original file line number
+Diff line change
  */
 const DROP_PROXIMITY_THRESHOLD = 0.05;
 -/**
 - * Object used to cache the position of a drag list, its items. and siblings.
 - * @docs-private
 - */
 -interface PositionCache {
 -  /** Cached positions of the items in the list. */
 -  items: ItemPositionCacheEntry[];
 -  /** Cached positions of the connected lists. */
 -  siblings: ListPositionCacheEntry[];
 -  /** Dimensions of the list itself. */
 -  self: ClientRect;
 -}
+-
 /**
  * Entry in the position cache for draggable items.
  * @docs-private
  */
 -interface ItemPositionCacheEntry {
 +interface CachedItemPosition {
   /** Instance of the drag item. */
   drag: DragRef;
   /** Dimensions of the item. */
   offset: number;
+}
 -/**
 - * Entry in the position cache for drop lists.
 - * @docs-private
 - */
 -interface ListPositionCacheEntry {
 -  /** Instance of the drop list. */
 -  drop: DropListRef;
 -  /** Dimensions of the list. */
 -  clientRect: ClientRect;
 -}
+-
 /**
  * Internal compile-time-only representation of a `DropListRef`.
  * Used to avoid circular import issues between the `DropListRef` and the `DragRef`.
   /** Whether an item in the list is being dragged. */
   private _isDragging = false;
 -  /** Cache of the dimensions of all the items and the sibling containers. */
 -  private _positionCache: PositionCache = {items: [], siblings: [], self: {} as ClientRect};
 +  /** Cache of the dimensions of all the items inside the container. */
 +  private _itemPositions: CachedItemPosition[] = [];
++
 +  /** Cached `ClientRect` of the drop list. */
 +  private _clientRect: ClientRect;
   /**
    * Draggable items that are currently active inside the container. Includes the items
   /** Draggable items in the container. */
   private _draggables: DragRef[];
 +  /** Drop lists that are connected to the current one. */
   private _siblings: DropListRef[] = [];
   /** Direction in which the list is oriented. */
   private _orientation: 'horizontal' | 'vertical' = 'vertical';
 -  /** Amount of connected siblings that currently have a dragged item. */
 -  private _activeSiblings = 0;
 +  /** Connected siblings that currently have a dragged item. */
 +  private _activeSiblings = new Set<DropListRef>();
   constructor(
     public element: ElementRef<HTMLElement>,
     this.exited.complete();
     this.dropped.complete();
     this.sorted.complete();
 +    this._activeSiblings.clear();
     this._dragDropRegistry.removeDropContainer(this);
+  }
     this.beforeStarted.next();
     this._isDragging = true;
     this._activeDraggables = this._draggables.slice();
 -    this._cachePositions();
 -    this._positionCache.siblings.forEach(sibling => sibling.drop._toggleIsReceiving(true));
 +    this._cacheOwnPosition();
 +    this._cacheItemPositions();
 +    this._siblings.forEach(sibling => sibling._startReceiving(this));
+  }
   /**
     // Note that the positions were already cached when we called `start` above,
     // but we need to refresh them since the amount of items has changed.
 -    this._cachePositions();
 +    this._cacheItemPositions();
+  }
   /**
     // The rest of the logic still stands no matter what orientation we're in, however
     // we need to invert the array when determining the index.
     const items = this._orientation === 'horizontal' && this._dir && this._dir.value === 'rtl' ?
 -        this._positionCache.items.slice().reverse() : this._positionCache.items;
 +        this._itemPositions.slice().reverse() : this._itemPositions;
     return findIndex(items, currentItem => currentItem.drag === item);
+  }
    * is currently being dragged inside a connected drop list.
    */
   isReceiving(): boolean {
 -    return this._activeSiblings > 0;
 +    return this._activeSiblings.size > 0;
+  }
   /**
       return;
+    }
 -    const siblings = this._positionCache.items;
 +    const siblings = this._itemPositions;
     const newIndex = this._getItemIndexFromPointerPosition(item, pointerX, pointerY, pointerDelta);
     if (newIndex === -1 && siblings.length > 0) {
     });
+  }
 +  /** Caches the position of the drop list. */
 +  private _cacheOwnPosition() {
 +    this._clientRect = this.element.nativeElement.getBoundingClientRect();
 +  }
++
   /** Refreshes the position cache of the items and sibling containers. */
 -  private _cachePositions() {
 +  private _cacheItemPositions() {
     const isHorizontal = this._orientation === 'horizontal';
 -    this._positionCache.self = this.element.nativeElement.getBoundingClientRect();
 -    this._positionCache.items = 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 clientRect = elementToMeasure.getBoundingClientRect();
+-
 -        return {
 -          drag,
 -          offset: 0,
 -          // We need to clone the `clientRect` here, because all the values on it are readonly
 -          // and we need to be able to update them. Also we can't use a spread here, because
 -          // the values on a `ClientRect` aren't own properties. See:
 -          // https://developer.mozilla.org/en-US/docs/Web/API/Element/getBoundingClientRect#Notes
 -          clientRect: {
 -            top: clientRect.top,
 -            right: clientRect.right,
 -            bottom: clientRect.bottom,
 -            left: clientRect.left,
 -            width: clientRect.width,
 -            height: clientRect.height
 -          }
 -        };
 -      })
 -      .sort((a, b) => {
 -        return isHorizontal ? a.clientRect.left - b.clientRect.left :
 -                              a.clientRect.top - b.clientRect.top;
 -      });
+-
 -    this._positionCache.siblings = this._siblings.map(drop => ({
 -      drop,
 -      clientRect: drop.element.nativeElement.getBoundingClientRect()
 -    }));
 -  }
+-
 -  /**
 -   * Toggles whether the list can receive the item that is currently being dragged.
 -   * Usually called by a sibling that initiated the dragging.
 -   */
 -  _toggleIsReceiving(isDragging: boolean) {
 -    this._activeSiblings = Math.max(0, this._activeSiblings + (isDragging ? 1 : -1));
 +    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 clientRect = elementToMeasure.getBoundingClientRect();
++
 +      return {
 +        drag,
 +        offset: 0,
 +        // We need to clone the `clientRect` here, because all the values on it are readonly
 +        // and we need to be able to update them. Also we can't use a spread here, because
 +        // the values on a `ClientRect` aren't own properties. See:
 +        // https://developer.mozilla.org/en-US/docs/Web/API/Element/getBoundingClientRect#Notes
 +        clientRect: {
 +          top: clientRect.top,
 +          right: clientRect.right,
 +          bottom: clientRect.bottom,
 +          left: clientRect.left,
 +          width: clientRect.width,
 +          height: clientRect.height
 +        }
 +      };
 +    }).sort((a, b) => {
 +      return isHorizontal ? a.clientRect.left - b.clientRect.left :
 +                            a.clientRect.top - b.clientRect.top;
 +    });
+  }
   /** Resets the container to its initial state. */
     // TODO(crisbeto): may have to wait for the animations to finish.
     this._activeDraggables.forEach(item => item.getRootElement().style.transform = '');
 -    this._positionCache.siblings.forEach(sibling => sibling.drop._toggleIsReceiving(false));
 +    this._siblings.forEach(sibling => sibling._stopReceiving(this));
     this._activeDraggables = [];
 -    this._positionCache.items = [];
 -    this._positionCache.siblings = [];
 +    this._itemPositions = [];
     this._previousSwap.drag = null;
     this._previousSwap.delta = 0;
+  }
    * @param delta Direction in which the user is moving.
    */
   private _getSiblingOffsetPx(currentIndex: number,
 -                              siblings: ItemPositionCacheEntry[],
 +                              siblings: CachedItemPosition[],
                               delta: 1 | -1) {
     const isHorizontal = this._orientation === 'horizontal';
    * @param pointerY Coordinates along the Y axis.
    */
   private _isPointerNearDropContainer(pointerX: number, pointerY: number): boolean {
 -    const {top, right, bottom, left, width, height} = this._positionCache.self;
 +    const {top, right, bottom, left, width, height} = this._clientRect;
     const xThreshold = width * DROP_PROXIMITY_THRESHOLD;
     const yThreshold = height * DROP_PROXIMITY_THRESHOLD;
    */
   private _getItemIndexFromPointerPosition(item: DragRef, pointerX: number, pointerY: number,
                                            delta?: {x: number, y: number}) {
+-
     const isHorizontal = this._orientation === 'horizontal';
 -    return findIndex(this._positionCache.items, ({drag, clientRect}, _, array) => {
 +    return findIndex(this._itemPositions, ({drag, clientRect}, _, array) => {
       if (drag === item) {
         // If there's only one item left in the container, it must be
         // the dragged item itself so we use it as a reference.
    * @param y Pointer position along the Y axis.
    */
   _isOverContainer(x: number, y: number): boolean {
 -    return isInsideClientRect(this._positionCache.self, x, y);
 +    return isInsideClientRect(this._clientRect, x, y);
+  }
   /**
    * @param x Position of the item along the X axis.
    * @param y Position of the item along the Y axis.
    */
 -  _getSiblingContainerFromPosition(item: DragRef, x: number, y: number): DropListRef | null {
 -    const results = this._positionCache.siblings.filter(sibling => {
 -      return isInsideClientRect(sibling.clientRect, x, y);
 -    });
 +  _getSiblingContainerFromPosition(item: DragRef, x: number, y: number): DropListRef | undefined {
 +    return this._siblings.find(sibling => sibling._canReceive(item, x, y));
 +  }
 -    // No drop containers are intersecting with the pointer.
 -    if (!results.length) {
 -      return null;
 +  /**
 +   * Checks whether the drop list can receive the passed-in item.
 +   * @param item Item that is being dragged into the list.
 +   * @param x Position of the item along the X axis.
 +   * @param y Position of the item along the Y axis.
 +   */
 +  _canReceive(item: DragRef, x: number, y: number): boolean {
 +    if (!this.enterPredicate(item, this) || !isInsideClientRect(this._clientRect, x, y)) {
 +      return false;
+    }
     const elementFromPoint = this._document.elementFromPoint(x, y);
     // If there's no element at the pointer position, then
     // the client rect is probably scrolled out of the view.
     if (!elementFromPoint) {
 -      return null;
 +      return false;
+    }
 +    const element = this.element.nativeElement;
++
     // The `ClientRect`, that we're using to find the container over which the user is
     // hovering, doesn't give us any information on whether the element has been scrolled
     // out of the view or whether it's overlapping with other containers. This means that
     // we could end up transferring the item into a container that's invisible or is positioned
     // below another one. We use the result from `elementFromPoint` to get the top-most element
     // at the pointer position and to find whether it's one of the intersecting drop containers.
 -    const result = results.find(sibling => {
 -      const element = sibling.drop.element.nativeElement;
 -      return element === elementFromPoint || element.contains(elementFromPoint);
 -    });
 +    return elementFromPoint === element || element.contains(elementFromPoint);
 +  }
++
 +  /**
 +   * Called by one of the connected drop lists when a dragging sequence has started.
 +   * @param sibling Sibling in which dragging has started.
 +   */
 +  _startReceiving(sibling: DropListRef) {
 +    const activeSiblings = this._activeSiblings;
 -    return result && result.drop.enterPredicate(item, result.drop) ? result.drop : null;
 +    if (!activeSiblings.has(sibling)) {
 +      activeSiblings.add(sibling);
 +      this._cacheOwnPosition();
 +    }
+  }
 +  /**
 +   * Called by a connected drop list when dragging has stopped.
 +   * @param sibling Sibling whose dragging has stopped.
 +   */
 +  _stopReceiving(sibling: DropListRef) {
 +    this._activeSiblings.delete(sibling);
 +  }
+}