fix(drag-drop): boundary not accounting for scrolling (#18612)

When the user starts dragging, we cache the `ClientRect` of the boundary so we know the area around which to limit the preview. The problem is that we weren't updating the cached position which cause it to behave incorrectly if the user scrolled after they start dragging.

These changes fix the issue by updating cached position. This ended up slightly more complicated than expected, because it has to interact with the auto scrolling.

I've also moved around some utilities for dealing with `ClientRect` so that they can be reused.

Fixes #18597.

Author: crisbeto

src/cdk/drag-drop/client-rect.ts

@@ -0,0 +1,69 @@
+/**
+ * @license
+ * Copyright Google LLC All Rights Reserved.
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.io/license
+ */
+
+/** Gets a mutable version of an element's bounding `ClientRect`. */
+export function getMutableClientRect(element: Element): ClientRect {
+  const clientRect = element.getBoundingClientRect();
+
+  // 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
+  return {
+    top: clientRect.top,
+    right: clientRect.right,
+    bottom: clientRect.bottom,
+    left: clientRect.left,
+    width: clientRect.width,
+    height: clientRect.height
+  };
+}
+
+/**
+ * Checks whether some coordinates are within a `ClientRect`.
+ * @param clientRect ClientRect that is being checked.
+ * @param x Coordinates along the X axis.
+ * @param y Coordinates along the Y axis.
+ */
+export function isInsideClientRect(clientRect: ClientRect, x: number, y: number) {
+  const {top, bottom, left, right} = clientRect;
+  return y >= top && y <= bottom && x >= left && x <= right;
+}
+
+/**
+ * Updates the top/left positions of a `ClientRect`, as well as their bottom/right counterparts.
+ * @param clientRect `ClientRect` that should be updated.
+ * @param top Amount to add to the `top` position.
+ * @param left Amount to add to the `left` position.
+ */
+export function adjustClientRect(clientRect: ClientRect, top: number, left: number) {
+  clientRect.top += top;
+  clientRect.bottom = clientRect.top + clientRect.height;
+
+  clientRect.left += left;
+  clientRect.right = clientRect.left + clientRect.width;
+}
+
+/**
+ * Checks whether the pointer coordinates are close to a ClientRect.
+ * @param rect ClientRect to check against.
+ * @param threshold Threshold around the ClientRect.
+ * @param pointerX Coordinates along the X axis.
+ * @param pointerY Coordinates along the Y axis.
+ */
+export function isPointerNearClientRect(rect: ClientRect,
+                                        threshold: number,
+                                        pointerX: number,
+                                        pointerY: number): boolean {
+  const {top, right, bottom, left, width, height} = rect;
+  const xThreshold = width * threshold;
+  const yThreshold = height * threshold;
+
+  return pointerY > top - yThreshold && pointerY < bottom + yThreshold &&
+         pointerX > left - xThreshold && pointerX < right + xThreshold;
+}

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

@@ -2027,6 +2027,39 @@ describe('CdkDrag', () => {
       expect(Math.floor(previewRect.right)).toBe(Math.floor(listRect.right));
     }));
 
