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

crisbeto · crisbeto · commit ce1bc9010ae1 · 2019-01-12T17:36:15.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
@@ -1448,6 +1448,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 = 'absolute';
+      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,15 @@ export class CdkDrag<T = any> implements AfterViewInit, OnChanges, OnDestroy {
     @Inject(DOCUMENT) private _document: any,
     private _ngZone: NgZone,
     private _viewContainerRef: ViewContainerRef,
-    private _viewportRuler: ViewportRuler,
+
+    // @breaking-change 8.0.0 `_viewportRuler` argument to be removed as it isn't being used.
+    _viewportRuler: ViewportRuler,
     private _dragDropRegistry: DragDropRegistry<DragRef, DropListRef>,
     @Inject(CDK_DRAG_CONFIG) private _config: DragRefConfig,
     @Optional() private _dir: Directionality) {
 
     const ref = this._dragRef = new DragRef(element, this._document, this._ngZone,
-      this._viewContainerRef, this._viewportRuler, this._dragDropRegistry,
+      this._viewContainerRef, this._dragDropRegistry,
       this._config, this.dropContainer ? this.dropContainer._dropListRef : undefined,
       this._dir);
     ref.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 {Directionality} from '@angular/cdk/bidi';
 import {normalizePassiveListenerOptions} from '@angular/cdk/platform';
 import {coerceBooleanProperty} from '@angular/cdk/coercion';
@@ -267,7 +266,6 @@ export class DragRef<T = any> {
     private _document: Document,
     private _ngZone: NgZone,
     private _viewContainerRef: ViewContainerRef,
-    private _viewportRuler: ViewportRuler,
     private _dragDropRegistry: DragDropRegistry<DragRef, DropListRef>,
     private _config: DragRefConfig,
     /** Droppable container that the draggable is a part of. */
@@ -630,7 +628,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();
