Skip to content

Commit c535444

Browse files
crisbetommalerba
crisbeto
authored and
mmalerba
committed
fix(cdk/drag-drop): account for enterPredicate when setting receiving class (#21346)
Currently we set a class when a container is able to receive the dragged item, however we add this class to all connected containers without accounting for other things that could prevent the item from entering, like `enterPredicate`. These changes add some logic to account for the predicate when setting the class. Fixes #21171. (cherry picked from commit 411b174)
1 parent 2e13216 commit c535444

3 files changed

Lines changed: 105 additions & 51 deletions

File tree

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

Lines changed: 63 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -4483,33 +4483,6 @@ describe('CdkDrag', () => {
44834483
expect(spy).toHaveBeenCalledWith(dragItem, dropInstances[1]);
44844484
}));
44854485

4486-
it('should not call the `enterPredicate` if the pointer is not over the container',
4487-
fakeAsync(() => {
4488-
const fixture = createComponent(ConnectedDropZones);
4489-
fixture.detectChanges();
4490-
4491-
const dropInstances = fixture.componentInstance.dropInstances.toArray();
4492-
const spy = jasmine.createSpy('enterPredicate spy').and.returnValue(true);
4493-
const groups = fixture.componentInstance.groupedDragItems.slice();
4494-
const dragElement = groups[0][1].element.nativeElement;
4495-
const targetRect = groups[1][2].element.nativeElement.getBoundingClientRect();
4496-
4497-
dropInstances[1].enterPredicate = spy;
4498-
fixture.detectChanges();
4499-
4500-
startDraggingViaMouse(fixture, dragElement);
4501-
4502-
dispatchMouseEvent(document, 'mousemove', targetRect.left - 1, targetRect.top - 1);
4503-
fixture.detectChanges();
4504-
4505-
expect(spy).not.toHaveBeenCalled();
4506-
4507-
dispatchMouseEvent(document, 'mousemove', targetRect.left + 1, targetRect.top + 1);
4508-
fixture.detectChanges();
4509-
4510-
expect(spy).toHaveBeenCalledTimes(1);
4511-
}));
4512-
45134486
it('should be able to start dragging after an item has been transferred', fakeAsync(() => {
45144487
const fixture = createComponent(ConnectedDropZones);
45154488
fixture.detectChanges();
@@ -5009,14 +4982,75 @@ describe('CdkDrag', () => {
50094982
expect(dropZones[0].classList)
50104983
.toContain(
50114984
'cdk-drop-list-receiving',
5012-
'Expected old container not to have the receiving class after exiting.');
4985+
'Expected old container to have the receiving class after exiting.');
50134986

50144987
expect(dropZones[1].classList)
50154988
.not.toContain(
50164989
'cdk-drop-list-receiving',
5017-
'Expected new container not to have the receiving class after entering.');
4990+
'Expected new container not to have the receiving class after exiting.');
50184991
}));
50194992

