refactor: move escape key logic to connected overlay directive

Author: crisbeto

src/lib/core/a11y/list-key-manager.spec.ts

@@ -1,7 +1,7 @@
 import {QueryList} from '@angular/core';
 import {fakeAsync, tick} from '@angular/core/testing';
 import {FocusKeyManager} from './focus-key-manager';
-import {DOWN_ARROW, UP_ARROW, TAB, HOME, END, ESCAPE} from '../keyboard/keycodes';
+import {DOWN_ARROW, UP_ARROW, TAB, HOME, END} from '../keyboard/keycodes';
 import {ListKeyManager} from './list-key-manager';
 import {ActiveDescendantKeyManager} from './activedescendant-key-manager';
 
@@ -39,7 +39,6 @@ describe('Key managers', () => {
   let TAB_EVENT: KeyboardEvent;
   let HOME_EVENT: KeyboardEvent;
   let END_EVENT: KeyboardEvent;
-  let ESCAPE_EVENT: KeyboardEvent;
 
   beforeEach(() => {
     itemList = new FakeQueryList<any>();
@@ -49,7 +48,6 @@ describe('Key managers', () => {
     TAB_EVENT = new FakeEvent(TAB) as KeyboardEvent;
     HOME_EVENT = new FakeEvent(HOME) as KeyboardEvent;
     END_EVENT = new FakeEvent(END) as KeyboardEvent;
-    ESCAPE_EVENT = new FakeEvent(ESCAPE) as KeyboardEvent;
   });
 
 
@@ -242,14 +240,6 @@ describe('Key managers', () => {
         expect(TAB_EVENT.defaultPrevented).toBe(false);
       });
 
-      it('should emit an event when escape is pressed', () => {
-        let spy = jasmine.createSpy('escape spy');
-        keyManager.escape.first().subscribe(spy);
-        keyManager.onKeydown(ESCAPE_EVENT);
-
-        expect(spy).toHaveBeenCalled();
-      });
-
       it('should activate the first item when pressing down on a clean key manager', () => {
         keyManager = new ListKeyManager<FakeFocusable>(itemList);
 

src/lib/core/a11y/list-key-manager.ts

@@ -1,5 +1,5 @@
 import {QueryList} from '@angular/core';
-import {UP_ARROW, DOWN_ARROW, TAB, HOME, END, ESCAPE} from '../core';
+import {UP_ARROW, DOWN_ARROW, TAB, HOME, END} from '../core';
 import {Observable} from 'rxjs/Observable';
 import {Subject} from 'rxjs/Subject';
 
@@ -19,7 +19,6 @@ export class ListKeyManager<T extends CanDisable> {
   private _activeItemIndex: number = null;
   private _activeItem: T;
   private _tabOut = new Subject<void>();
-  private _escape = new Subject<void>();
   private _wrap: boolean = false;
 
   constructor(private _items: QueryList<T>) {
@@ -64,9 +63,6 @@ export class ListKeyManager<T extends CanDisable> {
       case END:
         this.setLastItemActive();
         break;
-      case ESCAPE:
-        this._escape.next(null);
-        break;
       case TAB:
         // Note that we shouldn't prevent the default action on tab.
         this._tabOut.next(null);
@@ -125,14 +121,6 @@ export class ListKeyManager<T extends CanDisable> {
     return this._tabOut.asObservable();
   }
 
-  /**
-   * Observable that emits whenever the escape key is pressed, which usually indicates
-   * that the component should be closed.
-   */
-  get escape(): Observable<void> {
-    return this._escape.asObservable();
-  }
-
   /**
    * This method sets the active item, given a list of items and the delta between the
    * currently active item and the new active item. It will calculate differently

src/lib/core/overlay/overlay-directives.spec.ts

@@ -6,6 +6,8 @@ import {OverlayContainer} from './overlay-container';
 import {ConnectedPositionStrategy} from './position/connected-position-strategy';
 import {ConnectedOverlayPositionChange} from './position/connected-position';
 import {Dir} from '../rtl/dir';
+import {dispatchKeyboardEvent} from '../testing/dispatch-events';
+import {ESCAPE} from '../keyboard/keycodes';
 
 
 describe('Overlay directives', () => {
@@ -98,6 +100,17 @@ describe('Overlay directives', () => {
     expect(getPaneElement().getAttribute('dir')).toBe('ltr');
   });
 
+  it('should close when pressing escape', () => {
+    fixture.componentInstance.isOpen = true;
+    fixture.detectChanges();
+
+    dispatchKeyboardEvent(document, 'keydown', ESCAPE);
+    fixture.detectChanges();
+
+    expect(overlayContainerElement.textContent.trim()).toBe('',
+        'Expected overlay to have been detached.');
+  });
+
   describe('inputs', () => {
 
     it('should set the width', () => {

src/lib/core/overlay/overlay-directives.ts

@@ -9,7 +9,8 @@ import {
     Input,
     OnDestroy,
     Output,
-    ElementRef
+    ElementRef,
+    Renderer2,
 } from '@angular/core';
 import {Overlay, OVERLAY_PROVIDERS} from './overlay';
 import {OverlayRef} from './overlay-ref';
@@ -21,10 +22,12 @@ import {
 } from './position/connected-position';
 import {PortalModule} from '../portal/portal-directives';
 import {ConnectedPositionStrategy} from './position/connected-position-strategy';
-import {Subscription} from 'rxjs/Subscription';
 import {Dir, LayoutDirection} from '../rtl/dir';
 import {Scrollable} from './scroll/scrollable';
 import {coerceBooleanProperty} from '../coercion/boolean-property';
+import {ESCAPE} from '../keyboard/keycodes';
+import {Subscription} from 'rxjs/Subscription';
+
 
 /** Default set of positions for the overlay. Follows the behavior of a dropdown. */
 let defaultPositionList = [
@@ -68,6 +71,7 @@ export class ConnectedOverlayDirective implements OnDestroy {
   private _offsetX: number = 0;
   private _offsetY: number = 0;
   private _position: ConnectedPositionStrategy;
+  private _closeKeyListener: Function;
 
   /** Origin for the connected overlay. */
   @Input() origin: OverlayOrigin;
@@ -152,6 +156,7 @@ export class ConnectedOverlayDirective implements OnDestroy {
 
   constructor(
       private _overlay: Overlay,
+      private _renderer: Renderer2,
       templateRef: TemplateRef<any>,
       viewContainerRef: ViewContainerRef,
       @Optional() private _dir: Dir) {
@@ -249,6 +254,12 @@ export class ConnectedOverlayDirective implements OnDestroy {
 
     this._position.withDirection(this.dir);
     this._overlayRef.getState().direction = this.dir;
+    this._closeKeyListener = this._renderer.listen('document', 'keydown',
+      (event: KeyboardEvent) => {
+        if (event.keyCode === ESCAPE) {
+          this._detachOverlay();
+        }
+      });
 
     if (!this._overlayRef.hasAttached()) {
       this._overlayRef.attach(this._templatePortal);
@@ -273,6 +284,10 @@ export class ConnectedOverlayDirective implements OnDestroy {
       this._backdropSubscription.unsubscribe();
       this._backdropSubscription = null;
     }
+
+    if (this._closeKeyListener) {
+      this._closeKeyListener();
+    }
   }
 
   /** Destroys the overlay created by this directive. */
@@ -284,9 +299,14 @@ export class ConnectedOverlayDirective implements OnDestroy {
     if (this._backdropSubscription) {
       this._backdropSubscription.unsubscribe();
     }
+
     if (this._positionSubscription) {
       this._positionSubscription.unsubscribe();
     }
+
+    if (this._closeKeyListener) {
+      this._closeKeyListener();
+    }
   }
 }
 

src/lib/select/select.html

@@ -15,7 +15,7 @@
 
 <ng-template cdk-connected-overlay [origin]="origin" [open]="panelOpen" hasBackdrop (backdropClick)="close()"
   backdropClass="cdk-overlay-transparent-backdrop" [positions]="_positions" [minWidth]="_triggerWidth"
-  [offsetY]="_offsetY" [offsetX]="_offsetX" (attach)="_setScrollTop()">
+  [offsetY]="_offsetY" [offsetX]="_offsetX" (attach)="_setScrollTop()" (detach)="close()">
   <div class="mat-select-panel" [@transformPanel]="'showing'" (@transformPanel.done)="_onPanelDone()"
     (keydown)="_keyManager.onKeydown($event)" [style.transformOrigin]="_transformOrigin"
       [class.mat-select-panel-done-animating]="_panelDoneAnimating">

src/lib/select/select.spec.ts

@@ -22,7 +22,7 @@ import {
 import {ViewportRuler} from '../core/overlay/position/viewport-ruler';
 import {dispatchFakeEvent, dispatchKeyboardEvent} from '../core/testing/dispatch-events';
 import {wrappedErrorMessage} from '../core/testing/wrapped-error-message';
-import {TAB, ESCAPE} from '../core/keyboard/keycodes';
+import {TAB} from '../core/keyboard/keycodes';
 
 
 describe('MdSelect', () => {
@@ -220,20 +220,6 @@ describe('MdSelect', () => {
       });
     }));
 
-    it('should close the panel when pressing escape', async(() => {
-      trigger.click();
-      fixture.detectChanges();
-      expect(fixture.componentInstance.select.panelOpen).toBe(true);
-
-      const panel = overlayContainerElement.querySelector('.mat-select-panel');
-      dispatchKeyboardEvent(panel, 'keydown', ESCAPE);
-      fixture.detectChanges();
-
-      fixture.whenStable().then(() => {
-        expect(fixture.componentInstance.select.panelOpen).toBe(false);
-      });
-    }));
-
   });
 
   describe('selection logic', () => {

src/lib/select/select.ts

@@ -130,8 +130,8 @@ export class MdSelect implements AfterContentInit, OnDestroy, OnInit, ControlVal
   /** Subscription to changes in the option list. */
   private _changeSubscription: Subscription;
 
-  /** Subscription to tab and escape presses while overlay is focused. */
-  private _closeKeySubscription: Subscription;
+  /** Subscription to tab events while overlay is focused. */
+  private _tabSubscription: Subscription;
 
   /** Whether filling out the select is required in the form.  */
   private _required: boolean = false;
@@ -337,8 +337,8 @@ export class MdSelect implements AfterContentInit, OnDestroy, OnInit, ControlVal
       this._changeSubscription.unsubscribe();
     }
 
-    if (this._closeKeySubscription) {
-      this._closeKeySubscription.unsubscribe();
+    if (this._tabSubscription) {
+      this._tabSubscription.unsubscribe();
     }
   }
 
@@ -569,9 +569,7 @@ export class MdSelect implements AfterContentInit, OnDestroy, OnInit, ControlVal
   /** Sets up a key manager to listen to keyboard events on the overlay panel. */
   private _initKeyManager() {
     this._keyManager = new FocusKeyManager(this.options);
-    this._closeKeySubscription = Observable
-      .merge(this._keyManager.tabOut, this._keyManager.escape)
-      .subscribe(() => this.close());
+    this._tabSubscription = this._keyManager.tabOut.subscribe(() => this.close());
   }
 
   /** Drops current option subscriptions and IDs and resets from scratch. */