review response

Author: andrewseguin

src/lib/core/core.ts

@@ -6,7 +6,7 @@ import {PortalModule} from './portal/portal-directives';
 import {OverlayModule} from './overlay/overlay-directives';
 import {A11yModule, A11Y_PROVIDERS} from './a11y/index';
 import {OVERLAY_PROVIDERS} from './overlay/overlay';
-import {Scroll} from './scroll/scroll';
+import {ScrollDispatcher} from './scroll/scroll-dispatcher';
 import {ScrollModule} from './scroll/scrollable';
 
 
@@ -48,7 +48,7 @@ export {
 export * from './overlay/position/connected-position-strategy';
 export * from './overlay/position/connected-position';
 export * from './scroll/scrollable';
-export * from './scroll/scroll';
+export * from './scroll/scroll-dispatcher';
 
 // Gestures
 export {MdGestureConfig} from './gestures/MdGestureConfig';
@@ -101,8 +101,8 @@ export {coerceNumberProperty} from './coersion/number-property';
 export {DefaultStyleCompatibilityModeModule} from './compatibility/default-mode';
 export {NoConflictStyleCompatibilityMode} from './compatibility/no-conflict-mode';
 
-// Scroll
-export {Scroll} from './scroll/scroll';
+// ScrollDispatcher
+export {ScrollDispatcher} from './scroll/scroll-dispatcher';
 export {Scrollable} from './scroll/scrollable';
 
 @NgModule({
@@ -127,7 +127,7 @@ export class MdCoreModule {
   static forRoot(): ModuleWithProviders {
     return {
       ngModule: MdCoreModule,
-      providers: [A11Y_PROVIDERS, OVERLAY_PROVIDERS, Scroll],
+      providers: [A11Y_PROVIDERS, OVERLAY_PROVIDERS, ScrollDispatcher],
     };
   }
 }

src/lib/core/scroll/scroll.spec.ts renamed to src/lib/core/scroll/scroll-dispatcher.spec.ts

@@ -1,10 +1,10 @@
 import {inject, TestBed, async, ComponentFixture} from '@angular/core/testing';
 import {NgModule, Component, ViewChild, ElementRef} from '@angular/core';
-import {Scroll} from './scroll';
+import {ScrollDispatcher} from './scroll-dispatcher';
 import {ScrollModule, Scrollable} from './scrollable';
 
-describe('Scrollable', () => {
-  let scroll: Scroll;
+describe('Scroll Dispatcher', () => {
+  let scroll: ScrollDispatcher;
   let fixture: ComponentFixture<ScrollingComponent>;
 
   beforeEach(async(() => {
@@ -15,19 +15,19 @@ describe('Scrollable', () => {
     TestBed.compileComponents();
   }));
 
-  beforeEach(inject([Scroll], (s: Scroll) => {
+  beforeEach(inject([ScrollDispatcher], (s: ScrollDispatcher) => {
     scroll = s;
 
     fixture = TestBed.createComponent(ScrollingComponent);
     fixture.detectChanges();
   }));
 
-  it('should register the scrollable directive with the scroll service', () => {
+  it('should be registered with the scrollable directive with the scroll service', () => {
     const componentScrollable = fixture.componentInstance.scrollable;
     expect(scroll.scrollableReferences.has(componentScrollable)).toBe(true);
   });
 
-  it('should deregister the scrollable directive when the component is destroyed', () => {
+  it('should have the scrollable directive deregistered when the component is destroyed', () => {
     const componentScrollable = fixture.componentInstance.scrollable;
     expect(scroll.scrollableReferences.has(componentScrollable)).toBe(true);
 
@@ -48,7 +48,9 @@ describe('Scrollable', () => {
     // Emit a scroll event from the scrolling element in our component.
     // This event should be picked up by the scrollable directive and notify.
     // The notification should be picked up by the service.
-    fixture.componentInstance.scrollingElement.nativeElement.dispatchEvent(new Event('scroll'));
+    const scrollEvent = document.createEvent('UIEvents');
+    scrollEvent.initUIEvent('scroll', true, true, window, 0);
+    fixture.componentInstance.scrollingElement.nativeElement.dispatchEvent(scrollEvent);
 
     expect(hasDirectiveScrollNotified).toBe(true);
     expect(hasServiceScrollNotified).toBe(true);
@@ -58,7 +60,7 @@ describe('Scrollable', () => {
 
 /** Simple component that contains a large div and can be scrolled. */
 @Component({
-  template: `<div #scrollingElement md-scrollable style="height: 9999px"></div>`
+  template: `<div #scrollingElement cdk-scrollable style="height: 9999px"></div>`
 })
 class ScrollingComponent {
   @ViewChild(Scrollable) scrollable: Scrollable;
@@ -68,7 +70,7 @@ class ScrollingComponent {
 const TEST_COMPONENTS = [ScrollingComponent];
 @NgModule({
   imports: [ScrollModule],
-  providers: [Scroll],
+  providers: [ScrollDispatcher],
   exports: TEST_COMPONENTS,
   declarations: TEST_COMPONENTS,
   entryComponents: TEST_COMPONENTS,

src/lib/core/scroll/scroll.ts renamed to src/lib/core/scroll/scroll-dispatcher.ts

@@ -3,35 +3,36 @@ import {Scrollable} from './scrollable';
 import {Subject} from 'rxjs/Subject';
 import {Observable} from 'rxjs/Observable';
 import {Subscription} from 'rxjs/Subscription';
+import 'rxjs/add/observable/fromEvent';
 
 
 /**
  * Service contained all registered Scrollable references and emits an event when any one of the
  * Scrollable references emit a scrolled event.
  */
 @Injectable()
-export class Scroll {
+export class ScrollDispatcher {
   /** Subject for notifying that a registered scrollable reference element has been scrolled. */
-  _scrolled: Subject<Event> = new Subject();
+  _scrolled: Subject<void> = new Subject<void>();
 
   /**
    * Map of all the scrollable references that are registered with the service and their
    * scroll event subscriptions.
    */
-  scrollableReferences: Map<Scrollable, Subscription> = new Map();
+  scrollableReferences: WeakMap<Scrollable, Subscription> = new WeakMap();
 
   constructor() {
     // By default, notify a scroll event when the document is scrolled or the window is resized.
-    window.document.addEventListener('scroll', this._notify.bind(this));
-    window.addEventListener('resize', this._notify.bind(this));
+    Observable.fromEvent(window.document, 'scroll').subscribe(() => this._notify());
+    Observable.fromEvent(window, 'resize').subscribe(() => this._notify());
   }
 
   /**
    * Registers a Scrollable with the service and listens for its scrolled events. When the
    * scrollable is scrolled, the service emits the event in its scrolled observable.
    */
   register(scrollable: Scrollable): void {
-    const scrollSubscription = scrollable.elementScrolled().subscribe(this._notify.bind(this));
+    const scrollSubscription = scrollable.elementScrolled().subscribe(() => this._notify());
     this.scrollableReferences.set(scrollable, scrollSubscription);
   }
 
@@ -48,12 +49,12 @@ export class Scroll {
    * references (or window, document, or body) fire a scrolled event.
    * TODO: Add an event limiter that includes throttle with the leading and trailing events.
    */
-  scrolled(): Observable<Event> {
+  scrolled(): Observable<void> {
     return this._scrolled.asObservable();
   }
 
   /** Sends a notification that a scroll event has been fired. */
-  _notify(e: Event) {
-    this._scrolled.next(e);
+  _notify() {
+    this._scrolled.next();
   }
 }

src/lib/core/scroll/scrollable.ts

@@ -2,39 +2,33 @@ import {
   Directive, ElementRef, OnInit, OnDestroy, ModuleWithProviders,
   NgModule
 } from '@angular/core';
-import {Subject} from 'rxjs/Subject';
 import {Observable} from 'rxjs/Observable';
-import {Scroll} from './scroll';
+import {ScrollDispatcher} from './scroll-dispatcher';
+import 'rxjs/add/observable/fromEvent';
 
 
 /**
- * Sends an event when the directive's element is scrolled. Registers itself with the Scroll
+ * Sends an event when the directive's element is scrolled. Registers itself with the ScrollDispatcher
  * service to include itself as part of its collection of scrolling events that it can be listened
  * to through the service.
  */
 @Directive({
-  selector: '[md-scrollable]'
+  selector: '[cdk-scrollable]'
 })
 export class Scrollable implements OnInit, OnDestroy {
-  /** Subject for notifying that the element has been scrolled. */
-  private _elementScrolled: Subject<Event> = new Subject();
-
-  constructor(private _elementRef: ElementRef, private _scroll: Scroll) {}
+  constructor(private _elementRef: ElementRef, private _scroll: ScrollDispatcher) {}
 
   ngOnInit() {
     this._scroll.register(this);
-    this._elementRef.nativeElement.addEventListener('scroll', (e: Event) => {
-      this._elementScrolled.next(e);
-    });
   }
 
   ngOnDestroy() {
     this._scroll.deregister(this);
   }
 
-  /** Returns observable that emits an event when the scroll event is fired on the host element. */
-  elementScrolled(): Observable<Event> {
-    return this._elementScrolled.asObservable();
+  /** Returns observable that emits when the scroll event is fired on the host element. */
+  elementScrolled(): Observable<any> {
+    return Observable.fromEvent(this._elementRef.nativeElement, 'scroll');
   }
 }
 
@@ -47,7 +41,7 @@ export class ScrollModule {
   static forRoot(): ModuleWithProviders {
     return {
       ngModule: ScrollModule,
-      providers: [Scroll]
+      providers: [ScrollDispatcher]
     };
   }
 }

src/lib/sidenav/sidenav.html

@@ -3,6 +3,6 @@
 
 <ng-content select="md-sidenav, mat-sidenav"></ng-content>
 
-<div class="md-sidenav-content" [ngStyle]="_getStyles()" md-scrollable>
+<div class="md-sidenav-content" [ngStyle]="_getStyles()" cdk-scrollable>
   <ng-content></ng-content>
 </div>

src/lib/sidenav/sidenav.ts

@@ -17,14 +17,14 @@ import {
 } from '@angular/core';
 import {CommonModule} from '@angular/common';
 import {Dir, MdError, coerceBooleanProperty, DefaultStyleCompatibilityModeModule} from '../core';
-import {A11yModule, A11Y_PROVIDERS} from '../core/a11y/index';
+import {A11yModule} from '../core/a11y/index';
 import {FocusTrap} from '../core/a11y/focus-trap';
 import {ESCAPE} from '../core/keyboard/keycodes';
 import {OverlayModule} from '../core/overlay/overlay-directives';
 import {ScrollModule} from '../core/scroll/scrollable';
 import {InteractivityChecker} from '../core/a11y/interactivity-checker';
 import {MdLiveAnnouncer} from '../core/a11y/live-announcer';
-import {Scroll} from '../core/scroll/scroll';
+import {ScrollDispatcher} from '../core/scroll/scroll-dispatcher';
 
 
 /** Exception thrown when two MdSidenav are matching the same side. */
@@ -522,7 +522,7 @@ export class MdSidenavModule {
   static forRoot(): ModuleWithProviders {
     return {
       ngModule: MdSidenavModule,
-      providers: [MdLiveAnnouncer, InteractivityChecker, Scroll]
+      providers: [MdLiveAnnouncer, InteractivityChecker, ScrollDispatcher]
     };
   }
 }

src/lib/tooltip/tooltip.ts

@@ -29,7 +29,7 @@ import {MdTooltipInvalidPositionError} from './tooltip-errors';
 import {Observable} from 'rxjs/Observable';
 import {Subject} from 'rxjs/Subject';
 import {Dir} from '../core/rtl/dir';
-import {Scroll} from '../core/scroll/scroll';
+import {ScrollDispatcher} from '../core/scroll/scroll-dispatcher';
 import {ScrollModule} from '../core/scroll/scrollable';
 import {OverlayPositionBuilder} from '../core/overlay/position/overlay-position-builder';
 import {ViewportRuler} from '../core/overlay/position/viewport-ruler';
@@ -96,7 +96,7 @@ export class MdTooltip implements OnInit, OnDestroy {
   }
 
   constructor(private _overlay: Overlay,
-              private _scroll: Scroll,
+              private _scroll: ScrollDispatcher,
               private _elementRef: ElementRef,
               private _viewContainerRef: ViewContainerRef,
               private _ngZone: NgZone,
@@ -378,7 +378,7 @@ export class MdTooltipModule {
         Overlay,
         OverlayPositionBuilder,
         ViewportRuler,
-        Scroll
+        ScrollDispatcher
       ]
     };
   }

tools/gulp/tasks/components.ts

@@ -67,6 +67,7 @@ task(':build:components:rollup', [':build:components:inline'], () => {
 
     // Rxjs dependencies
     'rxjs/Subject': 'Rx',
+    'rxjs/add/observable/fromEvent': 'Rx.Observable',
     'rxjs/add/observable/forkJoin': 'Rx.Observable',
     'rxjs/add/observable/of': 'Rx.Observable',
     'rxjs/add/operator/toPromise': 'Rx.Observable.prototype',