4993+
it('should not set the receiving class if the item does not match the enter predicate',
4994+
fakeAsync(() => {
4995+
const fixture = createComponent(ConnectedDropZones);
4996+
fixture.detectChanges();
4997+
fixture.componentInstance.dropInstances.toArray()[1].enterPredicate = () => false;
4998+
4999+
const dropZones =
5000+
fixture.componentInstance.dropInstances.map(d => d.element.nativeElement);
5001+
const item = fixture.componentInstance.groupedDragItems[0][1];
5002+
5003+
expect(dropZones.every(c => !c.classList.contains('cdk-drop-list-receiving')))
5004+
.toBe(true, 'Expected neither of the containers to have the class.');
5005+
5006+
startDraggingViaMouse(fixture, item.element.nativeElement);
5007+
fixture.detectChanges();
5008+
5009+
expect(dropZones.every(c => !c.classList.contains('cdk-drop-list-receiving')))
5010+
.toBe(true, 'Expected neither of the containers to have the class.');
5011+
}));
5012+
5013+
it('should set the receiving class on the source container, even if the enter predicate ' +
5014+
'does not match', fakeAsync(() => {
5015+
const fixture = createComponent(ConnectedDropZones);
5016+
fixture.detectChanges();
5017+
fixture.componentInstance.dropInstances.toArray()[0].enterPredicate = () => false;
5018+
5019+
const groups = fixture.componentInstance.groupedDragItems;
5020+
const dropZones =
5021+
fixture.componentInstance.dropInstances.map(d => d.element.nativeElement);
5022+
const item = groups[0][1];
5023+
const targetRect = groups[1][2].element.nativeElement.getBoundingClientRect();
5024+
5025+
expect(dropZones.every(c => !c.classList.contains('cdk-drop-list-receiving')))
5026+
.toBe(true, 'Expected neither of the containers to have the class.');
5027+
5028+
startDraggingViaMouse(fixture, item.element.nativeElement);
5029+
5030+
expect(dropZones[0].classList)
5031+
.not.toContain(
5032+
'cdk-drop-list-receiving',
5033+
'Expected source container not to have the receiving class.');
5034+
5035+
expect(dropZones[1].classList)
5036+
.toContain(
5037+
'cdk-drop-list-receiving',
5038+
'Expected target container to have the receiving class.');
5039+
5040+
dispatchMouseEvent(document, 'mousemove', targetRect.left + 1, targetRect.top + 1);
5041+
fixture.detectChanges();
5042+
5043+
expect(dropZones[0].classList)
5044+
.toContain(
5045+
'cdk-drop-list-receiving',
5046+
'Expected old container to have the receiving class after exiting.');
5047+
5048+
expect(dropZones[1].classList)
5049+
.not.toContain(
5050+
'cdk-drop-list-receiving',
5051+
'Expected new container not to have the receiving class after exiting.');
5052+
}));
5053+
50205054
it('should be able to move the item over an intermediate container before ' +
50215055
'dropping it into the final one', fakeAsync(() => {
50225056
const fixture = createComponent(ConnectedDropZones);

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

Lines changed: 41 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ export class DropListRef<T = any> {
146146
private _parentPositions: ParentPositionTracker;
147147

148148
/** Cached `ClientRect` of the drop list. */
149-
private _clientRect: ClientRect;
149+
private _clientRect: ClientRect | undefined;
150150

151151
/**
152152
* Draggable items that are currently active inside the container. Includes the items
@@ -163,7 +163,7 @@ export class DropListRef<T = any> {
163163
private _previousSwap = {drag: null as DragRef | null, delta: 0, overlaps: false};
164164

165165
/** Draggable items in the container. */
166-
private _draggables: ReadonlyArray<DragRef>;
166+
private _draggables: ReadonlyArray<DragRef> = [];
167167

168168
/** Drop lists that are connected to the current one. */
169169
private _siblings: ReadonlyArray<DropListRef> = [];
@@ -240,19 +240,8 @@ export class DropListRef<T = any> {
240240

241241
/** Starts dragging an item. */
242242
start(): void {
243-
const styles = coerceElement(this.element).style as DragCSSStyleDeclaration;
244-
this.beforeStarted.next();
245-
this._isDragging = true;
246-
247-
// We need to disable scroll snapping while the user is dragging, because it breaks automatic
248-
// scrolling. The browser seems to round the value based on the snapping points which means
249-
// that we can't increment/decrement the scroll position.
250-
this._initialScrollSnap = styles.msScrollSnapType || styles.scrollSnapType || '';
251-
styles.scrollSnapType = styles.msScrollSnapType = 'none';
252-
this._cacheItems();
253-
this._siblings.forEach(sibling => sibling._startReceiving(this));
254-
this._viewportScrollSubscription.unsubscribe();
255-
this._listenToScrollEvents();
243+
this._draggingStarted();
244+
this._notifyReceivingSiblings();
256245
}
257246

258247
/**
@@ -264,7 +253,7 @@ export class DropListRef<T = any> {
264253
* out automatically.
265254
*/
266255
enter(item: DragRef, pointerX: number, pointerY: number, index?: number): void {
267-
this.start();
256+
this._draggingStarted();
268257

269258
// If sorting is disabled, we want the item to return to its starting
270259
// position if the user is returning it to its initial container.
@@ -323,6 +312,8 @@ export class DropListRef<T = any> {
323312
this._cacheItemPositions();
324313
this._cacheParentPositions();
325314

315+
// Notify siblings at the end so that the item has been inserted into the `activeDraggables`.
316+
this._notifyReceivingSiblings();
326317
this.entered.next({item, container: this, currentIndex: this.getItemIndex(item)});
327318
}
328319

@@ -463,7 +454,7 @@ export class DropListRef<T = any> {
463454
_sortItem(item: DragRef, pointerX: number, pointerY: number,
464455
pointerDelta: {x: number, y: number}): void {
465456
// Don't sort the item if sorting is disabled or it's out of range.
466-
if (this.sortingDisabled ||
457+
if (this.sortingDisabled || !this._clientRect ||
467458
!isPointerNearClientRect(this._clientRect, DROP_PROXIMITY_THRESHOLD, pointerX, pointerY)) {
468459
return;
469460
}
@@ -600,6 +591,22 @@ export class DropListRef<T = any> {
600591
this._stopScrollTimers.next();
601592
}
602593

594+
/** Starts the dragging sequence within the list. */
595+
private _draggingStarted() {
596+
const styles = coerceElement(this.element).style as DragCSSStyleDeclaration;
597+
this.beforeStarted.next();
598+
this._isDragging = true;
599+
600+
// We need to disable scroll snapping while the user is dragging, because it breaks automatic
601+
// scrolling. The browser seems to round the value based on the snapping points which means
602+
// that we can't increment/decrement the scroll position.
603+
this._initialScrollSnap = styles.msScrollSnapType || styles.scrollSnapType || '';
604+
styles.scrollSnapType = styles.msScrollSnapType = 'none';
605+
this._cacheItems();
606+
this._viewportScrollSubscription.unsubscribe();
607+
this._listenToScrollEvents();
608+
}
609+
603610
/** Caches the positions of the configured scrollable parents. */
604611
private _cacheParentPositions() {
605612
const element = coerceElement(this.element);
@@ -802,7 +809,7 @@ export class DropListRef<T = any> {
802809
* @param y Pointer position along the Y axis.
803810
*/
804811
_isOverContainer(x: number, y: number): boolean {
805-
return isInsideClientRect(this._clientRect, x, y);
812+
return this._clientRect != null && isInsideClientRect(this._clientRect, x, y);
806813
}
807814

808815
/**
@@ -823,7 +830,8 @@ export class DropListRef<T = any> {
823830
* @param y Position of the item along the Y axis.
824831
*/
825832
_canReceive(item: DragRef, x: number, y: number): boolean {
826-
if (!isInsideClientRect(this._clientRect, x, y) || !this.enterPredicate(item, this)) {
833+
if (!this._clientRect || !isInsideClientRect(this._clientRect, x, y) ||
834+
!this.enterPredicate(item, this)) {
827835
return false;
828836
}
829837

@@ -850,10 +858,16 @@ export class DropListRef<T = any> {
850858
* Called by one of the connected drop lists when a dragging sequence has started.
851859
* @param sibling Sibling in which dragging has started.
852860
*/
853-
_startReceiving(sibling: DropListRef) {
861+
_startReceiving(sibling: DropListRef, items: DragRef[]) {
854862
const activeSiblings = this._activeSiblings;
855863

856-
if (!activeSiblings.has(sibling)) {
864+
if (!activeSiblings.has(sibling) && items.every(item => {
865+
// Note that we have to add an exception to the `enterPredicate` for items that started off
866+
// in this drop list. The drag ref has logic that allows an item to return to its initial
867+
// container, if it has left the initial container and none of the connected containers
868+
// allow it to enter. See `DragRef._updateActiveDropContainer` for more context.
869+
return this.enterPredicate(item, this) || this._draggables.indexOf(item) > -1;
870+
})) {
857871
activeSiblings.add(sibling);
858872
this._cacheParentPositions();
859873
this._listenToScrollEvents();
@@ -917,6 +931,12 @@ export class DropListRef<T = any> {
917931

918932
return this._cachedShadowRoot;
919933
}
934+
935+
/** Notifies any siblings that may potentially receive the item. */
936+
private _notifyReceivingSiblings() {
937+
const draggedItems = this._activeDraggables.filter(item => item.isDragging());
938+
this._siblings.forEach(sibling => sibling._startReceiving(this, draggedItems));
939+
}
920940
}
921941

922942

‎tools/public_api_guard/cdk/drag-drop.d.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -374,7 +374,7 @@ export declare class DropListRef<T = any> {
374374
x: number;
375375
y: number;
376376
}): void;
377-
_startReceiving(sibling: DropListRef): void;
377+
_startReceiving(sibling: DropListRef, items: DragRef[]): void;
378378
_startScrollingIfNecessary(pointerX: number, pointerY: number): void;
379379
_stopReceiving(sibling: DropListRef): void;
380380
_stopScrolling(): void;

0 commit comments

Comments
 (0)