+    it('should update the boundary if the page is scrolled while dragging', fakeAsync(() => {
+      const fixture = createComponent(DraggableInDropZone);
+      fixture.componentInstance.boundarySelector = '.cdk-drop-list';
+      fixture.detectChanges();
+
+      const item = fixture.componentInstance.dragItems.toArray()[1].element.nativeElement;
+      const list = fixture.componentInstance.dropInstance.element.nativeElement;
+      const cleanup = makePageScrollable();
+      scrollTo(0, 10);
+      let listRect = list.getBoundingClientRect(); // Note that we need to measure after scrolling.
+
+      startDraggingViaMouse(fixture, item);
+      startDraggingViaMouse(fixture, item, listRect.right, listRect.bottom);
+      flush();
+      dispatchMouseEvent(document, 'mousemove', listRect.right, listRect.bottom);
+      fixture.detectChanges();
+
+      const preview = document.querySelector('.cdk-drag-preview')! as HTMLElement;
+      let previewRect = preview.getBoundingClientRect();
+      expect(Math.floor(previewRect.bottom)).toBe(Math.floor(listRect.bottom));
+
+      scrollTo(0, 0);
+      dispatchFakeEvent(document, 'scroll');
+      fixture.detectChanges();
+      listRect = list.getBoundingClientRect(); // We need to update these since we've scrolled.
+      dispatchMouseEvent(document, 'mousemove', listRect.right, listRect.bottom);
+      fixture.detectChanges();
+      previewRect = preview.getBoundingClientRect();
+
+      expect(Math.floor(previewRect.bottom)).toBe(Math.floor(listRect.bottom));
+      cleanup();
+    }));
+
     it('should clear the id from the preview', fakeAsync(() => {
       const fixture = createComponent(DraggableInDropZone);
       fixture.detectChanges();

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

@@ -17,6 +17,7 @@ import {DropListRefInternal as DropListRef} from './drop-list-ref';
 import {DragDropRegistry} from './drag-drop-registry';
 import {extendStyles, toggleNativeDragInteractions} from './drag-styling';
 import {getTransformTransitionDurationInMs} from './transition-duration';
+import {getMutableClientRect, adjustClientRect} from './client-rect';
 
 /** Object that can be used to configure the behavior of DragRef. */
 export interface DragRefConfig {
@@ -495,7 +496,7 @@ export class DragRef<T = any> {
     const position = this._pointerPositionAtLastDirectionChange;
 
     if (position && this._dropContainer) {
-      this._updateActiveDropContainer(position);
+      this._updateActiveDropContainer(this._getConstrainedPointerPosition(position));
     }
   }
 
@@ -556,9 +557,9 @@ export class DragRef<T = any> {
     // Prevent the default action as early as possible in order to block
     // native actions like dragging the selected text or images with the mouse.
     event.preventDefault();
+    const pointerPosition = this._getPointerPositionOnPage(event);
 
     if (!this._hasStartedDragging) {
-      const pointerPosition = this._getPointerPositionOnPage(event);
       const distanceX = Math.abs(pointerPosition.x - this._pickupPositionOnPage.x);
       const distanceY = Math.abs(pointerPosition.y - this._pickupPositionOnPage.y);
       const isOverThreshold = distanceX + distanceY >= this._config.dragStartThreshold;
@@ -595,7 +596,7 @@ export class DragRef<T = any> {
       }
     }
 
-    const constrainedPointerPosition = this._getConstrainedPointerPosition(event);
+    const constrainedPointerPosition = this._getConstrainedPointerPosition(pointerPosition);
     this._hasMoved = true;
     this._updatePointerDirectionDelta(constrainedPointerPosition);
 
@@ -775,11 +776,11 @@ export class DragRef<T = any> {
     this._pointerMoveSubscription = this._dragDropRegistry.pointerMove.subscribe(this._pointerMove);
     this._pointerUpSubscription = this._dragDropRegistry.pointerUp.subscribe(this._pointerUp);
     this._scrollSubscription = this._dragDropRegistry.scroll.pipe(startWith(null)).subscribe(() => {
-      this._scrollPosition = this._viewportRuler.getViewportScrollPosition();
+      this._updateOnScroll();
     });
 
     if (this._boundaryElement) {
-      this._boundaryRect = this._boundaryElement.getBoundingClientRect();
+      this._boundaryRect = getMutableClientRect(this._boundaryElement);
     }
 
     // If we have a custom preview we can't know ahead of time how large it'll be so we position
@@ -1033,8 +1034,7 @@ export class DragRef<T = any> {
 
 
   /** Gets the pointer position on the page, accounting for any position constraints. */
-  private _getConstrainedPointerPosition(event: MouseEvent | TouchEvent): Point {
-    const point = this._getPointerPositionOnPage(event);
+  private _getConstrainedPointerPosition(point: Point): Point {
     const constrainedPoint = this.constrainPosition ? this.constrainPosition(point, this) : point;
     const dropContainerLock = this._dropContainer ? this._dropContainer.lockAxis : null;
 
@@ -1219,6 +1219,22 @@ export class DragRef<T = any> {
 
     return value ? value.mouse : 0;
   }
+
+  /** Updates the internal state of the draggable element when scrolling has occurred. */
+  private _updateOnScroll() {
+    const oldScrollPosition = this._scrollPosition;
+    const currentScrollPosition = this._viewportRuler.getViewportScrollPosition();
+
+    // ClientRect dimensions are based on the page's scroll position so
+    // we have to update the cached boundary ClientRect if the user has scrolled.
+    if (oldScrollPosition && this._boundaryRect) {
+      const topDifference = oldScrollPosition.top - currentScrollPosition.top;
+      const leftDifference = oldScrollPosition.left - currentScrollPosition.left;
+      adjustClientRect(this._boundaryRect, topDifference, leftDifference);
+    }
+
+    this._scrollPosition = currentScrollPosition;
+  }
 }
 
 /**

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

@@ -16,6 +16,12 @@ import {takeUntil} from 'rxjs/operators';
 import {moveItemInArray} from './drag-utils';
 import {DragDropRegistry} from './drag-drop-registry';
 import {DragRefInternal as DragRef, Point} from './drag-ref';
+import {
+  isPointerNearClientRect,
+  adjustClientRect,
+  getMutableClientRect,
+  isInsideClientRect,
+} from './client-rect';
 
 /**
  * Proximity, as a ratio to width/height, at which a
@@ -446,7 +452,8 @@ 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 || !isPointerNearClientRect(this._clientRect, pointerX, pointerY)) {
+    if (this.sortingDisabled ||
+        !isPointerNearClientRect(this._clientRect, DROP_PROXIMITY_THRESHOLD, pointerX, pointerY)) {
       return;
     }
 
@@ -540,7 +547,8 @@ export class DropListRef<T = any> {
         return;
       }
 
-      if (isPointerNearClientRect(position.clientRect, pointerX, pointerY)) {
+      if (isPointerNearClientRect(position.clientRect, DROP_PROXIMITY_THRESHOLD,
+          pointerX, pointerY)) {
         [verticalScrollDirection, horizontalScrollDirection] = getElementScrollDirections(
             element as HTMLElement, position.clientRect, pointerX, pointerY);
 
@@ -921,35 +929,6 @@ export class DropListRef<T = any> {
 }
 
 
-/**
- * Updates the top/left positions of a `ClientRect`, as well as their bottom/right counterparts.
- * @param clientRect `ClientRect` that should be updated.
- * @param top Amount to add to the `top` position.
- * @param left Amount to add to the `left` position.
- */
-function adjustClientRect(clientRect: ClientRect, top: number, left: number) {
-  clientRect.top += top;
-  clientRect.bottom = clientRect.top + clientRect.height;
-
-  clientRect.left += left;
-  clientRect.right = clientRect.left + clientRect.width;
-}
-
-/**
- * Checks whether the pointer coordinates are close to a ClientRect.
- * @param rect ClientRect to check against.
- * @param pointerX Coordinates along the X axis.
- * @param pointerY Coordinates along the Y axis.
- */
-function isPointerNearClientRect(rect: ClientRect, pointerX: number, pointerY: number): boolean {
-  const {top, right, bottom, left, width, height} = rect;
-  const xThreshold = width * DROP_PROXIMITY_THRESHOLD;
-  const yThreshold = height * DROP_PROXIMITY_THRESHOLD;
-
-  return pointerY > top - yThreshold && pointerY < bottom + yThreshold &&
-         pointerX > left - xThreshold && pointerX < right + xThreshold;
-}
-
 /**
  * Finds the index of an item that matches a predicate function. Used as an equivalent
  * of `Array.prototype.findIndex` which isn't part of the standard Google typings.
@@ -968,37 +947,6 @@ function findIndex<T>(array: T[],
   return -1;
 }
 
-
-/**
- * Checks whether some coordinates are within a `ClientRect`.
- * @param clientRect ClientRect that is being checked.
- * @param x Coordinates along the X axis.
- * @param y Coordinates along the Y axis.
- */
-function isInsideClientRect(clientRect: ClientRect, x: number, y: number) {
-  const {top, bottom, left, right} = clientRect;
-  return y >= top && y <= bottom && x >= left && x <= right;
-}
-
-
-/** Gets a mutable version of an element's bounding `ClientRect`. */
-function getMutableClientRect(element: Element): ClientRect {
-  const clientRect = element.getBoundingClientRect();
-
-  // 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
-  return {
-    top: clientRect.top,
-    right: clientRect.right,
-    bottom: clientRect.bottom,
-    left: clientRect.left,
-    width: clientRect.width,
-    height: clientRect.height
-  };
-}
-
 /**
  * Increments the vertical scroll position of a node.
  * @param node Node whose scroll position should change.