fix(drag-drop): incorrectly sorting element inside dialog with blocked scrolling

crisbeto · crisbeto · commit 91e4c365a977 · 2019-01-26T14:59:16.000+01:00
We use the `ViewportRuler` to determine how much a page has been scrolled in order to compensate for it in some of the `ClientRect`-related calculations. This breaks down when the drop list is inside a dialog with blocked scrolling, because scroll blocking works by offsetting the `html` node which in turn throws off the `ViewportRuler`. These changes switches to reading the values off the `window` which won't be thrown off. Fixes #14280.
diff --git a/src/cdk/drag-drop/directives/drag.spec.ts b/src/cdk/drag-drop/directives/drag.spec.ts
@@ -1507,6 +1507,22 @@ describe('CdkDrag', () => {
       }));
     }));
 
+    it('should sort correctly if the <html> node has been offset', fakeAsync(() => {
+      const documentElement = document.documentElement!;
+      const fixture = createComponent(DraggableInDropZone);
+      fixture.detectChanges();
+
+      documentElement.style.position = 'absolute';
+      documentElement.style.top = '-100px';
+
+      assertDownwardSorting(fixture, fixture.componentInstance.dragItems.map(item => {
+        return item.element.nativeElement;
+      }));
+
+      documentElement.style.position = '';
+      documentElement.style.top = '';
+    }));
+
     it('should move the placeholder as an item is being sorted down on a scrolled page',
       fakeAsync(() => {
         const fixture = createComponent(DraggableInDropZone);
diff --git a/src/cdk/drag-drop/directives/drag.ts b/src/cdk/drag-drop/directives/drag.ts
@@ -168,13 +168,13 @@ export class CdkDrag<T = any> implements AfterViewInit, OnChanges, OnDestroy {
     @Inject(DOCUMENT) private _document: any,
     private _ngZone: NgZone,
     private _viewContainerRef: ViewContainerRef,
-    viewportRuler: ViewportRuler,
+    _viewportRuler: ViewportRuler,
     dragDropRegistry: DragDropRegistry<DragRef, DropListRef>,
     @Inject(CDK_DRAG_CONFIG) config: DragRefConfig,
     @Optional() private _dir: Directionality,
 
     /**
-     * @deprecated `viewportRuler` and `dragDropRegistry` parameters
+     * @deprecated `_viewportRuler` and `dragDropRegistry` parameters
      * to be removed. Also `dragDrop` parameter to be made required.
      * @breaking-change 8.0.0.
      */
@@ -185,8 +185,7 @@ export class CdkDrag<T = any> implements AfterViewInit, OnChanges, OnDestroy {
     if (dragDrop) {
       this._dragRef = dragDrop.createDrag(element, config);
     } else {
-      this._dragRef = new DragRef(element, config, _document, _ngZone, viewportRuler,
-          dragDropRegistry);
+      this._dragRef = new DragRef(element, config, _document, _ngZone, dragDropRegistry);
     }
 
     this._dragRef.data = this;
diff --git a/src/cdk/drag-drop/drag-ref.ts b/src/cdk/drag-drop/drag-ref.ts
@@ -7,7 +7,6 @@
  */
 
 import {EmbeddedViewRef, ElementRef, NgZone, ViewContainerRef, TemplateRef} from '@angular/core';
-import {ViewportRuler} from '@angular/cdk/scrolling';
 import {Direction} from '@angular/cdk/bidi';
 import {normalizePassiveListenerOptions} from '@angular/cdk/platform';
 import {coerceBooleanProperty, coerceElement} from '@angular/cdk/coercion';
@@ -269,7 +268,6 @@ export class DragRef<T = any> {
     private _config: DragRefConfig,
     private _document: Document,
     private _ngZone: NgZone,
-    private _viewportRuler: ViewportRuler,
     private _dragDropRegistry: DragDropRegistry<DragRef, DropListRef>) {
 
     this.withRootElement(element);
@@ -661,7 +659,12 @@ export class DragRef<T = any> {
     this._initialContainer = this._dropContainer!;
     this._pointerMoveSubscription = this._dragDropRegistry.pointerMove.subscribe(this._pointerMove);
     this._pointerUpSubscription = this._dragDropRegistry.pointerUp.subscribe(this._pointerUp);
-    this._scrollPosition = this._viewportRuler.getViewportScrollPosition();
+
+    // Note that we use the scrollX and scrollY directly, instead of going through the
+    // ViewportRuler, because the first value the ruler looks at is the top/left offset of the
+    // `document.documentElement` which works for most cases, but breaks if the element
+    // is offset by something like the `BlockScrollStrategy`.
+    this._scrollPosition = {top: window.scrollY, left: window.scrollX};
 
     if (this._boundaryElement) {
       this._boundaryRect = this._boundaryElement.getBoundingClientRect();
