refactor(scroll-dispatcher): remove the need to pass in a callback to the scrolled method (#6679)

Author: crisbeto

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

@@ -21,11 +21,9 @@ describe('CloseScrollStrategy', () => {
     TestBed.configureTestingModule({
       imports: [OverlayModule, PortalModule, OverlayTestModule],
       providers: [
-        {provide: ScrollDispatcher, useFactory: () => {
-          return {scrolled: (_delay: number, callback: () => any) => {
-            return scrolledSubject.asObservable().subscribe(callback);
-          }};
-        }}
+        {provide: ScrollDispatcher, useFactory: () => ({
+          scrolled: () => scrolledSubject.asObservable()
+        })}
       ]
     });
 

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

@@ -31,7 +31,7 @@ export class CloseScrollStrategy implements ScrollStrategy {
 
   enable() {
     if (!this._scrollSubscription) {
-      this._scrollSubscription = this._scrollDispatcher.scrolled(0, () => {
+      this._scrollSubscription = this._scrollDispatcher.scrolled(0).subscribe(() => {
         if (this._overlayRef.hasAttached()) {
           this._overlayRef.detach();
         }

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

@@ -21,11 +21,9 @@ describe('RepositionScrollStrategy', () => {
     TestBed.configureTestingModule({
       imports: [OverlayModule, PortalModule, OverlayTestModule],
       providers: [
-        {provide: ScrollDispatcher, useFactory: () => {
-          return {scrolled: (_delay: number, callback: () => any) => {
-            return scrolledSubject.asObservable().subscribe(callback);
-          }};
-        }}
+        {provide: ScrollDispatcher, useFactory: () => ({
+          scrolled: () => scrolledSubject.asObservable()
+        })}
       ]
     });
 

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

@@ -41,7 +41,7 @@ export class RepositionScrollStrategy implements ScrollStrategy {
     if (!this._scrollSubscription) {
       let throttle = this._config ? this._config.scrollThrottle : 0;
 
-      this._scrollSubscription = this._scrollDispatcher.scrolled(throttle, () => {
+      this._scrollSubscription = this._scrollDispatcher.scrolled(throttle).subscribe(() => {
         this._overlayRef.updatePosition();
       });
     }

src/cdk/scrolling/scroll-dispatcher.spec.ts

@@ -47,7 +47,7 @@ describe('Scroll Dispatcher', () => {
       // Listen for notifications from scroll service with a throttle of 100ms
       const throttleTime = 100;
       const serviceSpy = jasmine.createSpy('service scroll callback');
-      scroll.scrolled(throttleTime, serviceSpy);
+      scroll.scrolled(throttleTime).subscribe(serviceSpy);
 
       // Emit a scroll event from the scrolling element in our component.
       // This event should be picked up by the scrollable directive and notify.
@@ -70,7 +70,7 @@ describe('Scroll Dispatcher', () => {
       const spy = jasmine.createSpy('zone unstable callback');
       const subscription = fixture.ngZone!.onUnstable.subscribe(spy);
 
-      scroll.scrolled(0, () => {});
+      scroll.scrolled(0).subscribe(() => {});
       dispatchFakeEvent(document, 'scroll');
 
       expect(spy).not.toHaveBeenCalled();
@@ -89,7 +89,7 @@ describe('Scroll Dispatcher', () => {
 
     it('should be able to unsubscribe from the global scrollable', () => {
       const spy = jasmine.createSpy('global scroll callback');
-      const subscription = scroll.scrolled(0, spy);
+      const subscription = scroll.scrolled(0).subscribe(spy);
 
       dispatchFakeEvent(document, 'scroll');
       expect(spy).toHaveBeenCalledTimes(1);
@@ -132,7 +132,7 @@ describe('Scroll Dispatcher', () => {
     it('should lazily add global listeners as service subscriptions are added and removed', () => {
       expect(scroll._globalSubscription).toBeNull('Expected no global listeners on init.');
 
-      const subscription = scroll.scrolled(0, () => {});
+      const subscription = scroll.scrolled(0).subscribe(() => {});
 
       expect(scroll._globalSubscription).toBeTruthy(
           'Expected global listeners after a subscription has been added.');

src/cdk/scrolling/scroll-dispatcher.ts

@@ -10,7 +10,9 @@ import {ElementRef, Injectable, NgZone, Optional, SkipSelf} from '@angular/core'
 import {Platform} from '@angular/cdk/platform';
 import {Subject} from 'rxjs/Subject';
 import {Subscription} from 'rxjs/Subscription';
+import {Observable} from 'rxjs/Observable';
 import {fromEvent} from 'rxjs/observable/fromEvent';
+import {of as observableOf} from 'rxjs/observable/of';
 import {auditTime} from 'rxjs/operator/auditTime';
 import {Scrollable} from './scrollable';
 
@@ -27,7 +29,7 @@ export class ScrollDispatcher {
   constructor(private _ngZone: NgZone, private _platform: Platform) { }
 
   /** Subject for notifying that a registered scrollable reference element has been scrolled. */
-  _scrolled: Subject<void> = new Subject<void>();
+  private _scrolled: Subject<void> = new Subject<void>();
 
   /** Keeps track of the global `scroll` and `resize` subscriptions. */
   _globalSubscription: Subscription | null = null;
@@ -47,8 +49,7 @@ export class ScrollDispatcher {
    * @param scrollable Scrollable instance to be registered.
    */
   register(scrollable: Scrollable): void {
-    const scrollSubscription = scrollable.elementScrolled().subscribe(() => this._notify());
-
+    const scrollSubscription = scrollable.elementScrolled().subscribe(() => this._scrolled.next());
     this.scrollableReferences.set(scrollable, scrollSubscription);
   }
 
@@ -66,44 +67,34 @@ export class ScrollDispatcher {
   }
 
   /**
-   * Subscribes to an observable that emits an event whenever any of the registered Scrollable
+   * Returns an observable that emits an event whenever any of the registered Scrollable
    * references (or window, document, or body) fire a scrolled event. Can provide a time in ms
    * to override the default "throttle" time.
    */
-  scrolled(auditTimeInMs: number = DEFAULT_SCROLL_TIME, callback: () => any): Subscription {
-    // Scroll events can only happen on the browser, so do nothing if we're not on the browser.
-    if (!this._platform.isBrowser) {
-      return Subscription.EMPTY;
-    }
-
-    // In the case of a 0ms delay, use an observable without auditTime
-    // since it does add a perceptible delay in processing overhead.
-    let observable = auditTimeInMs > 0 ?
-      auditTime.call(this._scrolled.asObservable(), auditTimeInMs) :
-      this._scrolled.asObservable();
-
-    this._scrolledCount++;
-
-    if (!this._globalSubscription) {
-      this._globalSubscription = this._ngZone.runOutsideAngular(() => {
-        return fromEvent(window.document, 'scroll').subscribe(() => this._notify());
-      });
-    }
+  scrolled(auditTimeInMs: number = DEFAULT_SCROLL_TIME): Observable<void> {
+    return this._platform.isBrowser ? Observable.create(observer => {
+      if (!this._globalSubscription) {
+        this._addGlobalListener();
+      }
 
-    // Note that we need to do the subscribing from here, in order to be able to remove
-    // the global event listeners once there are no more subscriptions.
-    let subscription = observable.subscribe(callback);
+      // In the case of a 0ms delay, use an observable without auditTime
+      // since it does add a perceptible delay in processing overhead.
+      const subscription = auditTimeInMs > 0 ?
+        auditTime.call(this._scrolled, auditTimeInMs).subscribe(observer) :
+        this._scrolled.subscribe(observer);
 
-    subscription.add(() => {
-      this._scrolledCount--;
+      this._scrolledCount++;
 
-      if (this._globalSubscription && !this.scrollableReferences.size && !this._scrolledCount) {
-        this._globalSubscription.unsubscribe();
-        this._globalSubscription = null;
-      }
-    });
+      return () => {
+        subscription.unsubscribe();
+        this._scrolledCount--;
 
-    return subscription;
+        if (this._globalSubscription && !this.scrollableReferences.size && !this._scrolledCount) {
+          this._globalSubscription.unsubscribe();
+          this._globalSubscription = null;
+        }
+      };
+    }) : observableOf<void>();
   }
 
   /** Returns all registered Scrollables that contain the provided element. */
@@ -133,9 +124,11 @@ export class ScrollDispatcher {
     return false;
   }
 
-  /** Sends a notification that a scroll event has been fired. */
-  _notify() {
-    this._scrolled.next();
+  /** Sets up the global scroll and resize listeners. */
+  private _addGlobalListener() {
+    this._globalSubscription = this._ngZone.runOutsideAngular(() => {
+      return fromEvent(window.document, 'scroll').subscribe(() => this._scrolled.next());
+    });
   }
 }
 

src/cdk/scrolling/viewport-ruler.ts

@@ -33,22 +33,20 @@ export class ViewportRuler implements OnDestroy {
   private _change: Observable<Event>;
 
   /** Subscriptions to streams that invalidate the cached viewport dimensions. */
-  private _invalidateCacheSubscriptions: Subscription[];
+  private _invalidateCacheSubscription = Subscription.EMPTY;
 
   constructor(platform: Platform, ngZone: NgZone, scrollDispatcher: ScrollDispatcher) {
     this._change = platform.isBrowser ? ngZone.runOutsideAngular(() => {
       return merge<Event>(fromEvent(window, 'resize'), fromEvent(window, 'orientationchange'));
     }) : observableOf();
 
     // Subscribe to scroll and resize events and update the document rectangle on changes.
-    this._invalidateCacheSubscriptions = [
-      scrollDispatcher.scrolled(0, () => this._cacheViewportGeometry()),
-      this.change().subscribe(() => this._cacheViewportGeometry())
-    ];
+    this._invalidateCacheSubscription = merge(scrollDispatcher.scrolled(0), this.change())
+      .subscribe(() => this._cacheViewportGeometry());
   }
 
   ngOnDestroy() {
-    this._invalidateCacheSubscriptions.forEach(subscription => subscription.unsubscribe());
+    this._invalidateCacheSubscription.unsubscribe();
   }
 
   /** Gets a ClientRect for the viewport's bounds. */

src/lib/autocomplete/autocomplete.spec.ts

@@ -80,11 +80,9 @@ describe('MatAutocomplete', () => {
           return {getContainerElement: () => overlayContainerElement};
         }},
         {provide: Directionality, useFactory: () => ({value: dir})},
-        {provide: ScrollDispatcher, useFactory: () => {
-          return {scrolled: (_delay: number, callback: () => any) => {
-            return scrolledSubject.asObservable().subscribe(callback);
-          }};
-        }}
+        {provide: ScrollDispatcher, useFactory: () => ({
+          scrolled: () => scrolledSubject.asObservable()
+        })}
       ]
     });
 

src/lib/select/select.spec.ts

@@ -114,11 +114,9 @@ describe('MatSelect', () => {
           return {getContainerElement: () => overlayContainerElement};
         }},
         {provide: Directionality, useFactory: () => dir = { value: 'ltr' }},
-        {provide: ScrollDispatcher, useFactory: () => {
-          return {scrolled: (_delay: number, callback: () => any) => {
-            return scrolledSubject.asObservable().subscribe(callback);
-          }};
-        }}
+        {provide: ScrollDispatcher, useFactory: () => ({
+          scrolled: () => scrolledSubject.asObservable()
+        })}
       ]
     });