Skip to content

Commit 49b74a7

Browse files
crisbetoVivian Hu
authored and
Vivian Hu
committedJan 18, 2019
fix(drag-drop): dragging styling not being reset in some cases with OnPush change detection (#14725)
Fixes the `cdk-drop-list-dragging` and `cdk-drop-list-receiving` not being removed in some cases when using `OnPush` change detection. This seems to have been working until now, because there's always something else to trigger change detection, however if there isn't (e.g. the user isn't listening to the `dropped` event) they won't be reset.
1 parent 677b9c8 commit 49b74a7

File tree

2 files changed

+52
-6
lines changed

2 files changed

+52
-6
lines changed
 

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

+45
Original file line numberDiff line numberDiff line change
@@ -844,6 +844,30 @@ describe('CdkDrag', () => {
844844
expect(dropZone.element.nativeElement.classList).not.toContain('cdk-drop-dragging');
845845
}));
846846

847+
it('should toggle the drop dragging classes if there is nothing to trigger change detection',
848+
fakeAsync(() => {
849+
const fixture = createComponent(DraggableInDropZoneWithoutEvents);
850+
fixture.detectChanges();
851+
const item = fixture.componentInstance.dragItems.toArray()[1].element.nativeElement;
852+
const dropZone = fixture.componentInstance.dropInstance;
853+
854+
expect(dropZone.element.nativeElement.classList).not.toContain('cdk-drop-list-dragging');
855+
expect(item.classList).not.toContain('cdk-drag-dragging');
856+
857+
startDraggingViaMouse(fixture, item);
858+
859+
expect(dropZone.element.nativeElement.classList).toContain('cdk-drop-list-dragging');
860+
expect(item.classList).toContain('cdk-drag-dragging');
861+
862+
dispatchMouseEvent(document, 'mouseup');
863+
fixture.detectChanges();
864+
flush();
865+
fixture.detectChanges();
866+
867+
expect(dropZone.element.nativeElement.classList).not.toContain('cdk-drop-dragging');
868+
expect(item.classList).not.toContain('cdk-drag-dragging');
869+
}));
870+
847871
it('should toggle a class when the user starts dragging an item with OnPush change detection',
848872
fakeAsync(() => {
849873
const fixture = createComponent(DraggableInOnPushDropZone);
@@ -3115,6 +3139,27 @@ class NestedDropListGroups {
31153139
class DraggableOnNgContainer {}
31163140

31173141

3142+
@Component({
3143+
changeDetection: ChangeDetectionStrategy.OnPush,
3144+
template: `
3145+
<div cdkDropList style="width: 100px; background: pink;">
3146+
<div *ngFor="let item of items"
3147+
cdkDrag
3148+
[style.height.px]="item.height"
3149+
style="width: 100%; background: red;">{{item.value}}</div>
3150+
</div>
3151+
`
3152+
})
3153+
class DraggableInDropZoneWithoutEvents {
3154+
@ViewChildren(CdkDrag) dragItems: QueryList<CdkDrag>;
3155+
@ViewChild(CdkDropList) dropInstance: CdkDropList;
3156+
items = [
3157+
{value: 'Zero', height: ITEM_HEIGHT},
3158+
{value: 'One', height: ITEM_HEIGHT},
3159+
{value: 'Two', height: ITEM_HEIGHT},
3160+
{value: 'Three', height: ITEM_HEIGHT}
3161+
];
3162+
}
31183163

31193164
/**
31203165
* Component that passes through whatever content is projected into it.

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

+7-6
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ export class CdkDropList<T = any> implements CdkDropListContainer, OnDestroy {
151151
return this.enterPredicate(drag.data, drop.data);
152152
};
153153
this._syncInputs(ref);
154-
this._proxyEvents(ref);
154+
this._handleEvents(ref);
155155
CdkDropList._dropLists.push(this);
156156

157157
if (_group) {
@@ -275,11 +275,8 @@ export class CdkDropList<T = any> implements CdkDropListContainer, OnDestroy {
275275
});
276276
}
277277

278-
/**
279-
* Proxies the events from a DropListRef to events that
280-
* match the interfaces of the CdkDropList outputs.
281-
*/
282-
private _proxyEvents(ref: DropListRef<CdkDropList>) {
278+
/** Handles events from the underlying DropListRef. */
279+
private _handleEvents(ref: DropListRef<CdkDropList>) {
283280
ref.beforeStarted.subscribe(() => {
284281
this._changeDetectorRef.markForCheck();
285282
});
@@ -316,6 +313,10 @@ export class CdkDropList<T = any> implements CdkDropListContainer, OnDestroy {
316313
item: event.item.data,
317314
isPointerOverContainer: event.isPointerOverContainer
318315
});
316+
317+
// Mark for check since all of these events run outside of change
318+
// detection and we're not guaranteed for something else to have triggered it.
319+
this._changeDetectorRef.markForCheck();
319320
});
320321
}
321322

0 commit comments

Comments
 (0)
Please sign in to comment.