Skip to content

Commit 02c255c

Browse files
committed
fix(overlay): CloseScrollStrategy not triggering change detection on close
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`.
1 parent fe08c2b commit 02c255c

File tree

3 files changed

+24
-9
lines changed

3 files changed

+24
-9
lines changed

src/cdk/overlay/scroll/close-scroll-strategy.spec.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import {inject, TestBed, async} from '@angular/core/testing';
2-
import {NgModule, Component} from '@angular/core';
2+
import {NgModule, Component, NgZone} from '@angular/core';
33
import {Subject} from 'rxjs/Subject';
44
import {ComponentPortal, PortalModule} from '@angular/cdk/portal';
55
import {ScrollDispatcher} from '@angular/cdk/scrolling';
@@ -59,6 +59,17 @@ describe('CloseScrollStrategy', () => {
5959
expect(overlayRef.detach).not.toHaveBeenCalled();
6060
});
6161

62+
it('should detach inside the NgZone', () => {
63+
const spy = jasmine.createSpy('detachment spy');
64+
const subscription = overlayRef.detachments().subscribe(() => spy(NgZone.isInAngularZone()));
65+
66+
overlayRef.attach(componentPortal);
67+
scrolledSubject.next();
68+
69+
expect(spy).toHaveBeenCalledWith(true);
70+
subscription.unsubscribe();
71+
});
72+
6273
});
6374

6475

src/cdk/overlay/scroll/close-scroll-strategy.ts

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9+
import {NgZone} from '@angular/core';
910
import {ScrollStrategy, getMatScrollStrategyAlreadyAttachedError} from './scroll-strategy';
1011
import {OverlayRef} from '../overlay-ref';
1112
import {Subscription} from 'rxjs/Subscription';
@@ -19,7 +20,7 @@ export class CloseScrollStrategy implements ScrollStrategy {
1920
private _scrollSubscription: Subscription|null = null;
2021
private _overlayRef: OverlayRef;
2122

22-
constructor(private _scrollDispatcher: ScrollDispatcher) { }
23+
constructor(private _scrollDispatcher: ScrollDispatcher, private _ngZone: NgZone) { }
2324

2425
attach(overlayRef: OverlayRef) {
2526
if (this._overlayRef) {
@@ -32,11 +33,13 @@ export class CloseScrollStrategy implements ScrollStrategy {
3233
enable() {
3334
if (!this._scrollSubscription) {
3435
this._scrollSubscription = this._scrollDispatcher.scrolled(0).subscribe(() => {
35-
if (this._overlayRef.hasAttached()) {
36-
this._overlayRef.detach();
37-
}
36+
this._ngZone.run(() => {
37+
this.disable();
3838

39-
this.disable();
39+
if (this._overlayRef.hasAttached()) {
40+
this._overlayRef.detach();
41+
}
42+
});
4043
});
4144
}
4245
}

src/cdk/overlay/scroll/scroll-strategy-options.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import {Injectable} from '@angular/core';
9+
import {Injectable, NgZone} from '@angular/core';
1010
import {CloseScrollStrategy} from './close-scroll-strategy';
1111
import {NoopScrollStrategy} from './noop-scroll-strategy';
1212
import {BlockScrollStrategy} from './block-scroll-strategy';
@@ -28,13 +28,14 @@ import {
2828
export class ScrollStrategyOptions {
2929
constructor(
3030
private _scrollDispatcher: ScrollDispatcher,
31-
private _viewportRuler: ViewportRuler) { }
31+
private _viewportRuler: ViewportRuler,
32+
private _ngZone: NgZone) { }
3233

3334
/** Do nothing on scroll. */
3435
noop = () => new NoopScrollStrategy();
3536

3637
/** Close the overlay as soon as the user scrolls. */
37-
close = () => new CloseScrollStrategy(this._scrollDispatcher);
38+
close = () => new CloseScrollStrategy(this._scrollDispatcher, this._ngZone);
3839

3940
/** Block scrolling. */
4041
block = () => new BlockScrollStrategy(this._viewportRuler);

0 commit comments

Comments
 (0)