fix(drag-drop): not accounting for scroll position when dragging (#12098)

Author: crisbeto

src/cdk-experimental/drag-drop/BUILD.bazel

@@ -12,6 +12,7 @@ ng_module(
   deps = [
     "@rxjs",
     "//src/cdk/platform",
+    "//src/cdk/overlay",
   ],
   tsconfig = "//src/cdk-experimental:tsconfig-build.json",
 )

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

@@ -39,6 +39,21 @@ describe('CdkDrag', () => {
         expect(dragElement.style.transform).toBe('translate3d(50px, 100px, 0px)');
       }));
 
+      it('should drag an element freely to a particular position when the page is scrolled',
+        fakeAsync(() => {
+          const fixture = createComponent(StandaloneDraggable);
+          fixture.detectChanges();
+
+          const cleanup = makePageScrollable();
+          const dragElement = fixture.componentInstance.dragElement.nativeElement;
+
+          scrollTo(0, 500);
+          expect(dragElement.style.transform).toBeFalsy();
+          dragElementViaMouse(fixture, dragElement, 50, 100);
+          expect(dragElement.style.transform).toBe('translate3d(50px, 100px, 0px)');
+          cleanup();
+        }));
+
       it('should continue dragging the element from where it was left off', fakeAsync(() => {
         const fixture = createComponent(StandaloneDraggable);
         fixture.detectChanges();
@@ -52,6 +67,26 @@ describe('CdkDrag', () => {
         dragElementViaMouse(fixture, dragElement, 100, 200);
         expect(dragElement.style.transform).toBe('translate3d(150px, 300px, 0px)');
       }));
+
+      it('should continue dragging from where it was left off when the page is scrolled',
+        fakeAsync(() => {
+          const fixture = createComponent(StandaloneDraggable);
+          fixture.detectChanges();
+
+          const dragElement = fixture.componentInstance.dragElement.nativeElement;
+          const cleanup = makePageScrollable();
+
+          scrollTo(0, 500);
+          expect(dragElement.style.transform).toBeFalsy();
+
+          dragElementViaMouse(fixture, dragElement, 50, 100);
+          expect(dragElement.style.transform).toBe('translate3d(50px, 100px, 0px)');
+
+          dragElementViaMouse(fixture, dragElement, 100, 200);
+          expect(dragElement.style.transform).toBe('translate3d(150px, 300px, 0px)');
+
+          cleanup();
+        }));
     });
 
     describe('touch dragging', () => {
@@ -65,6 +100,21 @@ describe('CdkDrag', () => {
         expect(dragElement.style.transform).toBe('translate3d(50px, 100px, 0px)');
       }));
 
+      it('should drag an element freely to a particular position when the page is scrolled',
+        fakeAsync(() => {
+          const fixture = createComponent(StandaloneDraggable);
+          fixture.detectChanges();
+
+          const dragElement = fixture.componentInstance.dragElement.nativeElement;
+          const cleanup = makePageScrollable();
+
+          scrollTo(0, 500);
+          expect(dragElement.style.transform).toBeFalsy();
+          dragElementViaTouch(fixture, dragElement, 50, 100);
+          expect(dragElement.style.transform).toBe('translate3d(50px, 100px, 0px)');
+          cleanup();
+        }));
+
       it('should continue dragging the element from where it was left off', fakeAsync(() => {
         const fixture = createComponent(StandaloneDraggable);
         fixture.detectChanges();
@@ -79,6 +129,26 @@ describe('CdkDrag', () => {
         expect(dragElement.style.transform).toBe('translate3d(150px, 300px, 0px)');
       }));
 
+      it('should continue dragging from where it was left off when the page is scrolled',
+        fakeAsync(() => {
+          const fixture = createComponent(StandaloneDraggable);
+          fixture.detectChanges();
+
+          const dragElement = fixture.componentInstance.dragElement.nativeElement;
+          const cleanup = makePageScrollable();
+
+          scrollTo(0, 500);
+          expect(dragElement.style.transform).toBeFalsy();
+
+          dragElementViaTouch(fixture, dragElement, 50, 100);
+          expect(dragElement.style.transform).toBe('translate3d(50px, 100px, 0px)');
+
+          dragElementViaTouch(fixture, dragElement, 100, 200);
+          expect(dragElement.style.transform).toBe('translate3d(150px, 300px, 0px)');
+
+          cleanup();
+        }));
+
       it('should prevent the default `touchmove` action on the page while dragging',
         fakeAsync(() => {
           const fixture = createComponent(StandaloneDraggable);
@@ -257,58 +327,36 @@ describe('CdkDrag', () => {
     it('should move the placeholder as an item is being sorted down', fakeAsync(() => {
       const fixture = createComponent(DraggableInDropZone);
       fixture.detectChanges();
+      assertDownwardSorting(fixture);
+    }));
 
-      const items = fixture.componentInstance.dragItems.toArray();
-      const draggedItem = items[0].element.nativeElement;
-      const {top, left} = draggedItem.getBoundingClientRect();
-
-      dispatchMouseEvent(draggedItem, 'mousedown', left, top);
-      fixture.detectChanges();
-
-      const placeholder = document.querySelector('.cdk-drag-placeholder')! as HTMLElement;
-
-      // Drag over each item one-by-one going downwards.
-      for (let i = 0; i < items.length; i++) {
-        const elementRect = items[i].element.nativeElement.getBoundingClientRect();
-
-        // Add a few pixels to the top offset so we get some overlap.
-        dispatchMouseEvent(document, 'mousemove', elementRect.left, elementRect.top + 5);
+    it('should move the placeholder as an item is being sorted down on a scrolled page',
+      fakeAsync(() => {
+        const fixture = createComponent(DraggableInDropZone);
         fixture.detectChanges();
-        expect(getElementIndex(placeholder)).toBe(i);
-      }
+        const cleanup = makePageScrollable();
 
-      dispatchMouseEvent(document, 'mouseup');
-      fixture.detectChanges();
-      flush();
-    }));
+        scrollTo(0, 500);
+        assertDownwardSorting(fixture);
+        cleanup();
+      }));
 
     it('should move the placeholder as an item is being sorted up', fakeAsync(() => {
       const fixture = createComponent(DraggableInDropZone);
       fixture.detectChanges();
+      assertUpwardSorting(fixture);
+    }));
 
-      const items = fixture.componentInstance.dragItems.toArray();
-      const draggedItem = items[items.length - 1].element.nativeElement;
-      const {top, left} = draggedItem.getBoundingClientRect();
-
-      dispatchMouseEvent(draggedItem, 'mousedown', left, top);
-      fixture.detectChanges();
-
-      const placeholder = document.querySelector('.cdk-drag-placeholder')! as HTMLElement;
-
-      // Drag over each item one-by-one going upwards.
-      for (let i = items.length - 1; i > -1; i--) {
-        const elementRect = items[i].element.nativeElement.getBoundingClientRect();
-
-        // Remove a few pixels from the bottom offset so we get some overlap.
-        dispatchMouseEvent(document, 'mousemove', elementRect.left, elementRect.bottom - 5);
+    it('should move the placeholder as an item is being sorted up on a scrolled page',
+      fakeAsync(() => {
+        const fixture = createComponent(DraggableInDropZone);
         fixture.detectChanges();
-        expect(getElementIndex(placeholder)).toBe(Math.min(i + 1, items.length - 1));
-      }
+        const cleanup = makePageScrollable();
 
-      dispatchMouseEvent(document, 'mouseup');
-      fixture.detectChanges();
-      flush();
-    }));
+        scrollTo(0, 500);
+        assertUpwardSorting(fixture);
+        cleanup();
+      }));
 
     it('should clean up the preview element if the item is destroyed mid-drag', fakeAsync(() => {
       const fixture = createComponent(DraggableInDropZone);
@@ -580,3 +628,77 @@ function dragElementViaTouch(fixture: ComponentFixture<any>,
 function getElementIndex(element: HTMLElement) {
   return element.parentElement ? Array.from(element.parentElement.children).indexOf(element) : -1;
 }
+
+/**
+ * Adds a large element to the page in order to make it scrollable.
+ * @returns Function that should be used to clean up after the test is done.
+ */
+function makePageScrollable() {
+  const veryTallElement = document.createElement('div');
+  veryTallElement.style.width = '100%';
+  veryTallElement.style.height = '2000px';
+  document.body.appendChild(veryTallElement);
+
+  return () => {
+    scrollTo(0, 0);
+    veryTallElement.parentNode!.removeChild(veryTallElement);
+  };
+}
+
+/**
+ * Asserts that sorting an element down works correctly.
+ * @param fixture Fixture against which to run the assertions.
+ */
+function assertDownwardSorting(fixture: ComponentFixture<DraggableInDropZone>) {
+  const items = fixture.componentInstance.dragItems.toArray();
+  const draggedItem = items[0].element.nativeElement;
+  const {top, left} = draggedItem.getBoundingClientRect();
+
+  dispatchMouseEvent(draggedItem, 'mousedown', left, top);
+  fixture.detectChanges();
+
+  const placeholder = document.querySelector('.cdk-drag-placeholder')! as HTMLElement;
+
+  // Drag over each item one-by-one going downwards.
+  for (let i = 0; i < items.length; i++) {
+    const elementRect = items[i].element.nativeElement.getBoundingClientRect();
+
+    // Add a few pixels to the top offset so we get some overlap.
+    dispatchMouseEvent(document, 'mousemove', elementRect.left, elementRect.top + 5);
+    fixture.detectChanges();
+    expect(getElementIndex(placeholder)).toBe(i);
+  }
+
+  dispatchMouseEvent(document, 'mouseup');
+  fixture.detectChanges();
+  flush();
+}
+
+/**
+ * Asserts that sorting an element up works correctly.
+ * @param fixture Fixture against which to run the assertions.
+ */
+function assertUpwardSorting(fixture: ComponentFixture<DraggableInDropZone>) {
+  const items = fixture.componentInstance.dragItems.toArray();
+  const draggedItem = items[items.length - 1].element.nativeElement;
+  const {top, left} = draggedItem.getBoundingClientRect();
+
+  dispatchMouseEvent(draggedItem, 'mousedown', left, top);
+  fixture.detectChanges();
+
+  const placeholder = document.querySelector('.cdk-drag-placeholder')! as HTMLElement;
+
+  // Drag over each item one-by-one going upwards.
+  for (let i = items.length - 1; i > -1; i--) {
+    const elementRect = items[i].element.nativeElement.getBoundingClientRect();
+
+    // Remove a few pixels from the bottom offset so we get some overlap.
+    dispatchMouseEvent(document, 'mousemove', elementRect.left, elementRect.bottom - 5);
+    fixture.detectChanges();
+    expect(getElementIndex(placeholder)).toBe(Math.min(i + 1, items.length - 1));
+  }
+
+  dispatchMouseEvent(document, 'mouseup');
+  fixture.detectChanges();
+  flush();
+}

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

@@ -28,6 +28,7 @@ import {supportsPassiveEventListeners} from '@angular/cdk/platform';
 import {CdkDragStart, CdkDragEnd, CdkDragExit, CdkDragEnter, CdkDragDrop} from './drag-events';
 import {CdkDragPreview} from './drag-preview';
 import {CdkDragPlaceholder} from './drag-placeholder';
+import {ViewportRuler} from '@angular/cdk/overlay';
 
 /** Event options that can be used to bind an active event. */
 const activeEventOptions = supportsPassiveEventListeners() ? {passive: false} : false;
@@ -84,6 +85,9 @@ export class CdkDrag implements AfterContentInit, OnDestroy {
   /** Drop container in which the CdkDrag resided when dragging began. */
   private _initialContainer: CdkDropContainer;
 
+  /** Cached scroll position on the page when the element was picked up. */
+  private _scrollPosition: {top: number, left: number};
+
   /** Element that can be used to drag the draggable item. */
   @ContentChild(CdkDragHandle) _handle: CdkDragHandle;
 
@@ -117,7 +121,8 @@ export class CdkDrag implements AfterContentInit, OnDestroy {
     @Inject(CDK_DROP_CONTAINER) @Optional() @SkipSelf() public dropContainer: CdkDropContainer,
     @Inject(DOCUMENT) document: any,
     private _ngZone: NgZone,
-    private _viewContainerRef: ViewContainerRef) {
+    private _viewContainerRef: ViewContainerRef,
+    private _viewportRuler: ViewportRuler) {
       this._document = document;
     }
 
@@ -135,7 +140,7 @@ export class CdkDrag implements AfterContentInit, OnDestroy {
     dragElement.addEventListener('mousedown', this._pointerDown);
     dragElement.addEventListener('touchstart', this._pointerDown);
 
-    // Webkit won't preventDefault on a dynamically-added `touchmove` listener, which means that
+    // WebKit won't preventDefault on a dynamically-added `touchmove` listener, which means that
     // we need to add one ahead of time. See https://bugs.webkit.org/show_bug.cgi?id=184250.
     // TODO: move into a central registry.
     this._ngZone.runOutsideAngular(() => {
@@ -165,6 +170,7 @@ export class CdkDrag implements AfterContentInit, OnDestroy {
 
     this._isDragging = true;
     this._initialContainer = this.dropContainer;
+    this._scrollPosition = this._viewportRuler.getViewportScrollPosition();
 
     // If we have a custom preview template, the element won't be visible anyway so we avoid the
     // extra `getBoundingClientRect` calls and just move the preview next to the cursor.
@@ -371,10 +377,12 @@ export class CdkDrag implements AfterContentInit, OnDestroy {
     const elementRect = this.element.nativeElement.getBoundingClientRect();
     const handleElement = this._handle ? this._handle.element.nativeElement : null;
     const referenceRect = handleElement ? handleElement.getBoundingClientRect() : elementRect;
-    const x = this._isTouchEvent(event) ? event.targetTouches[0].pageX - referenceRect.left :
-                                          event.offsetX;
-    const y = this._isTouchEvent(event) ? event.targetTouches[0].pageY - referenceRect.top :
-                                          event.offsetY;
+    const x = this._isTouchEvent(event) ?
+        event.targetTouches[0].pageX - referenceRect.left - this._scrollPosition.left :
+        event.offsetX;
+    const y = this._isTouchEvent(event) ?
+        event.targetTouches[0].pageY - referenceRect.top - this._scrollPosition.top :
+        event.offsetY;
 
     return {
       x: referenceRect.left - elementRect.left + x,
@@ -456,8 +464,12 @@ export class CdkDrag implements AfterContentInit, OnDestroy {
 
   /** Determines the point of the page that was touched by the user. */
   private _getPointerPositionOnPage(event: MouseEvent | TouchEvent): Point {
-    return this._isTouchEvent(event) ? {x: event.touches[0].pageX, y: event.touches[0].pageY} :
-                                       {x: event.pageX, y: event.pageY};
+    const point = this._isTouchEvent(event) ? event.touches[0] : event;
+
+    return {
+      x: point.pageX - this._scrollPosition.left,
+      y: point.pageY - this._scrollPosition.top
+    };
   }
 
   /** Listener used to prevent `touchmove` events while the element is being dragged. */

src/cdk-experimental/drag-drop/drop.scss

@@ -1,7 +1,7 @@
 $cdk-z-index-drag-preview: 1000;
 
 .cdk-drag-preview {
-  position: absolute;
+  position: fixed;
   top: 0;
   left: 0;
   z-index: $cdk-z-index-drag-preview;