Skip to content

Commit db65d66

Browse files
committed
refactor: address feedback
1 parent 9f2864d commit db65d66

File tree

2 files changed

+23
-12
lines changed

2 files changed

+23
-12
lines changed

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,10 @@ describe('ViewportRuler', () => {
3333
scrollTo(0, 0);
3434
}));
3535

36+
afterEach(() => {
37+
ruler.ngOnDestroy();
38+
});
39+
3640
it('should get the viewport bounds when the page is not scrolled', () => {
3741
let bounds = ruler.getViewportRect();
3842
expect(bounds.top).toBe(0);
@@ -109,7 +113,7 @@ describe('ViewportRuler', () => {
109113
const subscription = ruler.change(0).subscribe(spy);
110114

111115
dispatchFakeEvent(window, 'resize');
112-
expect(spy).toHaveBeenCalledWith('resize');
116+
expect(spy).toHaveBeenCalled();
113117
subscription.unsubscribe();
114118
});
115119

@@ -118,7 +122,7 @@ describe('ViewportRuler', () => {
118122
const subscription = ruler.change(0).subscribe(spy);
119123

120124
dispatchFakeEvent(window, 'orientationchange');
121-
expect(spy).toHaveBeenCalledWith('orientationchange');
125+
expect(spy).toHaveBeenCalled();
122126
subscription.unsubscribe();
123127
});
124128

src/cdk/scrolling/viewport-ruler.ts

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

9-
import {Injectable, Optional, SkipSelf, NgZone} from '@angular/core';
9+
import {Injectable, Optional, SkipSelf, NgZone, OnDestroy} from '@angular/core';
1010
import {Platform} from '@angular/cdk/platform';
1111
import {ScrollDispatcher} from './scroll-dispatcher';
1212
import {Observable} from 'rxjs/Observable';
1313
import {Subject} from 'rxjs/Subject';
1414
import {fromEvent} from 'rxjs/observable/fromEvent';
1515
import {merge} from 'rxjs/observable/merge';
1616
import {auditTime} from 'rxjs/operator/auditTime';
17+
import {Subscription} from 'rxjs/Subscription';
1718

1819
/** Time in ms to throttle the resize events by default. */
1920
export const DEFAULT_RESIZE_TIME = 20;
@@ -23,27 +24,33 @@ export const DEFAULT_RESIZE_TIME = 20;
2324
* @docs-private
2425
*/
2526
@Injectable()
26-
export class ViewportRuler {
27+
export class ViewportRuler implements OnDestroy {
2728

2829
/** Cached document client rectangle. */
2930
private _documentRect?: ClientRect;
3031

3132
/** Stream of viewport change events. */
32-
private _changed = new Subject<string>();
33+
private _changed = new Subject<void>();
34+
35+
/** Subscriptions to streams that invalidate the cached viewport dimensions. */
36+
private _invalidateCacheSubscriptions: Subscription[];
3337

3438
constructor(platform: Platform, ngZone: NgZone, scrollDispatcher: ScrollDispatcher) {
3539
if (platform.isBrowser) {
36-
ngZone.runOutsideAngular(() => {
37-
merge<Event>(
38-
fromEvent(window, 'resize'),
39-
fromEvent(window, 'orientationchange')
40-
).subscribe(event => this._changed.next(event.type));
40+
this._changed = ngZone.runOutsideAngular(() => {
41+
return merge<Event>(fromEvent(window, 'resize'), fromEvent(window, 'orientationchange'));
4142
});
4243
}
4344

4445
// Subscribe to scroll and resize events and update the document rectangle on changes.
45-
scrollDispatcher.scrolled(0, () => this._cacheViewportGeometry());
46-
this.change().subscribe(() => this._cacheViewportGeometry());
46+
this._invalidateCacheSubscriptions = [
47+
scrollDispatcher.scrolled(0, () => this._cacheViewportGeometry()),
48+
this.change().subscribe(() => this._cacheViewportGeometry())
49+
];
50+
}
51+
52+
ngOnDestroy() {
53+
this._invalidateCacheSubscriptions.forEach(subscription => subscription.unsubscribe());
4754
}
4855

4956
/** Gets a ClientRect for the viewport's bounds. */

0 commit comments

Comments
 (0)