From 520940422194026ec6667eaeb86f87ad868df007 Mon Sep 17 00:00:00 2001 From: crisbeto Date: Fri, 4 Jan 2019 17:30:38 +0200 Subject: [PATCH] fix(drag-drop): dragging styling not being reset in some cases with OnPush change detection 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. --- src/cdk/drag-drop/directives/drag.spec.ts | 45 +++++++++++++++++++++++ src/cdk/drag-drop/directives/drop-list.ts | 13 ++++--- 2 files changed, 52 insertions(+), 6 deletions(-) diff --git a/src/cdk/drag-drop/directives/drag.spec.ts b/src/cdk/drag-drop/directives/drag.spec.ts index 64db4880263d..53b2f5e04b23 100644 --- a/src/cdk/drag-drop/directives/drag.spec.ts +++ b/src/cdk/drag-drop/directives/drag.spec.ts @@ -844,6 +844,30 @@ describe('CdkDrag', () => { expect(dropZone.element.nativeElement.classList).not.toContain('cdk-drop-dragging'); })); + it('should toggle the drop dragging classes if there is nothing to trigger change detection', + fakeAsync(() => { + const fixture = createComponent(DraggableInDropZoneWithoutEvents); + fixture.detectChanges(); + const item = fixture.componentInstance.dragItems.toArray()[1].element.nativeElement; + const dropZone = fixture.componentInstance.dropInstance; + + expect(dropZone.element.nativeElement.classList).not.toContain('cdk-drop-list-dragging'); + expect(item.classList).not.toContain('cdk-drag-dragging'); + + startDraggingViaMouse(fixture, item); + + expect(dropZone.element.nativeElement.classList).toContain('cdk-drop-list-dragging'); + expect(item.classList).toContain('cdk-drag-dragging'); + + dispatchMouseEvent(document, 'mouseup'); + fixture.detectChanges(); + flush(); + fixture.detectChanges(); + + expect(dropZone.element.nativeElement.classList).not.toContain('cdk-drop-dragging'); + expect(item.classList).not.toContain('cdk-drag-dragging'); + })); + it('should toggle a class when the user starts dragging an item with OnPush change detection', fakeAsync(() => { const fixture = createComponent(DraggableInOnPushDropZone); @@ -3115,6 +3139,27 @@ class NestedDropListGroups { class DraggableOnNgContainer {} +@Component({ + changeDetection: ChangeDetectionStrategy.OnPush, + template: ` +
+
{{item.value}}
+
+ ` +}) +class DraggableInDropZoneWithoutEvents { + @ViewChildren(CdkDrag) dragItems: QueryList; + @ViewChild(CdkDropList) dropInstance: CdkDropList; + items = [ + {value: 'Zero', height: ITEM_HEIGHT}, + {value: 'One', height: ITEM_HEIGHT}, + {value: 'Two', height: ITEM_HEIGHT}, + {value: 'Three', height: ITEM_HEIGHT} + ]; +} /** * Component that passes through whatever content is projected into it. diff --git a/src/cdk/drag-drop/directives/drop-list.ts b/src/cdk/drag-drop/directives/drop-list.ts index 0c0549eaf401..0e0197508c81 100644 --- a/src/cdk/drag-drop/directives/drop-list.ts +++ b/src/cdk/drag-drop/directives/drop-list.ts @@ -151,7 +151,7 @@ export class CdkDropList implements CdkDropListContainer, OnDestroy { return this.enterPredicate(drag.data, drop.data); }; this._syncInputs(ref); - this._proxyEvents(ref); + this._handleEvents(ref); CdkDropList._dropLists.push(this); if (_group) { @@ -275,11 +275,8 @@ export class CdkDropList implements CdkDropListContainer, OnDestroy { }); } - /** - * Proxies the events from a DropListRef to events that - * match the interfaces of the CdkDropList outputs. - */ - private _proxyEvents(ref: DropListRef) { + /** Handles events from the underlying DropListRef. */ + private _handleEvents(ref: DropListRef) { ref.beforeStarted.subscribe(() => { this._changeDetectorRef.markForCheck(); }); @@ -316,6 +313,10 @@ export class CdkDropList implements CdkDropListContainer, OnDestroy { item: event.item.data, isPointerOverContainer: event.isPointerOverContainer }); + + // Mark for check since all of these events run outside of change + // detection and we're not guaranteed for something else to have triggered it. + this._changeDetectorRef.markForCheck(); }); }