Skip to content

Commit 39d48c7

Browse files
authored
fix(drag-drop): not starting auto scroll when inside boundary (#19865)
The logic that starts the auto-scrolling sequence was using the pointer position that is constrained within the boundary element which wasn't allow it to start auto scrolling. These changes also fix some internal issues that I noticed along the way: * We were using the pointer position at the last direction change to re-sort the element while dragging. This is incorrect since the user might not have changed direction. Now we use the position at the last pointer event. * The `_getConstrainedPointerPosition` method was mutating the passed in `Point` which can throw things off downstream. * The logic that updates the `ClientRect` of the boundary element was doing so on any scroll, even if it comes from inside the boundary. Now we only do it if a parent of the boundary was scrolled. Fixes #18596.
1 parent 523f71a commit 39d48c7

File tree

2 files changed

+45
-15
lines changed

2 files changed

+45
-15
lines changed

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3590,6 +3590,25 @@ describe('CdkDrag', () => {
35903590
expect(list.scrollLeft).toBeLessThan(initialScrollDistance);
35913591
}));
35923592

3593+
it('should be able to start auto scrolling with a drag boundary', fakeAsync(() => {
3594+
const fixture = createComponent(DraggableInScrollableHorizontalDropZone);
3595+
fixture.componentInstance.boundarySelector = '.drop-list';
3596+
fixture.detectChanges();
3597+
const item = fixture.componentInstance.dragItems.first.element.nativeElement;
3598+
const list = fixture.componentInstance.dropInstance.element.nativeElement;
3599+
const listRect = list.getBoundingClientRect();
3600+
3601+
expect(list.scrollLeft).toBe(0);
3602+
3603+
startDraggingViaMouse(fixture, item);
3604+
dispatchMouseEvent(document, 'mousemove', listRect.left + listRect.width,
3605+
listRect.top + listRect.height / 2);
3606+
fixture.detectChanges();
3607+
tickAnimationFrames(20);
3608+
3609+
expect(list.scrollLeft).toBeGreaterThan(0);
3610+
}));
3611+
35933612
it('should stop scrolling if the user moves their pointer away', fakeAsync(() => {
35943613
const fixture = createComponent(DraggableInScrollableVerticalDropZone);
35953614
fixture.detectChanges();
@@ -5555,6 +5574,7 @@ const HORIZONTAL_FIXTURE_TEMPLATE = `
55555574
*ngFor="let item of items"
55565575
[style.width.px]="item.width"
55575576
[style.margin-right.px]="item.margin"
5577+
[cdkDragBoundary]="boundarySelector"
55585578
cdkDrag>{{item.value}}</div>
55595579
</div>
55605580
`;
@@ -5573,6 +5593,7 @@ class DraggableInHorizontalDropZone {
55735593
{value: 'Two', width: ITEM_WIDTH, margin: 0},
55745594
{value: 'Three', width: ITEM_WIDTH, margin: 0}
55755595
];
5596+
boundarySelector: string;
55765597
droppedSpy = jasmine.createSpy('dropped spy').and.callFake((event: CdkDragDrop<string[]>) => {
55775598
moveItemInArray(this.items, event.previousIndex, event.currentIndex);
55785599
});

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

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,9 @@ export class DragRef<T = any> {
154154
/** Pointer position at which the last change in the delta occurred. */
155155
private _pointerPositionAtLastDirectionChange: Point;
156156

157+
/** Position of the pointer at the last pointer event. */
158+
private _lastKnownPointerPosition: Point;
159+
157160
/**
158161
* Root DOM node of the drag instance. This is the element that will
159162
* be moved around as the user is dragging.
@@ -495,10 +498,10 @@ export class DragRef<T = any> {
495498

496499
/** Updates the item's sort order based on the last-known pointer position. */
497500
_sortFromLastPointerPosition() {
498-
const position = this._pointerPositionAtLastDirectionChange;
501+
const position = this._lastKnownPointerPosition;
499502

500503
if (position && this._dropContainer) {
501-
this._updateActiveDropContainer(this._getConstrainedPointerPosition(position));
504+
this._updateActiveDropContainer(this._getConstrainedPointerPosition(position), position);
502505
}
503506
}
504507

@@ -600,10 +603,11 @@ export class DragRef<T = any> {
600603

601604
const constrainedPointerPosition = this._getConstrainedPointerPosition(pointerPosition);
602605
this._hasMoved = true;
606+
this._lastKnownPointerPosition = pointerPosition;
603607
this._updatePointerDirectionDelta(constrainedPointerPosition);
604608

605609
if (this._dropContainer) {
606-
this._updateActiveDropContainer(constrainedPointerPosition);
610+
this._updateActiveDropContainer(constrainedPointerPosition, pointerPosition);
607611
} else {
608612
const activeTransform = this._activeTransform;
609613
activeTransform.x =
@@ -797,7 +801,8 @@ export class DragRef<T = any> {
797801
this._pickupPositionInElement = previewTemplate && previewTemplate.template &&
798802
!previewTemplate.matchSize ? {x: 0, y: 0} :
799803
this._getPointerPositionInElement(referenceElement, event);
800-
const pointerPosition = this._pickupPositionOnPage = this._getPointerPositionOnPage(event);
804+
const pointerPosition = this._pickupPositionOnPage = this._lastKnownPointerPosition =
805+
this._getPointerPositionOnPage(event);
801806
this._pointerDirectionDelta = {x: 0, y: 0};
802807
this._pointerPositionAtLastDirectionChange = {x: pointerPosition.x, y: pointerPosition.y};
803808
this._dragStartTime = Date.now();
@@ -846,7 +851,7 @@ export class DragRef<T = any> {
846851
* Updates the item's position in its drop container, or moves it
847852
* into a new one, depending on its current drag position.
848853
*/
849-
private _updateActiveDropContainer({x, y}: Point) {
854+
private _updateActiveDropContainer({x, y}: Point, {x: rawX, y: rawY}: Point) {
850855
// Drop container that draggable has been moved into.
851856
let newContainer = this._initialContainer._getSiblingContainerFromPosition(this, x, y);
852857

@@ -878,7 +883,7 @@ export class DragRef<T = any> {
878883
});
879884
}
880885

881-
this._dropContainer!._startScrollingIfNecessary(x, y);
886+
this._dropContainer!._startScrollingIfNecessary(rawX, rawY);
882887
this._dropContainer!._sortItem(this, x, y, this._pointerDirectionDelta);
883888
this._preview.style.transform =
884889
getTransform(x - this._pickupPositionInElement.x, y - this._pickupPositionInElement.y);
@@ -1053,13 +1058,13 @@ export class DragRef<T = any> {
10531058

10541059
/** Gets the pointer position on the page, accounting for any position constraints. */
10551060
private _getConstrainedPointerPosition(point: Point): Point {
1056-
const constrainedPoint = this.constrainPosition ? this.constrainPosition(point, this) : point;
10571061
const dropContainerLock = this._dropContainer ? this._dropContainer.lockAxis : null;
1062+
let {x, y} = this.constrainPosition ? this.constrainPosition(point, this) : point;
10581063

10591064
if (this.lockAxis === 'x' || dropContainerLock === 'x') {
1060-
constrainedPoint.y = this._pickupPositionOnPage.y;
1065+
y = this._pickupPositionOnPage.y;
10611066
} else if (this.lockAxis === 'y' || dropContainerLock === 'y') {
1062-
constrainedPoint.x = this._pickupPositionOnPage.x;
1067+
x = this._pickupPositionOnPage.x;
10631068
}
10641069

10651070
if (this._boundaryRect) {
@@ -1071,11 +1076,11 @@ export class DragRef<T = any> {
10711076
const minX = boundaryRect.left + pickupX;
10721077
const maxX = boundaryRect.right - (previewRect.width - pickupX);
10731078

1074-
constrainedPoint.x = clamp(constrainedPoint.x, minX, maxX);
1075-
constrainedPoint.y = clamp(constrainedPoint.y, minY, maxY);
1079+
x = clamp(x, minX, maxX);
1080+
y = clamp(y, minY, maxY);
10761081
}
10771082

1078-
return constrainedPoint;
1083+
return {x, y};
10791084
}
10801085

10811086

@@ -1244,9 +1249,13 @@ export class DragRef<T = any> {
12441249
const scrollDifference = this._parentPositions.handleScroll(event);
12451250

12461251
if (scrollDifference) {
1247-
// ClientRect dimensions are based on the page's scroll position so
1248-
// we have to update the cached boundary ClientRect if the user has scrolled.
1249-
if (this._boundaryRect) {
1252+
const target = event.target as Node;
1253+
1254+
// ClientRect dimensions are based on the scroll position of the page and its parent node so
1255+
// we have to update the cached boundary ClientRect if the user has scrolled. Check for
1256+
// the `document` specifically since IE doesn't support `contains` on it.
1257+
if (this._boundaryRect && (target === this._document ||
1258+
(target !== this._boundaryElement && target.contains(this._boundaryElement)))) {
12501259
adjustClientRect(this._boundaryRect, scrollDifference.top, scrollDifference.left);
12511260
}
12521261

0 commit comments

Comments
 (0)