From 59268fa5bfb646302c4c9c97fab3f1d0faa233fb Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Sun, 12 Jun 2022 09:09:12 +0200 Subject: [PATCH] fix(cdk/drag-drop): constrainPosition not working as expected (#25061) Fixes that the `constrainPosition` input wasn't constraining the position as described in the docs. This may have worked at some point, but it's definitely broken now. Our tests didn't catch it, because they were looking at the `transform` which was wrong, instead of the position the element ended up at. Fixes #25046. (cherry picked from commit 2c956c020cfd88d709c27805c01d81a6b7ac8fcc) --- src/cdk/drag-drop/directives/drag.spec.ts | 7 ++- src/cdk/drag-drop/directives/drag.ts | 10 ++-- src/cdk/drag-drop/drag-ref.ts | 65 +++++++++++++++-------- tools/public_api_guard/cdk/drag-drop.md | 4 +- 4 files changed, 58 insertions(+), 28 deletions(-) diff --git a/src/cdk/drag-drop/directives/drag.spec.ts b/src/cdk/drag-drop/directives/drag.spec.ts index 349947bd72a9..314deae4c929 100644 --- a/src/cdk/drag-drop/directives/drag.spec.ts +++ b/src/cdk/drag-drop/directives/drag.spec.ts @@ -1081,8 +1081,12 @@ describe('CdkDrag', () => { expect(spy).toHaveBeenCalledWith( jasmine.objectContaining({x: 300, y: 300}), jasmine.any(DragRef), + jasmine.anything(), ); - expect(dragElement.style.transform).toBe('translate3d(50px, 50px, 0px)'); + + const elementRect = dragElement.getBoundingClientRect(); + expect(Math.floor(elementRect.top)).toBe(50); + expect(Math.floor(elementRect.left)).toBe(50); })); it('should throw if drag item is attached to an ng-container', fakeAsync(() => { @@ -3680,6 +3684,7 @@ describe('CdkDrag', () => { expect(spy).toHaveBeenCalledWith( jasmine.objectContaining({x: 200, y: 200}), jasmine.any(DragRef), + jasmine.anything(), ); expect(Math.floor(previewRect.top)).toBe(50); expect(Math.floor(previewRect.left)).toBe(50); diff --git a/src/cdk/drag-drop/directives/drag.ts b/src/cdk/drag-drop/directives/drag.ts index 982d27028f84..43402365bd34 100644 --- a/src/cdk/drag-drop/directives/drag.ts +++ b/src/cdk/drag-drop/directives/drag.ts @@ -132,10 +132,14 @@ export class CdkDrag implements AfterViewInit, OnChanges, OnDestroy { /** * Function that can be used to customize the logic of how the position of the drag item * is limited while it's being dragged. Gets called with a point containing the current position - * of the user's pointer on the page and should return a point describing where the item should - * be rendered. + * of the user's pointer on the page, a reference to the item being dragged and its dimenstions. + * Should return a point describing where the item should be rendered. */ - @Input('cdkDragConstrainPosition') constrainPosition?: (point: Point, dragRef: DragRef) => Point; + @Input('cdkDragConstrainPosition') constrainPosition?: ( + userPointerPosition: Point, + dragRef: DragRef, + dimensions: ClientRect, + ) => Point; /** Class to be added to the preview element. */ @Input('cdkDragPreviewClass') previewClass: string | string[]; diff --git a/src/cdk/drag-drop/drag-ref.ts b/src/cdk/drag-drop/drag-ref.ts index a3cddece0739..ab387825c27d 100644 --- a/src/cdk/drag-drop/drag-ref.ts +++ b/src/cdk/drag-drop/drag-ref.ts @@ -240,6 +240,9 @@ export class DragRef { /** Whether the native dragging interactions have been enabled on the root element. */ private _nativeInteractionsEnabled = true; + /** Client rect of the root element when the dragging sequence has started. */ + private _initialClientRect?: ClientRect; + /** Cached dimensions of the preview element. Should be read via `_getPreviewRect`. */ private _previewRect?: ClientRect; @@ -355,10 +358,14 @@ export class DragRef { /** * Function that can be used to customize the logic of how the position of the drag item * is limited while it's being dragged. Gets called with a point containing the current position - * of the user's pointer on the page and should return a point describing where the item should - * be rendered. + * of the user's pointer on the page, a reference to the item being dragged and its dimenstions. + * Should return a point describing where the item should be rendered. */ - constrainPosition?: (point: Point, dragRef: DragRef) => Point; + constrainPosition?: ( + userPointerPosition: Point, + dragRef: DragRef, + dimensions: ClientRect, + ) => Point; constructor( element: ElementRef | HTMLElement, @@ -695,12 +702,12 @@ export class DragRef { if (this._dropContainer) { this._updateActiveDropContainer(constrainedPointerPosition, pointerPosition); } else { + // If there's a position constraint function, we want the element's top/left to be at the + // specific position on the page. Use the initial position as a reference if that's the case. + const offset = this.constrainPosition ? this._initialClientRect! : this._pickupPositionOnPage; const activeTransform = this._activeTransform; - activeTransform.x = - constrainedPointerPosition.x - this._pickupPositionOnPage.x + this._passiveTransform.x; - activeTransform.y = - constrainedPointerPosition.y - this._pickupPositionOnPage.y + this._passiveTransform.y; - + activeTransform.x = constrainedPointerPosition.x - offset.x + this._passiveTransform.x; + activeTransform.y = constrainedPointerPosition.y - offset.y + this._passiveTransform.y; this._applyRootElementTransform(activeTransform.x, activeTransform.y); } @@ -886,6 +893,7 @@ export class DragRef { // Avoid multiple subscriptions and memory leaks when multi touch // (isDragging check above isn't enough because of possible temporal and/or dimensional delays) this._removeSubscriptions(); + this._initialClientRect = this._rootElement.getBoundingClientRect(); this._pointerMoveSubscription = this._dragDropRegistry.pointerMove.subscribe(this._pointerMove); this._pointerUpSubscription = this._dragDropRegistry.pointerUp.subscribe(this._pointerUp); this._scrollSubscription = this._dragDropRegistry @@ -903,7 +911,7 @@ export class DragRef { this._pickupPositionInElement = previewTemplate && previewTemplate.template && !previewTemplate.matchSize ? {x: 0, y: 0} - : this._getPointerPositionInElement(referenceElement, event); + : this._getPointerPositionInElement(this._initialClientRect, referenceElement, event); const pointerPosition = (this._pickupPositionOnPage = this._lastKnownPointerPosition = @@ -925,7 +933,11 @@ export class DragRef { this._destroyPreview(); this._destroyPlaceholder(); - this._boundaryRect = this._previewRect = this._initialTransform = undefined; + this._initialClientRect = + this._boundaryRect = + this._previewRect = + this._initialTransform = + undefined; // Re-enter the NgZone since we bound `document` events on the outside. this._ngZone.run(() => { @@ -1013,10 +1025,15 @@ export class DragRef { if (this.isDragging()) { this._dropContainer!._startScrollingIfNecessary(rawX, rawY); this._dropContainer!._sortItem(this, x, y, this._pointerDirectionDelta); - this._applyPreviewTransform( - x - this._pickupPositionInElement.x, - y - this._pickupPositionInElement.y, - ); + + if (this.constrainPosition) { + this._applyPreviewTransform(x, y); + } else { + this._applyPreviewTransform( + x - this._pickupPositionInElement.x, + y - this._pickupPositionInElement.y, + ); + } } } @@ -1033,7 +1050,7 @@ export class DragRef { if (previewTemplate && previewConfig) { // Measure the element before we've inserted the preview // since the insertion could throw off the measurement. - const rootRect = previewConfig.matchSize ? this._rootElement.getBoundingClientRect() : null; + const rootRect = previewConfig.matchSize ? this._initialClientRect : null; const viewRef = previewConfig.viewContainer.createEmbeddedView( previewTemplate, previewConfig.context, @@ -1050,9 +1067,8 @@ export class DragRef { ); } } else { - const element = this._rootElement; - preview = deepCloneNode(element); - matchElementSize(preview, element.getBoundingClientRect()); + preview = deepCloneNode(this._rootElement); + matchElementSize(preview, this._initialClientRect!); if (this._initialTransform) { preview.style.transform = this._initialTransform; @@ -1170,10 +1186,10 @@ export class DragRef { * @param event Event that initiated the dragging. */ private _getPointerPositionInElement( + elementRect: ClientRect, referenceElement: HTMLElement, event: MouseEvent | TouchEvent, ): Point { - const elementRect = this._rootElement.getBoundingClientRect(); const handleElement = referenceElement === this._rootElement ? null : referenceElement; const referenceRect = handleElement ? handleElement.getBoundingClientRect() : elementRect; const point = isTouchEvent(event) ? event.targetTouches[0] : event; @@ -1222,7 +1238,9 @@ export class DragRef { /** Gets the pointer position on the page, accounting for any position constraints. */ private _getConstrainedPointerPosition(point: Point): Point { const dropContainerLock = this._dropContainer ? this._dropContainer.lockAxis : null; - let {x, y} = this.constrainPosition ? this.constrainPosition(point, this) : point; + let {x, y} = this.constrainPosition + ? this.constrainPosition(point, this, this._initialClientRect!) + : point; if (this.lockAxis === 'x' || dropContainerLock === 'x') { y = this._pickupPositionOnPage.y; @@ -1361,8 +1379,9 @@ export class DragRef { return; } - const boundaryRect = this._boundaryElement.getBoundingClientRect(); + // Note: don't use `_clientRectAtStart` here, because we want the latest position. const elementRect = this._rootElement.getBoundingClientRect(); + const boundaryRect = this._boundaryElement.getBoundingClientRect(); // It's possible that the element got hidden away after dragging (e.g. by switching to a // different tab). Don't do anything in this case so we don't clear the user's position. @@ -1511,7 +1530,9 @@ export class DragRef { // Cache the preview element rect if we haven't cached it already or if // we cached it too early before the element dimensions were computed. if (!this._previewRect || (!this._previewRect.width && !this._previewRect.height)) { - this._previewRect = (this._preview || this._rootElement).getBoundingClientRect(); + this._previewRect = this._preview + ? this._preview.getBoundingClientRect() + : this._initialClientRect!; } return this._previewRect; diff --git a/tools/public_api_guard/cdk/drag-drop.md b/tools/public_api_guard/cdk/drag-drop.md index d016ded0a554..631a2ea61882 100644 --- a/tools/public_api_guard/cdk/drag-drop.md +++ b/tools/public_api_guard/cdk/drag-drop.md @@ -55,7 +55,7 @@ export class CdkDrag implements AfterViewInit, OnChanges, OnDestroy { dropContainer: CdkDropListInternal, _document: any, _ngZone: NgZone, _viewContainerRef: ViewContainerRef, config: DragDropConfig, _dir: Directionality, dragDrop: DragDrop, _changeDetectorRef: ChangeDetectorRef, _selfHandle?: CdkDragHandle | undefined, _parentDrag?: CdkDrag | undefined); boundaryElement: string | ElementRef | HTMLElement; - constrainPosition?: (point: Point, dragRef: DragRef) => Point; + constrainPosition?: (userPointerPosition: Point, dragRef: DragRef, dimensions: ClientRect) => Point; data: T; get disabled(): boolean; set disabled(value: BooleanInput); @@ -359,7 +359,7 @@ export class DragDropRegistry { constructor(element: ElementRef | HTMLElement, _config: DragRefConfig, _document: Document, _ngZone: NgZone, _viewportRuler: ViewportRuler, _dragDropRegistry: DragDropRegistry); readonly beforeStarted: Subject; - constrainPosition?: (point: Point, dragRef: DragRef) => Point; + constrainPosition?: (userPointerPosition: Point, dragRef: DragRef, dimensions: ClientRect) => Point; data: T; get disabled(): boolean; set disabled(value: boolean);