fix(cdk/drag-drop): account for enterPredicate when setting receiving class (#21346)

Currently we set a class when a container is able to receive the dragged item,
however we add this class to all connected containers without accounting
for other things that could prevent the item from entering, like `enterPredicate`.

These changes add some logic to account for the predicate when setting the class.

Fixes #21171.

Author: crisbeto

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

@@ -4483,33 +4483,6 @@ describe('CdkDrag', () => {
         expect(spy).toHaveBeenCalledWith(dragItem, dropInstances[1]);
       }));
 
-    it('should not call the `enterPredicate` if the pointer is not over the container',
-      fakeAsync(() => {
-        const fixture = createComponent(ConnectedDropZones);
-        fixture.detectChanges();
-
-        const dropInstances = fixture.componentInstance.dropInstances.toArray();
-        const spy = jasmine.createSpy('enterPredicate spy').and.returnValue(true);
-        const groups = fixture.componentInstance.groupedDragItems.slice();
-        const dragElement = groups[0][1].element.nativeElement;
-        const targetRect = groups[1][2].element.nativeElement.getBoundingClientRect();
-
-        dropInstances[1].enterPredicate = spy;
-        fixture.detectChanges();
-
-        startDraggingViaMouse(fixture, dragElement);
-
-        dispatchMouseEvent(document, 'mousemove', targetRect.left - 1, targetRect.top - 1);
-        fixture.detectChanges();
-
-        expect(spy).not.toHaveBeenCalled();
-
-        dispatchMouseEvent(document, 'mousemove', targetRect.left + 1, targetRect.top + 1);
-        fixture.detectChanges();
-
-        expect(spy).toHaveBeenCalledTimes(1);
-      }));
-
     it('should be able to start dragging after an item has been transferred', fakeAsync(() => {
       const fixture = createComponent(ConnectedDropZones);
       fixture.detectChanges();
@@ -5009,14 +4982,75 @@ describe('CdkDrag', () => {
          expect(dropZones[0].classList)
              .toContain(
                  'cdk-drop-list-receiving',
-                 'Expected old container not to have the receiving class after exiting.');
+                 'Expected old container to have the receiving class after exiting.');
 
          expect(dropZones[1].classList)
              .not.toContain(
                  'cdk-drop-list-receiving',
-                 'Expected new container not to have the receiving class after entering.');
+                 'Expected new container not to have the receiving class after exiting.');
        }));
 
