Skip to content

Commit 22afc37

Browse files
committed
fix(drag-drop): error when dragging items inside transplanted views with OnPush change detection
This is a bit of an edge case, but nevertheless it's something that can happen. The way the dragging sequence works is that the drop list keeps track of its items via `ContentChildren` and it registers itself with them once they're picked up by the `QueryList`. If an item doesn't have a container when the drag sequence is started, it is considered a "standalone" drag. This becomes a problem with transplanted views (e.g. ones created by `cdk-table`) that use OnPush where we could end up in a situation where the items aren't picked up during the first pass, but when the user starts dragging, we trigger change detection and the `QueryList` is updated, but at that point it's too late, because we've already started the dragging sequence with a different set of information. These changes work around the issue by detecting when we're in this particular setup and triggering a second change detection which should refresh the `QueryList`. Fixes #18341.
1 parent 8865e78 commit 22afc37

File tree

4 files changed

+41
-0
lines changed

4 files changed

+41
-0
lines changed

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

+12
Original file line numberDiff line numberDiff line change
@@ -3624,6 +3624,18 @@ describe('CdkDrag', () => {
36243624
expect(styles.scrollSnapType || styles.msScrollSnapType).toBe('block');
36253625
}));
36263626

3627+
it('should notify the drop list when an item has been added', fakeAsync(() => {
3628+
const fixture = createComponent(DraggableInDropZone);
3629+
fixture.detectChanges();
3630+
const list = fixture.componentInstance.dropInstance;
3631+
spyOn(list, '_itemAdded');
3632+
3633+
fixture.componentInstance.items.push({value: 'extra', height: ITEM_HEIGHT, margin: 0});
3634+
fixture.detectChanges();
3635+
3636+
expect(list._itemAdded).toHaveBeenCalled();
3637+
}));
3638+
36273639
});
36283640

36293641
describe('in a connected drop container', () => {

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

+4
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,10 @@ export class CdkDrag<T = any> implements AfterViewInit, OnChanges, OnDestroy {
206206
this._assignDefaults(config);
207207
}
208208

209+
if (dropContainer) {
210+
dropContainer._itemAdded(this);
211+
}
212+
209213
this._syncInputs(this._dragRef);
210214
this._handleEvents(this._dragRef);
211215
}

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

+24
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import {
2121
SkipSelf,
2222
AfterContentInit,
2323
Inject,
24+
NgZone,
2425
} from '@angular/core';
2526
import {Directionality} from '@angular/cdk/bidi';
2627
import {ScrollDispatcher} from '@angular/cdk/scrolling';
@@ -68,6 +69,18 @@ export class CdkDropList<T = any> implements AfterContentInit, OnDestroy {
6869
/** Keeps track of the drop lists that are currently on the page. */
6970
private static _dropLists: CdkDropList[] = [];
7071

72+
/**
73+
* In some cases (mainly transplanted views with OnPush, see #18341) we may end up in a situation
74+
* where there are no items on the first change detection pass, but the items get picked up as
75+
* soon as the user triggers another pass by dragging. This is a problem, because the item would
76+
* have to switch from standalone mode to drag mode in the middle of the dragging sequence which
77+
* is too late since the two modes save different kinds of information. We can detect when the
78+
* app is in this state by checking whether an item found the list via DI, but isn't part of the
79+
* `_draggables` list. When that happens we need to trigger another change detection pass so that
80+
* it gets picked up. We use this flag to ensure the extra pass doesn't happen more than once.
81+
*/
82+
private _hasVerifiedItemState: boolean;
83+
7184
/** Reference to the underlying drop list instance. */
7285
_dropListRef: DropListRef<CdkDropList<T>>;
7386

@@ -275,6 +288,17 @@ export class CdkDropList<T = any> implements AfterContentInit, OnDestroy {
275288
return this._dropListRef.getItemIndex(item._dragRef);
276289
}
277290

291+
/** Used by the drag items to notify the list that they've been created. */
292+
_itemAdded(item: CdkDrag) {
293+
if (!this._hasVerifiedItemState) {
294+
this._hasVerifiedItemState = true;
295+
296+
if (this._draggables && this._draggables.toArray().indexOf(item) === -1) {
297+
Promise.resolve().then(() => this._changeDetectorRef.markForCheck());
298+
}
299+
}
300+
}
301+
278302
/** Syncs the inputs of the CdkDropList with the options of the underlying DropListRef. */
279303
private _setupInputSyncSubscription(ref: DropListRef<CdkDropList>) {
280304
if (this._dir) {

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

+1
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,7 @@ export declare class CdkDropList<T = any> implements AfterContentInit, OnDestroy
163163
constructor(
164164
element: ElementRef<HTMLElement>, dragDrop: DragDrop, _changeDetectorRef: ChangeDetectorRef, _dir?: Directionality | undefined, _group?: CdkDropListGroup<CdkDropList<any>> | undefined,
165165
_scrollDispatcher?: ScrollDispatcher | undefined, config?: DragDropConfig);
166+
_itemAdded(item: CdkDrag): void;
166167
drop(item: CdkDrag, currentIndex: number, previousContainer: CdkDropList, isPointerOverContainer: boolean): void;
167168
enter(item: CdkDrag, pointerX: number, pointerY: number): void;
168169
exit(item: CdkDrag): void;

0 commit comments

Comments
 (0)