Skip to content

Commit 46bc0a9

Browse files
crisbetocrisbeto
committed
fix(drag-drop): incorrectly sorting element inside dialog with blocked scrolling
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.
1 parent 57aadc2 commit 46bc0a9

File tree

5 files changed

+30
-12
lines changed

5 files changed

+30
-12
lines changed

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1530,6 +1530,22 @@ describe('CdkDrag', () => {
15301530
}));
15311531
}));
15321532

1533+
it('should sort correctly if the <html> node has been offset', fakeAsync(() => {
1534+
const documentElement = document.documentElement!;
1535+
const fixture = createComponent(DraggableInDropZone);
1536+
fixture.detectChanges();
1537+
1538+
documentElement.style.position = 'absolute';
1539+
documentElement.style.top = '-100px';
1540+
1541+
assertDownwardSorting(fixture, fixture.componentInstance.dragItems.map(item => {
1542+
return item.element.nativeElement;
1543+
}));
1544+
1545+
documentElement.style.position = '';
1546+
documentElement.style.top = '';
1547+
}));
1548+
15331549
it('should move the placeholder as an item is being sorted down on a scrolled page',
15341550
fakeAsync(() => {
15351551
const fixture = createComponent(DraggableInDropZone);

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ export class CdkDrag<T = any> implements AfterViewInit, OnChanges, OnDestroy {
170170
@Inject(DOCUMENT) private _document: any,
171171
private _ngZone: NgZone,
172172
private _viewContainerRef: ViewContainerRef,
173-
viewportRuler: ViewportRuler,
173+
_viewportRuler: ViewportRuler,
174174
dragDropRegistry: DragDropRegistry<DragRef, DropListRef>,
175175
@Inject(CDK_DRAG_CONFIG) config: DragRefConfig,
176176
@Optional() private _dir: Directionality,
@@ -188,8 +188,7 @@ export class CdkDrag<T = any> implements AfterViewInit, OnChanges, OnDestroy {
188188
if (dragDrop) {
189189
this._dragRef = dragDrop.createDrag(element, config);
190190
} else {
191-
this._dragRef = new DragRef(element, config, _document, _ngZone, viewportRuler,
192-
dragDropRegistry);
191+
this._dragRef = new DragRef(element, config, _document, _ngZone, dragDropRegistry);
193192
}
194193

195194
this._dragRef.data = this;

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,8 @@ export class DragDrop {
2727
constructor(
2828
@Inject(DOCUMENT) private _document: any,
2929
private _ngZone: NgZone,
30-
private _viewportRuler: ViewportRuler,
30+
// @breaking-change 8.0.0 `_viewportRuler` parameter isn't being used. To be removed.
31+
_viewportRuler: ViewportRuler,
3132
private _dragDropRegistry: DragDropRegistry<DragRef, DropListRef>) {}
3233

3334
/**
@@ -38,8 +39,7 @@ export class DragDrop {
3839
createDrag<T = any>(element: ElementRef<HTMLElement> | HTMLElement,
3940
config: DragRefConfig = DEFAULT_CONFIG): DragRef<T> {
4041

41-
return new DragRef<T>(element, config, this._document, this._ngZone, this._viewportRuler,
42-
this._dragDropRegistry);
42+
return new DragRef<T>(element, config, this._document, this._ngZone, this._dragDropRegistry);
4343
}
4444

4545
/**

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

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
*/
88

99
import {EmbeddedViewRef, ElementRef, NgZone, ViewContainerRef, TemplateRef} from '@angular/core';
10-
import {ViewportRuler} from '@angular/cdk/scrolling';
1110
import {Direction} from '@angular/cdk/bidi';
1211
import {normalizePassiveListenerOptions} from '@angular/cdk/platform';
1312
import {coerceBooleanProperty, coerceElement} from '@angular/cdk/coercion';
@@ -269,7 +268,6 @@ export class DragRef<T = any> {
269268
private _config: DragRefConfig,
270269
private _document: Document,
271270
private _ngZone: NgZone,
272-
private _viewportRuler: ViewportRuler,
273271
private _dragDropRegistry: DragDropRegistry<DragRef, DropListRef>) {
274272

275273
this.withRootElement(element);
@@ -661,7 +659,12 @@ export class DragRef<T = any> {
661659
this._initialContainer = this._dropContainer!;
662660
this._pointerMoveSubscription = this._dragDropRegistry.pointerMove.subscribe(this._pointerMove);
663661
this._pointerUpSubscription = this._dragDropRegistry.pointerUp.subscribe(this._pointerUp);
664-
this._scrollPosition = this._viewportRuler.getViewportScrollPosition();
662+
663+
// Note that we use the scrollX and scrollY directly, instead of going through the
664+
// ViewportRuler, because the first value the ruler looks at is the top/left offset of the
665+
// `document.documentElement` which works for most cases, but breaks if the element
666+
// is offset by something like the `BlockScrollStrategy`.
667+
this._scrollPosition = {top: window.scrollY, left: window.scrollX};
665668

666669
if (this._boundaryElement) {
667670
this._boundaryRect = this._boundaryElement.getBoundingClientRect();

‎tools/public_api_guard/cdk/drag-drop.d.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,8 @@ export declare class CdkDrag<T = any> implements AfterViewInit, OnChanges, OnDes
2727
started: EventEmitter<CdkDragStart>;
2828
constructor(
2929
element: ElementRef<HTMLElement>,
30-
dropContainer: CdkDropList, _document: any, _ngZone: NgZone, _viewContainerRef: ViewContainerRef, viewportRuler: ViewportRuler, dragDropRegistry: DragDropRegistry<DragRef, DropListRef>, config: DragRefConfig, _dir: Directionality,
31-
dragDrop?: DragDrop, _changeDetectorRef?: ChangeDetectorRef | undefined);
30+
dropContainer: CdkDropList, _document: any, _ngZone: NgZone, _viewContainerRef: ViewContainerRef, _viewportRuler: ViewportRuler, dragDropRegistry: DragDropRegistry<DragRef, DropListRef>, config: DragRefConfig, _dir: Directionality,
31+
dragDrop?: DragDrop, _changeDetectorRef?: ChangeDetectorRef | undefined);
3232
getPlaceholderElement(): HTMLElement;
3333
getRootElement(): HTMLElement;
3434
ngAfterViewInit(): void;
@@ -242,7 +242,7 @@ export declare class DragRef<T = any> {
242242
started: Subject<{
243243
source: DragRef<any>;
244244
}>;
245-
constructor(element: ElementRef<HTMLElement> | HTMLElement, _config: DragRefConfig, _document: Document, _ngZone: NgZone, _viewportRuler: ViewportRuler, _dragDropRegistry: DragDropRegistry<DragRef, DropListRef>);
245+
constructor(element: ElementRef<HTMLElement> | HTMLElement, _config: DragRefConfig, _document: Document, _ngZone: NgZone, _dragDropRegistry: DragDropRegistry<DragRef, DropListRef>);
246246
_withDropContainer(container: DropListRef): void;
247247
disableHandle(handle: HTMLElement): void;
248248
dispose(): void;

0 commit comments

Comments
 (0)