+    it('should not set the receiving class if the item does not match the enter predicate',
+      fakeAsync(() => {
+        const fixture = createComponent(ConnectedDropZones);
+        fixture.detectChanges();
+        fixture.componentInstance.dropInstances.toArray()[1].enterPredicate = () => false;
+
+        const dropZones =
+            fixture.componentInstance.dropInstances.map(d => d.element.nativeElement);
+        const item = fixture.componentInstance.groupedDragItems[0][1];
+
+        expect(dropZones.every(c => !c.classList.contains('cdk-drop-list-receiving')))
+            .toBe(true, 'Expected neither of the containers to have the class.');
+
+        startDraggingViaMouse(fixture, item.element.nativeElement);
+        fixture.detectChanges();
+
+        expect(dropZones.every(c => !c.classList.contains('cdk-drop-list-receiving')))
+            .toBe(true, 'Expected neither of the containers to have the class.');
+      }));
+
+    it('should set the receiving class on the source container, even if the enter predicate ' +
+      'does not match', fakeAsync(() => {
+        const fixture = createComponent(ConnectedDropZones);
+        fixture.detectChanges();
+        fixture.componentInstance.dropInstances.toArray()[0].enterPredicate = () => false;
+
+        const groups = fixture.componentInstance.groupedDragItems;
+        const dropZones =
+            fixture.componentInstance.dropInstances.map(d => d.element.nativeElement);
+        const item = groups[0][1];
+        const targetRect = groups[1][2].element.nativeElement.getBoundingClientRect();
+
+        expect(dropZones.every(c => !c.classList.contains('cdk-drop-list-receiving')))
+            .toBe(true, 'Expected neither of the containers to have the class.');
+
+        startDraggingViaMouse(fixture, item.element.nativeElement);
+
+        expect(dropZones[0].classList)
+            .not.toContain(
+                'cdk-drop-list-receiving',
+                'Expected source container not to have the receiving class.');
+
+        expect(dropZones[1].classList)
+            .toContain(
+                'cdk-drop-list-receiving',
+                'Expected target container to have the receiving class.');
+
+        dispatchMouseEvent(document, 'mousemove', targetRect.left + 1, targetRect.top + 1);
+        fixture.detectChanges();
+
+        expect(dropZones[0].classList)
+            .toContain(
+                'cdk-drop-list-receiving',
+                'Expected old container to have the receiving class after exiting.');
+
+        expect(dropZones[1].classList)
+            .not.toContain(
+                'cdk-drop-list-receiving',
+                'Expected new container not to have the receiving class after exiting.');
+      }));
+
     it('should be able to move the item over an intermediate container before ' +
       'dropping it into the final one', fakeAsync(() => {
         const fixture = createComponent(ConnectedDropZones);

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

@@ -146,7 +146,7 @@ export class DropListRef<T = any> {
   private _parentPositions: ParentPositionTracker;
 
   /** Cached `ClientRect` of the drop list. */
-  private _clientRect: ClientRect;
+  private _clientRect: ClientRect | undefined;
 
   /**
    * Draggable items that are currently active inside the container. Includes the items
@@ -163,7 +163,7 @@ export class DropListRef<T = any> {
   private _previousSwap = {drag: null as DragRef | null, delta: 0, overlaps: false};
 
   /** Draggable items in the container. */
-  private _draggables: ReadonlyArray<DragRef>;
+  private _draggables: ReadonlyArray<DragRef> = [];
 
   /** Drop lists that are connected to the current one. */
   private _siblings: ReadonlyArray<DropListRef> = [];
@@ -240,19 +240,8 @@ export class DropListRef<T = any> {
 
   /** Starts dragging an item. */
   start(): void {
-    const styles = coerceElement(this.element).style as DragCSSStyleDeclaration;
-    this.beforeStarted.next();
-    this._isDragging = true;
-
-    // We need to disable scroll snapping while the user is dragging, because it breaks automatic
-    // scrolling. The browser seems to round the value based on the snapping points which means
-    // that we can't increment/decrement the scroll position.
-    this._initialScrollSnap = styles.msScrollSnapType || styles.scrollSnapType || '';
-    styles.scrollSnapType = styles.msScrollSnapType = 'none';
-    this._cacheItems();
-    this._siblings.forEach(sibling => sibling._startReceiving(this));
-    this._viewportScrollSubscription.unsubscribe();
-    this._listenToScrollEvents();
+    this._draggingStarted();
+    this._notifyReceivingSiblings();
   }
 
   /**
@@ -264,7 +253,7 @@ export class DropListRef<T = any> {
    *   out automatically.
    */
   enter(item: DragRef, pointerX: number, pointerY: number, index?: number): void {
-    this.start();
+    this._draggingStarted();
 
     // If sorting is disabled, we want the item to return to its starting
     // position if the user is returning it to its initial container.
@@ -323,6 +312,8 @@ export class DropListRef<T = any> {
     this._cacheItemPositions();
     this._cacheParentPositions();
 
+    // Notify siblings at the end so that the item has been inserted into the `activeDraggables`.
+    this._notifyReceivingSiblings();
     this.entered.next({item, container: this, currentIndex: this.getItemIndex(item)});
   }
 
@@ -463,7 +454,7 @@ export class DropListRef<T = any> {
   _sortItem(item: DragRef, pointerX: number, pointerY: number,
             pointerDelta: {x: number, y: number}): void {
     // Don't sort the item if sorting is disabled or it's out of range.
-    if (this.sortingDisabled ||
+    if (this.sortingDisabled || !this._clientRect ||
         !isPointerNearClientRect(this._clientRect, DROP_PROXIMITY_THRESHOLD, pointerX, pointerY)) {
       return;
     }
@@ -600,6 +591,22 @@ export class DropListRef<T = any> {
     this._stopScrollTimers.next();
   }
 
+  /** Starts the dragging sequence within the list. */
+  private _draggingStarted() {
+    const styles = coerceElement(this.element).style as DragCSSStyleDeclaration;
+    this.beforeStarted.next();
+    this._isDragging = true;
+
+    // We need to disable scroll snapping while the user is dragging, because it breaks automatic
+    // scrolling. The browser seems to round the value based on the snapping points which means
+    // that we can't increment/decrement the scroll position.
+    this._initialScrollSnap = styles.msScrollSnapType || styles.scrollSnapType || '';
+    styles.scrollSnapType = styles.msScrollSnapType = 'none';
+    this._cacheItems();
+    this._viewportScrollSubscription.unsubscribe();
+    this._listenToScrollEvents();
+  }
+
   /** Caches the positions of the configured scrollable parents. */
   private _cacheParentPositions() {
     const element = coerceElement(this.element);
@@ -802,7 +809,7 @@ export class DropListRef<T = any> {
    * @param y Pointer position along the Y axis.
    */
   _isOverContainer(x: number, y: number): boolean {
-    return isInsideClientRect(this._clientRect, x, y);
+    return this._clientRect != null && isInsideClientRect(this._clientRect, x, y);
   }
 
   /**
@@ -823,7 +830,8 @@ export class DropListRef<T = any> {
    * @param y Position of the item along the Y axis.
    */
   _canReceive(item: DragRef, x: number, y: number): boolean {
-    if (!isInsideClientRect(this._clientRect, x, y) || !this.enterPredicate(item, this)) {
+    if (!this._clientRect || !isInsideClientRect(this._clientRect, x, y) ||
+        !this.enterPredicate(item, this)) {
       return false;
     }
 
@@ -850,10 +858,16 @@ export class DropListRef<T = any> {
    * 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) {
+  _startReceiving(sibling: DropListRef, items: DragRef[]) {
     const activeSiblings = this._activeSiblings;
 
-    if (!activeSiblings.has(sibling)) {
+    if (!activeSiblings.has(sibling) && items.every(item => {
+      // Note that we have to add an exception to the `enterPredicate` for items that started off
+      // in this drop list. The drag ref has logic that allows an item to return to its initial
+      // container, if it has left the initial container and none of the connected containers
+      // allow it to enter. See `DragRef._updateActiveDropContainer` for more context.
+      return this.enterPredicate(item, this) || this._draggables.indexOf(item) > -1;
+    })) {
       activeSiblings.add(sibling);
       this._cacheParentPositions();
       this._listenToScrollEvents();
@@ -917,6 +931,12 @@ export class DropListRef<T = any> {
 
     return this._cachedShadowRoot;
   }
+
+  /** Notifies any siblings that may potentially receive the item. */
+  private _notifyReceivingSiblings() {
+    const draggedItems = this._activeDraggables.filter(item => item.isDragging());
+    this._siblings.forEach(sibling => sibling._startReceiving(this, draggedItems));
+  }
 }
 
 

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

@@ -374,7 +374,7 @@ export declare class DropListRef<T = any> {
         x: number;
         y: number;
     }): void;
-    _startReceiving(sibling: DropListRef): void;
+    _startReceiving(sibling: DropListRef, items: DragRef[]): void;
     _startScrollingIfNecessary(pointerX: number, pointerY: number): void;
     _stopReceiving(sibling: DropListRef): void;
     _stopScrolling(): void;