Skip to content

Commit 6d45a7c

Browse files
committed
fix(scrolling): viewport ruler change obserable run inside the zone
1 parent ef16735 commit 6d45a7c

File tree

4 files changed

+51
-24
lines changed

4 files changed

+51
-24
lines changed

src/cdk/scrolling/viewport-ruler.spec.ts

Lines changed: 29 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import {ScrollingModule} from './public-api';
33
import {ViewportRuler} from './viewport-ruler';
44
import {dispatchFakeEvent} from '@angular/cdk/testing/private';
55
import {NgZone} from '@angular/core';
6+
import {Subscription} from 'rxjs';
67

78
// For all tests, we assume the browser window is 1024x786 (outerWidth x outerHeight).
89
// The karma config has been set to this for local tests, and it is the default size
@@ -13,7 +14,8 @@ import {NgZone} from '@angular/core';
1314
// window.innerHeight in the unit test instead of hard-coded values.
1415

1516
describe('ViewportRuler', () => {
16-
let ruler: ViewportRuler;
17+
let viewportRuler: ViewportRuler;
18+
let ngZone: NgZone;
1719

1820
let startingWindowWidth = window.innerWidth;
1921
let startingWindowHeight = window.innerHeight;
@@ -28,23 +30,24 @@ describe('ViewportRuler', () => {
2830
providers: [ViewportRuler]
2931
}));
3032

31-
beforeEach(inject([ViewportRuler], (viewportRuler: ViewportRuler) => {
32-
ruler = viewportRuler;
33+
beforeEach(inject([ViewportRuler, NgZone], (v: ViewportRuler, n: NgZone) => {
34+
viewportRuler = v;
35+
ngZone = n;
3336
scrollTo(0, 0);
3437
}));
3538

3639
afterEach(() => {
37-
ruler.ngOnDestroy();
40+
viewportRuler.ngOnDestroy();
3841
});
3942

4043
it('should get the viewport size', () => {
41-
let size = ruler.getViewportSize();
44+
let size = viewportRuler.getViewportSize();
4245
expect(size.width).toBe(window.innerWidth);
4346
expect(size.height).toBe(window.innerHeight);
4447
});
4548

4649
it('should get the viewport bounds when the page is not scrolled', () => {
47-
let bounds = ruler.getViewportRect();
50+
let bounds = viewportRuler.getViewportRect();
4851
expect(bounds.top).toBe(0);
4952
expect(bounds.left).toBe(0);
5053
expect(bounds.bottom).toBe(window.innerHeight);
@@ -56,7 +59,7 @@ describe('ViewportRuler', () => {
5659

5760
scrollTo(1500, 2000);
5861

59-
let bounds = ruler.getViewportRect();
62+
let bounds = viewportRuler.getViewportRect();
6063

6164
// In the iOS simulator (BrowserStack & SauceLabs), adding the content to the
6265
// body causes karma's iframe for the test to stretch to fit that content once we attempt to
@@ -82,7 +85,7 @@ describe('ViewportRuler', () => {
8285
});
8386

8487
it('should get the scroll position when the page is not scrolled', () => {
85-
let scrollPos = ruler.getViewportScrollPosition();
88+
let scrollPos = viewportRuler.getViewportScrollPosition();
8689
expect(scrollPos.top).toBe(0);
8790
expect(scrollPos.left).toBe(0);
8891
});
@@ -102,7 +105,7 @@ describe('ViewportRuler', () => {
102105
return;
103106
}
104107

105-
let scrollPos = ruler.getViewportScrollPosition();
108+
let scrollPos = viewportRuler.getViewportScrollPosition();
106109
expect(scrollPos.top).toBe(2000);
107110
expect(scrollPos.left).toBe(1500);
108111

@@ -112,7 +115,7 @@ describe('ViewportRuler', () => {
112115
describe('changed event', () => {
113116
it('should dispatch an event when the window is resized', () => {
114117
const spy = jasmine.createSpy('viewport changed spy');
115-
const subscription = ruler.change(0).subscribe(spy);
118+
const subscription = viewportRuler.change(0).subscribe(spy);
116119

117120
dispatchFakeEvent(window, 'resize');
118121
expect(spy).toHaveBeenCalled();
@@ -121,7 +124,7 @@ describe('ViewportRuler', () => {
121124

122125
it('should dispatch an event when the orientation is changed', () => {
123126
const spy = jasmine.createSpy('viewport changed spy');
124-
const subscription = ruler.change(0).subscribe(spy);
127+
const subscription = viewportRuler.change(0).subscribe(spy);
125128

126129
dispatchFakeEvent(window, 'orientationchange');
127130
expect(spy).toHaveBeenCalled();
@@ -130,7 +133,7 @@ describe('ViewportRuler', () => {
130133

131134
it('should be able to throttle the callback', fakeAsync(() => {
132135
const spy = jasmine.createSpy('viewport changed spy');
133-
const subscription = ruler.change(1337).subscribe(spy);
136+
const subscription = viewportRuler.change(1337).subscribe(spy);
134137

135138
dispatchFakeEvent(window, 'resize');
136139
expect(spy).not.toHaveBeenCalled();
@@ -143,12 +146,25 @@ describe('ViewportRuler', () => {
143146

144147
it('should run the resize event outside the NgZone', () => {
145148
const spy = jasmine.createSpy('viewport changed spy');
146-
const subscription = ruler.change(0).subscribe(() => spy(NgZone.isInAngularZone()));
149+
const subscription = viewportRuler.change(0).subscribe(() => spy(NgZone.isInAngularZone()));
147150

148151
dispatchFakeEvent(window, 'resize');
149152
expect(spy).toHaveBeenCalledWith(false);
150153
subscription.unsubscribe();
151154
});
152155

156+
it('should run events outside of the NgZone, even if the subcription is from inside', () => {
157+
const spy = jasmine.createSpy('viewport changed spy');
158+
let subscription: Subscription;
159+
160+
ngZone.run(() => {
161+
subscription = viewportRuler.change(0).subscribe(() => spy(NgZone.isInAngularZone()));
162+
dispatchFakeEvent(window, 'resize');
163+
});
164+
165+
expect(spy).toHaveBeenCalledWith(false);
166+
subscription!.unsubscribe();
167+
});
168+
153169
});
154170
});

src/cdk/scrolling/viewport-ruler.ts

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,15 @@ export interface ViewportScrollPosition {
2020
left: number;
2121
}
2222

23+
/** Runs this observable outside the zone */
24+
export function runOutsideZone<T>(zone: NgZone): any {
25+
return (source: Observable<T>): Observable<T> => {
26+
return new Observable(observer => {
27+
return zone.runOutsideAngular<Subscription>(() => source.subscribe(observer));
28+
});
29+
};
30+
}
31+
2332
/**
2433
* Simple utility for getting the bounds of the browser viewport.
2534
* @docs-private
@@ -36,15 +45,12 @@ export class ViewportRuler implements OnDestroy {
3645
private _invalidateCache: Subscription;
3746

3847
constructor(private _platform: Platform, ngZone: NgZone) {
39-
ngZone.runOutsideAngular(() => {
40-
this._change = _platform.isBrowser ?
41-
merge(fromEvent(window, 'resize'), fromEvent(window, 'orientationchange')) :
42-
observableOf();
48+
this._change = _platform.isBrowser ?
49+
merge(fromEvent(window, 'resize'), fromEvent(window, 'orientationchange'))
50+
.pipe(runOutsideZone(ngZone)) :
51+
observableOf();
4352

44-
// Note that we need to do the subscription inside `runOutsideAngular`
45-
// since subscribing is what causes the event listener to be added.
4653
this._invalidateCache = this.change().subscribe(() => this._updateViewportSize());
47-
});
4854
}
4955

5056
ngOnDestroy() {

src/material/select/select.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -572,8 +572,11 @@ export class MatSelect extends _MatSelectMixinBase implements AfterContentInit,
572572
.pipe(takeUntil(this._destroy))
573573
.subscribe(() => {
574574
if (this._panelOpen) {
575-
this._triggerRect = this.trigger.nativeElement.getBoundingClientRect();
576-
this._changeDetectorRef.markForCheck();
575+
// Run inside the zone because viewport ruler change() event is outside it.
576+
this._ngZone.run(() => {
577+
this._triggerRect = this.trigger.nativeElement.getBoundingClientRect();
578+
this._changeDetectorRef.markForCheck();
579+
});
577580
}
578581
});
579582
}

src/material/tabs/paginated-tab-header.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -204,8 +204,10 @@ export abstract class MatPaginatedTabHeader implements AfterContentChecked, Afte
204204
// On dir change or window resize, realign the ink bar and update the orientation of
205205
// the key manager if the direction has changed.
206206
merge(dirChange, resize, this._items.changes).pipe(takeUntil(this._destroyed)).subscribe(() => {
207-
realign();
208-
this._keyManager.withHorizontalOrientation(this._getLayoutDirection());
207+
this._ngZone.run(() => {
208+
realign();
209+
this._keyManager.withHorizontalOrientation(this._getLayoutDirection());
210+
});
209211
});
210212

211213
// If there is a change in the focus key manager we need to emit the `indexFocused`

0 commit comments

Comments
 (0)