Address comments

Author: willshowell

src/cdk/overlay/keyboard/overlay-keyboard-dispatcher.spec.ts

@@ -34,17 +34,17 @@ describe('OverlayKeyboardDispatcher', () => {
     const overlayTwo = overlay.create();
 
     // Attach overlays
-    keyboardDispatcher.attach(overlayOne);
-    keyboardDispatcher.attach(overlayTwo);
+    keyboardDispatcher.add(overlayOne);
+    keyboardDispatcher.add(overlayTwo);
 
     expect(keyboardDispatcher._attachedOverlays.length)
         .toBe(2, 'Expected both overlays to be tracked.');
     expect(keyboardDispatcher._attachedOverlays[0]).toBe(overlayOne, 'Expected one to be first.');
     expect(keyboardDispatcher._attachedOverlays[1]).toBe(overlayTwo, 'Expected two to be last.');
 
     // Detach first one and re-attach it
-    keyboardDispatcher.detach(overlayOne);
-    keyboardDispatcher.attach(overlayOne);
+    keyboardDispatcher.remove(overlayOne);
+    keyboardDispatcher.add(overlayOne);
 
     expect(keyboardDispatcher._attachedOverlays[0])
         .toBe(overlayTwo, 'Expected two to now be first.');
@@ -62,8 +62,8 @@ describe('OverlayKeyboardDispatcher', () => {
     overlayTwo.keyboardEvents().subscribe(overlayTwoSpy);
 
     // Attach overlays
-    keyboardDispatcher.attach(overlayOne);
-    keyboardDispatcher.attach(overlayTwo);
+    keyboardDispatcher.add(overlayOne);
+    keyboardDispatcher.add(overlayTwo);
 
     dispatchKeyboardEvent(document.body, 'keydown', ESCAPE);
 
@@ -82,8 +82,8 @@ describe('OverlayKeyboardDispatcher', () => {
     overlayTwo.keyboardEvents().subscribe(overlayTwoSpy);
 
     // Attach overlays
-    keyboardDispatcher.attach(overlayOne);
-    keyboardDispatcher.attach(overlayTwo);
+    keyboardDispatcher.add(overlayOne);
+    keyboardDispatcher.add(overlayTwo);
 
     const overlayOnePane = overlayOne.overlayElement;
 

src/cdk/overlay/keyboard/overlay-keyboard-dispatcher.ts

@@ -38,12 +38,12 @@ export class OverlayKeyboardDispatcher implements OnDestroy {
   }
 
   /** Add a new overlay to the list of attached overlay refs. */
-  attach(overlayRef: OverlayRef): void {
+  add(overlayRef: OverlayRef): void {
     this._attachedOverlays.push(overlayRef);
   }
 
   /** Remove an overlay from the list of attached overlay refs. */
-  detach(overlayRef: OverlayRef): void {
+  remove(overlayRef: OverlayRef): void {
     const index = this._attachedOverlays.indexOf(overlayRef);
     if (index > -1) {
       this._attachedOverlays.splice(index, 1);

src/cdk/overlay/overlay-ref.ts

@@ -9,6 +9,7 @@
 import {NgZone} from '@angular/core';
 import {PortalHost, Portal} from '@angular/cdk/portal';
 import {OverlayState} from './overlay-state';
+import {OverlayKeyboardDispatcher} from './keyboard/overlay-keyboard-dispatcher';
 import {Observable} from 'rxjs/Observable';
 import {Subject} from 'rxjs/Subject';
 
@@ -30,7 +31,8 @@ export class OverlayRef implements PortalHost {
       private _portalHost: PortalHost,
       private _pane: HTMLElement,
       private _state: OverlayState,
-      private _ngZone: NgZone) {
+      private _ngZone: NgZone,
+      private _keyboardDispatcher: OverlayKeyboardDispatcher) {
 
     _state.scrollStrategy.attach(this);
   }
@@ -78,6 +80,9 @@ export class OverlayRef implements PortalHost {
     // Only emit the `attachments` event once all other setup is done.
     this._attachments.next();
 
+    // Track this overlay by the keyboard dispatcher
+    this._keyboardDispatcher.add(this);
+
     return attachResult;
   }
 
@@ -99,6 +104,9 @@ export class OverlayRef implements PortalHost {
     // Only emit after everything is detached.
     this._detachments.next();
 
+    // Remove this overlay from keyboard dispatcher tracking
+    this._keyboardDispatcher.remove(this);
+
     return detachmentResult;
   }
 
@@ -129,28 +137,28 @@ export class OverlayRef implements PortalHost {
   }
 
   /**
-   * Returns an observable that emits when the backdrop has been clicked.
+   * Gets an observable that emits when the backdrop has been clicked.
    */
   backdropClick(): Observable<void> {
     return this._backdropClick.asObservable();
   }
 
-  /** Returns an observable that emits when the overlay has been attached. */
+  /** Gets an observable that emits when the overlay has been attached. */
   attachments(): Observable<void> {
     return this._attachments.asObservable();
   }
 
-  /** Returns an observable that emits when the overlay has been detached. */
+  /** Gets an observable that emits when the overlay has been detached. */
   detachments(): Observable<void> {
     return this._detachments.asObservable();
   }
 
-  /** Returns an observable that emits when the overlay has been disposed. */
+  /** Gets an observable that emits when the overlay has been disposed. */
   disposed(): Observable<void> {
     return this._disposed.asObservable();
   }
 
-  /** Returns an observable of keyboard events targeted to this overlay. */
+  /** Gets an observable of keyboard events targeted to this overlay. */
   keyboardEvents(): Observable<KeyboardEvent> {
     return this._dispatchedKeyboardEvents.asObservable();
   }

src/cdk/overlay/overlay.ts

@@ -14,7 +14,6 @@ import {
   NgZone,
 } from '@angular/core';
 import {DomPortalHost} from '@angular/cdk/portal';
-import {takeUntil} from '@angular/cdk/rxjs';
 import {OverlayState} from './overlay-state';
 import {OverlayRef} from './overlay-ref';
 import {OverlayPositionBuilder} from './position/overlay-position-builder';
@@ -58,9 +57,7 @@ export class Overlay {
   create(state: OverlayState = defaultState): OverlayRef {
     const pane = this._createPaneElement();
     const portalHost = this._createPortalHost(pane);
-    const overlayRef = new OverlayRef(portalHost, pane, state, this._ngZone);
-    this._trackOverlayAttachments(overlayRef);
-    return overlayRef;
+    return new OverlayRef(portalHost, pane, state, this._ngZone, this._keyboardDispatcher);
   }
 
   /**
@@ -93,18 +90,5 @@ export class Overlay {
   private _createPortalHost(pane: HTMLElement): DomPortalHost {
     return new DomPortalHost(pane, this._componentFactoryResolver, this._appRef, this._injector);
   }
-  /**
-   * Subscribe to attach and detach events and register them accordingly
-   * with the keyboard dispatcher service.
-   */
-  private _trackOverlayAttachments(overlayRef: OverlayRef): void {
-    // Append overlay ref when attached
-    takeUntil.call(overlayRef.attachments(), overlayRef.disposed())
-      .subscribe(() => this._keyboardDispatcher.attach(overlayRef));
-
-    // Remove overlay ref when detached
-    takeUntil.call(overlayRef.detachments(), overlayRef.disposed())
-        .subscribe(() => this._keyboardDispatcher.detach(overlayRef));
-  }
 
 }