Skip to content

Commit cfa2d04

Browse files
crisbetojelbourn
authored andcommitted
fix(drag-drop): don't move item in list if pointer is too far away (#12827)
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 c8ae9fd commit cfa2d04

File tree

3 files changed

+116
-18
lines changed

3 files changed

+116
-18
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-container.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,10 @@ export interface CdkDropContainer<T = any> {
3636
/**
3737
* Emits an event to indicate that the user moved an item into the container.
3838
* @param item Item that was moved into the container.
39-
* @param xOffset Position of the item along the X axis.
40-
* @param yOffset Position of the item along the Y axis.
39+
* @param pointerX Position of the item along the X axis.
40+
* @param pointerY Position of the item along the Y axis.
4141
*/
42-
enter(item: CdkDrag, xOffset: number, yOffset: number): void;
42+
enter(item: CdkDrag, pointerX: number, pointerY: number): void;
4343

4444
/**
4545
* Removes an item from the container after it was dragged into another container by the user.
@@ -52,7 +52,7 @@ export interface CdkDropContainer<T = any> {
5252
* @param item Item whose index should be determined.
5353
*/
5454
getItemIndex(item: CdkDrag): number;
55-
_sortItem(item: CdkDrag, xOffset: number, yOffset: number): void;
55+
_sortItem(item: CdkDrag, pointerX: number, pointerY: number): void;
5656
_draggables: QueryList<CdkDrag>;
5757
_getSiblingContainerFromPosition(item: CdkDrag, x: number, y: number): CdkDropContainer | null;
5858
}

src/cdk/drag-drop/drop.ts

Lines changed: 42 additions & 14 deletions
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+
* Proximity, as a ratio to width/height, at which a
35+
* dragged item will affect the drop container.
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
/**
@@ -150,16 +157,16 @@ export class CdkDrop<T = any> implements OnInit, OnDestroy {
150157
/**
151158
* Emits an event to indicate that the user moved an item into the container.
152159
* @param item Item that was moved into the container.
153-
* @param xOffset Position of the item along the X axis.
154-
* @param yOffset Position of the item along the Y axis.
160+
* @param pointerX Position of the item along the X axis.
161+
* @param pointerY Position of the item along the Y axis.
155162
*/
156-
enter(item: CdkDrag, xOffset: number, yOffset: number): void {
163+
enter(item: CdkDrag, pointerX: number, pointerY: number): void {
157164
this.entered.emit({item, container: this});
158165
this.start();
159166

160167
// We use the coordinates of where the item entered the drop
161168
// zone to figure out at which index it should be inserted.
162-
const newIndex = this._getItemIndexFromPointerPosition(item, xOffset, yOffset);
169+
const newIndex = this._getItemIndexFromPointerPosition(item, pointerX, pointerY);
163170
const currentIndex = this._activeDraggables.indexOf(item);
164171
const newPositionReference = this._activeDraggables[newIndex];
165172
const placeholder = item.getPlaceholderElement();
@@ -211,12 +218,17 @@ export class CdkDrop<T = any> implements OnInit, OnDestroy {
211218
/**
212219
* Sorts an item inside the container based on its position.
213220
* @param item Item to be sorted.
214-
* @param xOffset Position of the item along the X axis.
215-
* @param yOffset Position of the item along the Y axis.
221+
* @param pointerX Position of the item along the X axis.
222+
* @param pointerY Position of the item along the Y axis.
216223
*/
217-
_sortItem(item: CdkDrag, xOffset: number, yOffset: number): void {
224+
_sortItem(item: CdkDrag, pointerX: number, pointerY: number): void {
225+
// Don't sort the item if it's out of range.
226+
if (!this._isPointerNearDropContainer(pointerX, pointerY)) {
227+
return;
228+
}
229+
218230
const siblings = this._positionCache.items;
219-
const newIndex = this._getItemIndexFromPointerPosition(item, xOffset, yOffset);
231+
const newIndex = this._getItemIndexFromPointerPosition(item, pointerX, pointerY);
220232

221233
if (newIndex === -1 && siblings.length > 0) {
222234
return;
@@ -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. */
@@ -351,10 +365,10 @@ export class CdkDrop<T = any> implements OnInit, OnDestroy {
351365
/**
352366
* Gets the index of an item in the drop container, based on the position of the user's pointer.
353367
* @param item Item that is being sorted.
354-
* @param xOffset Position of the user's pointer along the X axis.
355-
* @param yOffset Position of the user's pointer along the Y axis.
368+
* @param pointerX Position of the user's pointer along the X axis.
369+
* @param pointerY Position of the user's pointer along the Y axis.
356370
*/
357-
private _getItemIndexFromPointerPosition(item: CdkDrag, xOffset: number, yOffset: number) {
371+
private _getItemIndexFromPointerPosition(item: CdkDrag, pointerX: number, pointerY: number) {
358372
return this._positionCache.items.findIndex(({drag, clientRect}, _, array) => {
359373
if (drag === item) {
360374
// If there's only one item left in the container, it must be
@@ -365,8 +379,22 @@ export class CdkDrop<T = any> implements OnInit, OnDestroy {
365379
return this.orientation === 'horizontal' ?
366380
// Round these down since most browsers report client rects with
367381
// sub-pixel precision, whereas the mouse coordinates are rounded to pixels.
368-
xOffset >= Math.floor(clientRect.left) && xOffset <= Math.floor(clientRect.right) :
369-
yOffset >= Math.floor(clientRect.top) && yOffset <= Math.floor(clientRect.bottom);
382+
pointerX >= Math.floor(clientRect.left) && pointerX <= Math.floor(clientRect.right) :
383+
pointerY >= Math.floor(clientRect.top) && pointerY <= Math.floor(clientRect.bottom);
370384
});
371385
}
386+
387+
/**
388+
* Checks whether the pointer coordinates are close to the drop container.
389+
* @param pointerX Coordinates along the X axis.
390+
* @param pointerY Coordinates along the Y axis.
391+
*/
392+
private _isPointerNearDropContainer(pointerX: number, pointerY: 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 pointerY > top - yThreshold && pointerY < bottom + yThreshold &&
398+
pointerX > left - xThreshold && pointerX < right + xThreshold;
399+
}
372400
}

0 commit comments

Comments
 (0)