Skip to content

Commit 423cecb

Browse files
committed
fix(drag-drop): don't move item in list if pointer is too far away
Currently the drop container will move an item in a list, no matter how far away the pointer is, as long as it overlaps another item along the proper axis. This can be annoying to users, because it can lead to unwanted re-arrangements of the list. These changes rework the logic so that the list is only re-arranged if the pointer is inside the drop container or within a buffer around it.
1 parent 29d5173 commit 423cecb

File tree

2 files changed

+99
-1
lines changed

2 files changed

+99
-1
lines changed

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

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -461,6 +461,41 @@ describe('CdkDrag', () => {
461461
.toEqual(['One', 'Two', 'Zero', 'Three']);
462462
}));
463463

464+
it('should not move items in a vertical list if the pointer is too far away', fakeAsync(() => {
465+
const fixture = createComponent(DraggableInDropZone);
466+
fixture.detectChanges();
467+
const dragItems = fixture.componentInstance.dragItems;
468+
469+
expect(dragItems.map(drag => drag.element.nativeElement.textContent!.trim()))
470+
.toEqual(['Zero', 'One', 'Two', 'Three']);
471+
472+
const firstItem = dragItems.first;
473+
const thirdItemRect = dragItems.toArray()[2].element.nativeElement.getBoundingClientRect();
474+
475+
// Move the cursor all the way to the right so it doesn't intersect along the x axis.
476+
dragElementViaMouse(fixture, firstItem.element.nativeElement,
477+
thirdItemRect.right + 1000, thirdItemRect.top + 1);
478+
flush();
479+
fixture.detectChanges();
480+
481+
expect(fixture.componentInstance.droppedSpy).toHaveBeenCalledTimes(1);
482+
483+
const event = fixture.componentInstance.droppedSpy.calls.mostRecent().args[0];
484+
485+
// Assert the event like this, rather than `toHaveBeenCalledWith`, because Jasmine will
486+
// go into an infinite loop trying to stringify the event, if the test fails.
487+
expect(event).toEqual({
488+
previousIndex: 0,
489+
currentIndex: 0,
490+
item: firstItem,
491+
container: fixture.componentInstance.dropInstance,
492+
previousContainer: fixture.componentInstance.dropInstance
493+
});
494+
495+
expect(dragItems.map(drag => drag.element.nativeElement.textContent!.trim()))
496+
.toEqual(['Zero', 'One', 'Two', 'Three']);
497+
}));
498+
464499
it('should not move the original element from its initial DOM position', fakeAsync(() => {
465500
const fixture = createComponent(DraggableInDropZone);
466501
fixture.detectChanges();
@@ -518,6 +553,41 @@ describe('CdkDrag', () => {
518553
.toEqual(['One', 'Two', 'Zero', 'Three']);
519554
}));
520555

556+
it('should not move items in a horizontal list if pointer is too far away', fakeAsync(() => {
557+
const fixture = createComponent(DraggableInHorizontalDropZone);
558+
fixture.detectChanges();
559+
const dragItems = fixture.componentInstance.dragItems;
560+
561+
expect(dragItems.map(drag => drag.element.nativeElement.textContent!.trim()))
562+
.toEqual(['Zero', 'One', 'Two', 'Three']);
563+
564+
const firstItem = dragItems.first;
565+
const thirdItemRect = dragItems.toArray()[2].element.nativeElement.getBoundingClientRect();
566+
567+
// Move the cursor all the way to the bottom so it doesn't intersect along the y axis.
568+
dragElementViaMouse(fixture, firstItem.element.nativeElement,
569+
thirdItemRect.left + 1, thirdItemRect.bottom + 1000);
570+
flush();
571+
fixture.detectChanges();
572+
573+
expect(fixture.componentInstance.droppedSpy).toHaveBeenCalledTimes(1);
574+
575+
const event = fixture.componentInstance.droppedSpy.calls.mostRecent().args[0];
576+
577+
// Assert the event like this, rather than `toHaveBeenCalledWith`, because Jasmine will
578+
// go into an infinite loop trying to stringify the event, if the test fails.
579+
expect(event).toEqual({
580+
previousIndex: 0,
581+
currentIndex: 0,
582+
item: firstItem,
583+
container: fixture.componentInstance.dropInstance,
584+
previousContainer: fixture.componentInstance.dropInstance
585+
});
586+
587+
expect(dragItems.map(drag => drag.element.nativeElement.textContent!.trim()))
588+
.toEqual(['Zero', 'One', 'Two', 'Three']);
589+
}));
590+
521591
it('should create a preview element while the item is dragged', fakeAsync(() => {
522592
const fixture = createComponent(DraggableInDropZone);
523593
fixture.detectChanges();

src/cdk/drag-drop/drop.ts

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,12 @@ import {moveItemInArray} from './drag-utils';
3030
/** Counter used to generate unique ids for drop zones. */
3131
let _uniqueIdCounter = 0;
3232

33+
/**
34+
* Buffer, in percent of the width/height, to add around the drop container when
35+
* determining whether an item should affect the list order.
36+
*/
37+
const DROP_PROXIMITY_THRESHOLD = 0.05;
38+
3339
/** Container that wraps a set of draggable items. */
3440
@Component({
3541
moduleId: module.id,
@@ -112,7 +118,8 @@ export class CdkDrop<T = any> implements OnInit, OnDestroy {
112118
/** Cache of the dimensions of all the items and the sibling containers. */
113119
private _positionCache = {
114120
items: [] as {drag: CdkDrag, clientRect: ClientRect, offset: number}[],
115-
siblings: [] as {drop: CdkDrop, clientRect: ClientRect}[]
121+
siblings: [] as {drop: CdkDrop, clientRect: ClientRect}[],
122+
self: {} as ClientRect
116123
};
117124

118125
/**
@@ -215,6 +222,11 @@ export class CdkDrop<T = any> implements OnInit, OnDestroy {
215222
* @param yOffset Position of the item along the Y axis.
216223
*/
217224
_sortItem(item: CdkDrag, xOffset: number, yOffset: number): void {
225+
// Don't sort the item if it's out of range.
226+
if (!this._isPointerNearDropContainer(xOffset, yOffset)) {
227+
return;
228+
}
229+
218230
const siblings = this._positionCache.items;
219231
const newIndex = this._getItemIndexFromPointerPosition(item, xOffset, yOffset);
220232

@@ -321,6 +333,8 @@ export class CdkDrop<T = any> implements OnInit, OnDestroy {
321333
.map(drop => typeof drop === 'string' ? this._dragDropRegistry.getDropContainer(drop)! : drop)
322334
.filter(drop => drop && drop !== this)
323335
.map(drop => ({drop, clientRect: drop.element.nativeElement.getBoundingClientRect()}));
336+
337+
this._positionCache.self = this.element.nativeElement.getBoundingClientRect();
324338
}
325339

326340
/** Resets the container to its initial state. */
@@ -369,4 +383,18 @@ export class CdkDrop<T = any> implements OnInit, OnDestroy {
369383
yOffset >= Math.floor(clientRect.top) && yOffset <= Math.floor(clientRect.bottom);
370384
});
371385
}
386+
387+
/**
388+
* Checks whether the pointer coordinates are close to the drop container.
389+
* @param xOffset Coordinates along the X axis.
390+
* @param yOffset Coordinates along the Y axis.
391+
*/
392+
private _isPointerNearDropContainer(xOffset: number, yOffset: number): boolean {
393+
const {top, right, bottom, left, width, height} = this._positionCache.self;
394+
const xThreshold = width * DROP_PROXIMITY_THRESHOLD;
395+
const yThreshold = height * DROP_PROXIMITY_THRESHOLD;
396+
397+
return yOffset > top - yThreshold && yOffset < bottom + yThreshold &&
398+
xOffset > left - xThreshold && xOffset < right + xThreshold;
399+
}
372400
}

0 commit comments

Comments
 (0)