fix(drag-drop): handle list and viewport auto scroll regions overlapping (#16675)

The way auto-scrolling is currently set up is that we have regions that are X% of the width/height of a node and whenever the user's pointer gets into the region, we start scrolling either the element or the page. The problem with our current approach is that as soon we find that the user's pointer is in one node's scroll region, we disregard any of the other scroll regions and we start scrolling, even if the element can't actually scroll. This is problematic in cases where the scroll list might be close to the viewport's edge and thus it could have its region overlapping with the viewport's. These changes address the issue in two ways:

1. We have the list's scroll regions take precedence over the ones of the page.
2. If the regions overlap, we check whether the element can continue scrolling that direction and if it can't, we start scrolling the page.

Fixes #16647.

Author: crisbeto

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

@@ -3081,9 +3081,47 @@ describe('CdkDrag', () => {
       cleanup();
     }));
 
-    it('should auto-scroll the viewport, not the list, when the pointer is over the edge of ' +
+    it('should auto-scroll the list, not the viewport, when the pointer is over the edge of ' +
       'both the list and the viewport', fakeAsync(() => {
-        const fixture = createComponent(DraggableInDropZone);
+        const fixture = createComponent(DraggableInScrollableVerticalDropZone);
+        fixture.detectChanges();
+
+        const list = fixture.componentInstance.dropInstance.element.nativeElement;
+        const viewportRuler: ViewportRuler = TestBed.get(ViewportRuler);
+        const item = fixture.componentInstance.dragItems.first.element.nativeElement;
+
+        // Position the list so that its top aligns with the viewport top. That way the pointer
+        // will both over its top edge and the viewport's. We use top instead of bottom, because
+        // bottom behaves weirdly when we run tests on mobile devices.
+        list.style.position = 'fixed';
+        list.style.left = '50%';
+        list.style.top = '0';
+        list.style.margin = '0';
+
+        const listRect = list.getBoundingClientRect();
+        const cleanup = makePageScrollable();
+
+        scrollTo(0, viewportRuler.getViewportSize().height * 5);
+        list.scrollTop = 50;
+
+        const initialScrollDistance = viewportRuler.getViewportScrollPosition().top;
+        expect(initialScrollDistance).toBeGreaterThan(0);
+        expect(list.scrollTop).toBe(50);
+
+        startDraggingViaMouse(fixture, item);
+        dispatchMouseEvent(document, 'mousemove', listRect.left + listRect.width / 2, 0);
+        fixture.detectChanges();
+        tickAnimationFrames(20);
+
+        expect(viewportRuler.getViewportScrollPosition().top).toBe(initialScrollDistance);
+        expect(list.scrollTop).toBeLessThan(50);
+
+        cleanup();
+      }));
+
+    it('should auto-scroll the viewport, when the pointer is over the edge of both the list ' +
+      'and the viewport, if the list cannot be scrolled in that direction', fakeAsync(() => {
+        const fixture = createComponent(DraggableInScrollableVerticalDropZone);
         fixture.detectChanges();
 
         const list = fixture.componentInstance.dropInstance.element.nativeElement;
@@ -3102,6 +3140,7 @@ describe('CdkDrag', () => {
         const cleanup = makePageScrollable();
 
         scrollTo(0, viewportRuler.getViewportSize().height * 5);
+        list.scrollTop = 0;
 
         const initialScrollDistance = viewportRuler.getViewportScrollPosition().top;
         expect(initialScrollDistance).toBeGreaterThan(0);

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

@@ -510,31 +510,28 @@ export class DropListRef<T = any> {
     let verticalScrollDirection = AutoScrollVerticalDirection.NONE;
     let horizontalScrollDirection = AutoScrollHorizontalDirection.NONE;
 
+    // Check whether we should start scrolling the container.
+    if (this._isPointerNearDropContainer(pointerX, pointerY)) {
+      const element = coerceElement(this.element);
+
+      [verticalScrollDirection, horizontalScrollDirection] =
+          getElementScrollDirections(element, this._clientRect, pointerX, pointerY);
+
+      if (verticalScrollDirection || horizontalScrollDirection) {
+        scrollNode = element;
+      }
+    }
+
     // @breaking-change 9.0.0 Remove null check for _viewportRuler once it's a required parameter.
-    // Check whether we're in range to scroll the viewport.
-    if (this._viewportRuler) {
+    // Otherwise check if we can start scrolling the viewport.
+    if (this._viewportRuler && !verticalScrollDirection && !horizontalScrollDirection) {
       const {width, height} = this._viewportRuler.getViewportSize();
       const clientRect = {width, height, top: 0, right: width, bottom: height, left: 0};
       verticalScrollDirection = getVerticalScrollDirection(clientRect, pointerY);
       horizontalScrollDirection = getHorizontalScrollDirection(clientRect, pointerX);
       scrollNode = window;
     }
 
-    // If we couldn't find a scroll direction based on the
-    // window, try with the container, if the pointer is close by.
-    if (!verticalScrollDirection && !horizontalScrollDirection &&
-        this._isPointerNearDropContainer(pointerX, pointerY)) {
-      verticalScrollDirection = getVerticalScrollDirection(this._clientRect, pointerY);
-      horizontalScrollDirection = getHorizontalScrollDirection(this._clientRect, pointerX);
-      scrollNode = coerceElement(this.element);
-    }
-
-    // TODO(crisbeto): we also need to account for whether the view or element are scrollable in
-    // the first place. With the current approach we'll still try to scroll them, but it just
-    // won't do anything. The only case where this is relevant is that if we have a scrollable
-    // list close to the viewport edge where the viewport isn't scrollable. In this case the
-    // we'll be trying to scroll the viewport rather than the list.
-
     if (scrollNode && (verticalScrollDirection !== this._verticalScrollDirection ||
         horizontalScrollDirection !== this._horizontalScrollDirection ||
         scrollNode !== this._scrollNode)) {
@@ -1006,3 +1003,49 @@ function getHorizontalScrollDirection(clientRect: ClientRect, pointerX: number)
 
   return AutoScrollHorizontalDirection.NONE;
 }
+
+/**
+ * Gets the directions in which an element node should be scrolled,
+ * assuming that the user's pointer is already within it scrollable region.
+ * @param element Element for which we should calculate the scroll direction.
+ * @param clientRect Bounding client rectangle of the element.
+ * @param pointerX Position of the user's pointer along the x axis.
+ * @param pointerY Position of the user's pointer along the y axis.
+ */
+function getElementScrollDirections(element: HTMLElement, clientRect: ClientRect, pointerX: number,
+  pointerY: number): [AutoScrollVerticalDirection, AutoScrollHorizontalDirection] {
+  const computedVertical = getVerticalScrollDirection(clientRect, pointerY);
+  const computedHorizontal = getHorizontalScrollDirection(clientRect, pointerX);
+  let verticalScrollDirection = AutoScrollVerticalDirection.NONE;
+  let horizontalScrollDirection = AutoScrollHorizontalDirection.NONE;
+
+  // Note that we here we do some extra checks for whether the element is actually scrollable in
+  // a certain direction and we only assign the scroll direction if it is. We do this so that we
+  // can allow other elements to be scrolled, if the current element can't be scrolled anymore.
+  // This allows us to handle cases where the scroll regions of two scrollable elements overlap.
+  if (computedVertical) {
+    const scrollTop = element.scrollTop;
+
+    if (computedVertical === AutoScrollVerticalDirection.UP) {
+      if (scrollTop > 0) {
+        verticalScrollDirection = AutoScrollVerticalDirection.UP;
+      }
+    } else if (element.scrollHeight - scrollTop > element.clientHeight) {
+      verticalScrollDirection = AutoScrollVerticalDirection.DOWN;
+    }
+  }
+
+  if (computedHorizontal) {
+    const scrollLeft = element.scrollLeft;
+
+    if (computedHorizontal === AutoScrollHorizontalDirection.LEFT) {
+      if (scrollLeft > 0) {
+        horizontalScrollDirection = AutoScrollHorizontalDirection.LEFT;
+      }
+    } else if (element.scrollWidth - scrollLeft > element.clientWidth) {
+      horizontalScrollDirection = AutoScrollHorizontalDirection.RIGHT;
+    }
+  }
+
+  return [verticalScrollDirection, horizontalScrollDirection];
+}