Skip to content

Commit 73dd58b

Browse files
committed
fix(cdk/drag-drop): constrainPosition not working as expected
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.
1 parent d40da65 commit 73dd58b

File tree

4 files changed

+58
-28
lines changed

4 files changed

+58
-28
lines changed

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1081,8 +1081,12 @@ describe('CdkDrag', () => {
10811081
expect(spy).toHaveBeenCalledWith(
10821082
jasmine.objectContaining({x: 300, y: 300}),
10831083
jasmine.any(DragRef),
1084+
jasmine.anything(),
10841085
);
1085-
expect(dragElement.style.transform).toBe('translate3d(50px, 50px, 0px)');
1086+
1087+
const elementRect = dragElement.getBoundingClientRect();
1088+
expect(Math.floor(elementRect.top)).toBe(50);
1089+
expect(Math.floor(elementRect.left)).toBe(50);
10861090
}));
10871091

10881092
it('should throw if drag item is attached to an ng-container', fakeAsync(() => {
@@ -3680,6 +3684,7 @@ describe('CdkDrag', () => {
36803684
expect(spy).toHaveBeenCalledWith(
36813685
jasmine.objectContaining({x: 200, y: 200}),
36823686
jasmine.any(DragRef),
3687+
jasmine.anything(),
36833688
);
36843689
expect(Math.floor(previewRect.top)).toBe(50);
36853690
expect(Math.floor(previewRect.left)).toBe(50);

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

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -132,10 +132,14 @@ export class CdkDrag<T = any> implements AfterViewInit, OnChanges, OnDestroy {
132132
/**
133133
* Function that can be used to customize the logic of how the position of the drag item
134134
* is limited while it's being dragged. Gets called with a point containing the current position
135-
* of the user's pointer on the page and should return a point describing where the item should
136-
* be rendered.
135+
* of the user's pointer on the page, a reference to the item being dragged and its dimenstions.
136+
* Should return a point describing where the item should be rendered.
137137
*/
138-
@Input('cdkDragConstrainPosition') constrainPosition?: (point: Point, dragRef: DragRef) => Point;
138+
@Input('cdkDragConstrainPosition') constrainPosition?: (
139+
userPointerPosition: Point,
140+
dragRef: DragRef,
141+
dimensions: ClientRect,
142+
) => Point;
139143

140144
/** Class to be added to the preview element. */
141145
@Input('cdkDragPreviewClass') previewClass: string | string[];

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

Lines changed: 43 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,9 @@ export class DragRef<T = any> {
240240
/** Whether the native dragging interactions have been enabled on the root element. */
241241
private _nativeInteractionsEnabled = true;
242242

243+
/** Client rect of the root element when the dragging sequence has started. */
244+
private _initialClientRect?: ClientRect;
245+
243246
/** Cached dimensions of the preview element. Should be read via `_getPreviewRect`. */
244247
private _previewRect?: ClientRect;
245248

@@ -355,10 +358,14 @@ export class DragRef<T = any> {
355358
/**
356359
* Function that can be used to customize the logic of how the position of the drag item
357360
* is limited while it's being dragged. Gets called with a point containing the current position
358-
* of the user's pointer on the page and should return a point describing where the item should
359-
* be rendered.
361+
* of the user's pointer on the page, a reference to the item being dragged and its dimenstions.
362+
* Should return a point describing where the item should be rendered.
360363
*/
361-
constrainPosition?: (point: Point, dragRef: DragRef) => Point;
364+
constrainPosition?: (
365+
userPointerPosition: Point,
366+
dragRef: DragRef,
367+
dimensions: ClientRect,
368+
) => Point;
362369

363370
constructor(
364371
element: ElementRef<HTMLElement> | HTMLElement,
@@ -695,12 +702,12 @@ export class DragRef<T = any> {
695702
if (this._dropContainer) {
696703
this._updateActiveDropContainer(constrainedPointerPosition, pointerPosition);
697704
} else {
705+
// If there's a position constraint function, we want the element's top/left to be at the
706+
// specific position on the page. Use the initial position as a reference if that's the case.
707+
const offset = this.constrainPosition ? this._initialClientRect! : this._pickupPositionOnPage;
698708
const activeTransform = this._activeTransform;
699-
activeTransform.x =
700-
constrainedPointerPosition.x - this._pickupPositionOnPage.x + this._passiveTransform.x;
701-
activeTransform.y =
702-
constrainedPointerPosition.y - this._pickupPositionOnPage.y + this._passiveTransform.y;
703-
709+
activeTransform.x = constrainedPointerPosition.x - offset.x + this._passiveTransform.x;
710+
activeTransform.y = constrainedPointerPosition.y - offset.y + this._passiveTransform.y;
704711
this._applyRootElementTransform(activeTransform.x, activeTransform.y);
705712
}
706713

@@ -886,6 +893,7 @@ export class DragRef<T = any> {
886893
// Avoid multiple subscriptions and memory leaks when multi touch
887894
// (isDragging check above isn't enough because of possible temporal and/or dimensional delays)
888895
this._removeSubscriptions();
896+
this._initialClientRect = this._rootElement.getBoundingClientRect();
889897
this._pointerMoveSubscription = this._dragDropRegistry.pointerMove.subscribe(this._pointerMove);
890898
this._pointerUpSubscription = this._dragDropRegistry.pointerUp.subscribe(this._pointerUp);
891899
this._scrollSubscription = this._dragDropRegistry
@@ -903,7 +911,7 @@ export class DragRef<T = any> {
903911
this._pickupPositionInElement =
904912
previewTemplate && previewTemplate.template && !previewTemplate.matchSize
905913
? {x: 0, y: 0}
906-
: this._getPointerPositionInElement(referenceElement, event);
914+
: this._getPointerPositionInElement(this._initialClientRect, referenceElement, event);
907915
const pointerPosition =
908916
(this._pickupPositionOnPage =
909917
this._lastKnownPointerPosition =
@@ -925,7 +933,11 @@ export class DragRef<T = any> {
925933

926934
this._destroyPreview();
927935
this._destroyPlaceholder();
928-
this._boundaryRect = this._previewRect = this._initialTransform = undefined;
936+
this._initialClientRect =
937+
this._boundaryRect =
938+
this._previewRect =
939+
this._initialTransform =
940+
undefined;
929941

930942
// Re-enter the NgZone since we bound `document` events on the outside.
931943
this._ngZone.run(() => {
@@ -1013,10 +1025,15 @@ export class DragRef<T = any> {
10131025
if (this.isDragging()) {
10141026
this._dropContainer!._startScrollingIfNecessary(rawX, rawY);
10151027
this._dropContainer!._sortItem(this, x, y, this._pointerDirectionDelta);
1016-
this._applyPreviewTransform(
1017-
x - this._pickupPositionInElement.x,
1018-
y - this._pickupPositionInElement.y,
1019-
);
1028+
1029+
if (this.constrainPosition) {
1030+
this._applyPreviewTransform(x, y);
1031+
} else {
1032+
this._applyPreviewTransform(
1033+
x - this._pickupPositionInElement.x,
1034+
y - this._pickupPositionInElement.y,
1035+
);
1036+
}
10201037
}
10211038
}
10221039

@@ -1033,7 +1050,7 @@ export class DragRef<T = any> {
10331050
if (previewTemplate && previewConfig) {
10341051
// Measure the element before we've inserted the preview
10351052
// since the insertion could throw off the measurement.
1036-
const rootRect = previewConfig.matchSize ? this._rootElement.getBoundingClientRect() : null;
1053+
const rootRect = previewConfig.matchSize ? this._initialClientRect : null;
10371054
const viewRef = previewConfig.viewContainer.createEmbeddedView(
10381055
previewTemplate,
10391056
previewConfig.context,
@@ -1050,9 +1067,8 @@ export class DragRef<T = any> {
10501067
);
10511068
}
10521069
} else {
1053-
const element = this._rootElement;
1054-
preview = deepCloneNode(element);
1055-
matchElementSize(preview, element.getBoundingClientRect());
1070+
preview = deepCloneNode(this._rootElement);
1071+
matchElementSize(preview, this._initialClientRect!);
10561072

10571073
if (this._initialTransform) {
10581074
preview.style.transform = this._initialTransform;
@@ -1170,10 +1186,10 @@ export class DragRef<T = any> {
11701186
* @param event Event that initiated the dragging.
11711187
*/
11721188
private _getPointerPositionInElement(
1189+
elementRect: ClientRect,
11731190
referenceElement: HTMLElement,
11741191
event: MouseEvent | TouchEvent,
11751192
): Point {
1176-
const elementRect = this._rootElement.getBoundingClientRect();
11771193
const handleElement = referenceElement === this._rootElement ? null : referenceElement;
11781194
const referenceRect = handleElement ? handleElement.getBoundingClientRect() : elementRect;
11791195
const point = isTouchEvent(event) ? event.targetTouches[0] : event;
@@ -1222,7 +1238,9 @@ export class DragRef<T = any> {
12221238
/** Gets the pointer position on the page, accounting for any position constraints. */
12231239
private _getConstrainedPointerPosition(point: Point): Point {
12241240
const dropContainerLock = this._dropContainer ? this._dropContainer.lockAxis : null;
1225-
let {x, y} = this.constrainPosition ? this.constrainPosition(point, this) : point;
1241+
let {x, y} = this.constrainPosition
1242+
? this.constrainPosition(point, this, this._initialClientRect!)
1243+
: point;
12261244

12271245
if (this.lockAxis === 'x' || dropContainerLock === 'x') {
12281246
y = this._pickupPositionOnPage.y;
@@ -1361,8 +1379,9 @@ export class DragRef<T = any> {
13611379
return;
13621380
}
13631381

1364-
const boundaryRect = this._boundaryElement.getBoundingClientRect();
1382+
// Note: don't use `_clientRectAtStart` here, because we want the latest position.
13651383
const elementRect = this._rootElement.getBoundingClientRect();
1384+
const boundaryRect = this._boundaryElement.getBoundingClientRect();
13661385

13671386
// It's possible that the element got hidden away after dragging (e.g. by switching to a
13681387
// 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<T = any> {
15111530
// Cache the preview element rect if we haven't cached it already or if
15121531
// we cached it too early before the element dimensions were computed.
15131532
if (!this._previewRect || (!this._previewRect.width && !this._previewRect.height)) {
1514-
this._previewRect = (this._preview || this._rootElement).getBoundingClientRect();
1533+
this._previewRect = this._preview
1534+
? this._preview.getBoundingClientRect()
1535+
: this._initialClientRect!;
15151536
}
15161537

15171538
return this._previewRect;

tools/public_api_guard/cdk/drag-drop.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ export class CdkDrag<T = any> implements AfterViewInit, OnChanges, OnDestroy {
5555
dropContainer: CdkDropListInternal,
5656
_document: any, _ngZone: NgZone, _viewContainerRef: ViewContainerRef, config: DragDropConfig, _dir: Directionality, dragDrop: DragDrop, _changeDetectorRef: ChangeDetectorRef, _selfHandle?: CdkDragHandle | undefined, _parentDrag?: CdkDrag<any> | undefined);
5757
boundaryElement: string | ElementRef<HTMLElement> | HTMLElement;
58-
constrainPosition?: (point: Point, dragRef: DragRef) => Point;
58+
constrainPosition?: (userPointerPosition: Point, dragRef: DragRef, dimensions: ClientRect) => Point;
5959
data: T;
6060
get disabled(): boolean;
6161
set disabled(value: BooleanInput);
@@ -359,7 +359,7 @@ export class DragDropRegistry<I extends {
359359
export class DragRef<T = any> {
360360
constructor(element: ElementRef<HTMLElement> | HTMLElement, _config: DragRefConfig, _document: Document, _ngZone: NgZone, _viewportRuler: ViewportRuler, _dragDropRegistry: DragDropRegistry<DragRef, DropListRefInternal>);
361361
readonly beforeStarted: Subject<void>;
362-
constrainPosition?: (point: Point, dragRef: DragRef) => Point;
362+
constrainPosition?: (userPointerPosition: Point, dragRef: DragRef, dimensions: ClientRect) => Point;
363363
data: T;
364364
get disabled(): boolean;
365365
set disabled(value: boolean);

0 commit comments

Comments
 (0)