From c0ba25a0e8d6f2f298ed8897b596104d20debc9c Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Thu, 2 Nov 2017 04:32:31 +0100 Subject: [PATCH] fix(overlay): CloseScrollStrategy not triggering change detection on close (#7929) After some refactoring, the `ScrollDispatcher` no longer dispatches inside the `NgZone`, which caused the `CloseScrollStrategy` to break, because it detaches without triggering change detection. These changes bring the detachment back into the `NgZone`. Fixes #7922. --- .../overlay/scroll/close-scroll-strategy.spec.ts | 13 ++++++++++++- src/cdk/overlay/scroll/close-scroll-strategy.ts | 13 ++++++++----- src/cdk/overlay/scroll/scroll-strategy-options.ts | 7 ++++--- 3 files changed, 24 insertions(+), 9 deletions(-) diff --git a/src/cdk/overlay/scroll/close-scroll-strategy.spec.ts b/src/cdk/overlay/scroll/close-scroll-strategy.spec.ts index 57617f6c8bef..503c795e4326 100644 --- a/src/cdk/overlay/scroll/close-scroll-strategy.spec.ts +++ b/src/cdk/overlay/scroll/close-scroll-strategy.spec.ts @@ -1,5 +1,5 @@ import {inject, TestBed, async} from '@angular/core/testing'; -import {NgModule, Component} from '@angular/core'; +import {NgModule, Component, NgZone} from '@angular/core'; import {Subject} from 'rxjs/Subject'; import {ComponentPortal, PortalModule} from '@angular/cdk/portal'; import {ScrollDispatcher} from '@angular/cdk/scrolling'; @@ -59,6 +59,17 @@ describe('CloseScrollStrategy', () => { expect(overlayRef.detach).not.toHaveBeenCalled(); }); + it('should detach inside the NgZone', () => { + const spy = jasmine.createSpy('detachment spy'); + const subscription = overlayRef.detachments().subscribe(() => spy(NgZone.isInAngularZone())); + + overlayRef.attach(componentPortal); + scrolledSubject.next(); + + expect(spy).toHaveBeenCalledWith(true); + subscription.unsubscribe(); + }); + }); diff --git a/src/cdk/overlay/scroll/close-scroll-strategy.ts b/src/cdk/overlay/scroll/close-scroll-strategy.ts index e7defe7942da..970e3afcebe5 100644 --- a/src/cdk/overlay/scroll/close-scroll-strategy.ts +++ b/src/cdk/overlay/scroll/close-scroll-strategy.ts @@ -6,6 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ +import {NgZone} from '@angular/core'; import {ScrollStrategy, getMatScrollStrategyAlreadyAttachedError} from './scroll-strategy'; import {OverlayRef} from '../overlay-ref'; import {Subscription} from 'rxjs/Subscription'; @@ -19,7 +20,7 @@ export class CloseScrollStrategy implements ScrollStrategy { private _scrollSubscription: Subscription|null = null; private _overlayRef: OverlayRef; - constructor(private _scrollDispatcher: ScrollDispatcher) { } + constructor(private _scrollDispatcher: ScrollDispatcher, private _ngZone: NgZone) { } /** Attaches this scroll strategy to an overlay. */ attach(overlayRef: OverlayRef) { @@ -34,11 +35,13 @@ export class CloseScrollStrategy implements ScrollStrategy { enable() { if (!this._scrollSubscription) { this._scrollSubscription = this._scrollDispatcher.scrolled(0).subscribe(() => { - if (this._overlayRef.hasAttached()) { - this._overlayRef.detach(); - } + this._ngZone.run(() => { + this.disable(); - this.disable(); + if (this._overlayRef.hasAttached()) { + this._overlayRef.detach(); + } + }); }); } } diff --git a/src/cdk/overlay/scroll/scroll-strategy-options.ts b/src/cdk/overlay/scroll/scroll-strategy-options.ts index 2fb71b1e696a..07473fd51ef4 100644 --- a/src/cdk/overlay/scroll/scroll-strategy-options.ts +++ b/src/cdk/overlay/scroll/scroll-strategy-options.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {Injectable} from '@angular/core'; +import {Injectable, NgZone} from '@angular/core'; import {CloseScrollStrategy} from './close-scroll-strategy'; import {NoopScrollStrategy} from './noop-scroll-strategy'; import {BlockScrollStrategy} from './block-scroll-strategy'; @@ -28,13 +28,14 @@ import { export class ScrollStrategyOptions { constructor( private _scrollDispatcher: ScrollDispatcher, - private _viewportRuler: ViewportRuler) { } + private _viewportRuler: ViewportRuler, + private _ngZone: NgZone) { } /** Do nothing on scroll. */ noop = () => new NoopScrollStrategy(); /** Close the overlay as soon as the user scrolls. */ - close = () => new CloseScrollStrategy(this._scrollDispatcher); + close = () => new CloseScrollStrategy(this._scrollDispatcher, this._ngZone); /** Block scrolling. */ block = () => new BlockScrollStrategy(this._viewportRuler);