fix(dropdown): close on escape from anywhere

Fixes #1741 Closes #2051
ng-bootstrap · Jul 6, 2018 · 2495570 · 2495570
1 parent 5af6c4b
commit 2495570
Show file tree

Hide file tree

Showing 2 changed files with 107 additions and 40 deletions.
diff --git a/src/dropdown/dropdown.spec.ts b/src/dropdown/dropdown.spec.ts
@@ -1,5 +1,6 @@
 import {TestBed, ComponentFixture, inject} from '@angular/core/testing';
 import {createGenericTestComponent} from '../test/common';
+import {Key} from '../util/key';
 
 import {Component} from '@angular/core';
 import {By} from '@angular/platform-browser';
@@ -19,6 +20,15 @@ function getMenuEl(tc) {
   return tc.querySelector(`[ngbDropdownMenu]`);
 }
 
+function createFakeEscapeKeyUpEvent(): Event {
+  const key = Key.Escape;
+  const event = document.createEvent('KeyboardEvent') as any;
+  let initEvent = (event.initKeyEvent || event.initKeyboardEvent).bind(event);
+  initEvent('keyup', true, true, window, 0, 0, 0, 0, 0, key);
+  Object.defineProperties(event, {which: {get: () => key}});
+  return event;
+}
+
 const jasmineMatchers = {
   toBeShown: function(util, customEqualityTests) {
     return {
@@ -361,44 +371,87 @@ describe('ngb-dropdown-toggle', () => {
     expect(compiled).toBeShown();
   });
 
-  it('should close on ESC', () => {
-    const html = `
-      <div ngbDropdown>
-          <button ngbDropdownToggle>Toggle dropdown</button>
-          <div ngbDropdownMenu></div>
-      </div>`;
+  describe('escape closing', () => {
+    it('should close on ESC from anywhere', () => {
+      const fixture = createTestComponent(`
+          <div ngbDropdown [autoClose]="true">
+              <button ngbDropdownToggle>Toggle dropdown</button>
+              <div ngbDropdownMenu></div>
+          </div>`);
+      const compiled = fixture.nativeElement;
 
-    const fixture = createTestComponent(html);
-    const compiled = fixture.nativeElement;
-    const buttonEl = compiled.querySelector('button');
+      compiled.querySelector('button').click();
+      fixture.detectChanges();
+      expect(compiled).toBeShown();
 
-    buttonEl.click();
-    fixture.detectChanges();
-    expect(compiled).toBeShown();
+      document.dispatchEvent(createFakeEscapeKeyUpEvent());
+      fixture.detectChanges();
 
-    fixture.debugElement.query(By.directive(NgbDropdown)).triggerEventHandler('keyup.esc', {});
-    fixture.detectChanges();
-    expect(compiled).not.toBeShown();
-  });
+      expect(compiled).not.toBeShown();
+    });
+    it('should close on ESC from the toggling button', () => {
+      const fixture = createTestComponent(`
+        <div ngbDropdown [autoClose]="true">
+            <button ngbDropdownToggle>Toggle dropdown</button>
+            <div ngbDropdownMenu></div>
+        </div>`);
+      const compiled = fixture.nativeElement;
+      const buttonElement = compiled.querySelector('button');
 
-  it('should not close on ESC if autoClose is set to false', () => {
-    const html = `
-      <div ngbDropdown [autoClose]="false">
-          <button ngbDropdownToggle>Toggle dropdown</button>
-          <div ngbDropdownMenu></div>
-      </div>`;
+      buttonElement.click();
+      fixture.detectChanges();
+      expect(compiled).toBeShown();
 
-    const fixture = createTestComponent(html);
-    const compiled = fixture.nativeElement;
-    const buttonEl = compiled.querySelector('button');
+      buttonElement.dispatchEvent(createFakeEscapeKeyUpEvent());
+      fixture.detectChanges();
 
-    buttonEl.click();
-    fixture.detectChanges();
-    expect(compiled).toBeShown();
+      expect(compiled).not.toBeShown();
+    });
 
-    fixture.debugElement.query(By.directive(NgbDropdown)).triggerEventHandler('keyup.esc', {});
-    fixture.detectChanges();
-    expect(compiled).toBeShown();
+    it('should not close on ESC from the toggling button if autoClose is set to false', () => {
+      const fixture = createTestComponent(`
+        <div ngbDropdown [autoClose]="false">
+            <button ngbDropdownToggle>Toggle dropdown</button>
+            <div ngbDropdownMenu></div>
+        </div>`);
+      const compiled = fixture.nativeElement;
+      const buttonElement = compiled.querySelector('button');
+
+      buttonElement.click();
+      fixture.detectChanges();
+      expect(compiled).toBeShown();
+
+      buttonElement.dispatchEvent(createFakeEscapeKeyUpEvent());
+      fixture.detectChanges();
+
+      const expectation = expect(compiled);
+      expectation.toBeShown();
+      buttonElement.click();
+      fixture.detectChanges();
+      expectation.not.toBeShown();
+    });
+    it('should not close on ESC from anywhere if autoClose is set to false', () => {
+      const fixture = createTestComponent(`
+          <div ngbDropdown [autoClose]="false">
+              <button ngbDropdownToggle>Toggle dropdown</button>
+              <div ngbDropdownMenu></div>
+          </div>`);
+      const compiled = fixture.nativeElement;
+      const buttonElement = compiled.querySelector('button');
+
+      buttonElement.click();
+      fixture.detectChanges();
+      expect(compiled).toBeShown();
+
+      document.dispatchEvent(createFakeEscapeKeyUpEvent());
+      fixture.detectChanges();
+
+      const expectation = expect(compiled);
+      expectation.toBeShown();
+      buttonElement.click();
+      fixture.detectChanges();
+      expectation.not.toBeShown();
+    });
   });
 
   it('should not close on item click if autoClose is set to false', () => {

diff --git a/src/dropdown/dropdown.ts b/src/dropdown/dropdown.ts
@@ -9,10 +9,16 @@ import {
   ContentChild,
   NgZone,
   Renderer2,
-  OnInit
+  OnInit,
+  OnDestroy,
+  ChangeDetectorRef
 } from '@angular/core';
+import {DOCUMENT} from '@angular/common';
+import {fromEvent} from 'rxjs';
+import {filter, takeUntil} from 'rxjs/operators';
 import {NgbDropdownConfig} from './dropdown-config';
 import {positionElements, PlacementArray, Placement} from '../util/positioning';
+import {Key} from '../util/key';
 
 /**
  */
@@ -101,13 +107,10 @@ export class NgbDropdownToggle extends NgbDropdownAnchor {
 @Directive({
   selector: '[ngbDropdown]',
   exportAs: 'ngbDropdown',
-  host: {
-    '[class.show]': 'isOpen()',
-    '(keyup.esc)': 'closeFromOutsideEsc()',
-    '(document:click)': 'closeFromClick($event)'
-  }
+  host: {'[class.show]': 'isOpen()', '(document:click)': 'closeFromClick($event)'}
 })
-export class NgbDropdown implements OnInit {
+export class NgbDropdown implements OnInit,
+    OnDestroy {
   private _zoneSubscription: any;
 
   @ContentChild(NgbDropdownMenu) private _menu: NgbDropdownMenu;
@@ -142,10 +145,12 @@ export class NgbDropdown implements OnInit {
    */
   @Output() openChange = new EventEmitter();
 
-  constructor(config: NgbDropdownConfig, ngZone: NgZone) {
+  constructor(
+      private _changeDetector: ChangeDetectorRef, config: NgbDropdownConfig, @Inject(DOCUMENT) private _document: any,
+      private _ngZone: NgZone) {
     this.placement = config.placement;
     this.autoClose = config.autoClose;
-    this._zoneSubscription = ngZone.onStable.subscribe(() => { this._positionMenu(); });
+    this._zoneSubscription = _ngZone.onStable.subscribe(() => { this._positionMenu(); });
   }
 
   ngOnInit() {
@@ -167,6 +172,15 @@ export class NgbDropdown implements OnInit {
       this._open = true;
       this._positionMenu();
       this.openChange.emit(true);
+      this._ngZone.runOutsideAngular(
+          () => fromEvent<KeyboardEvent>(this._document, 'keyup')
+                    .pipe(
+                        takeUntil(this.openChange.pipe(filter(open => open === false))),
+                        filter(event => event.which === Key.Escape))
+                    .subscribe(() => {
+                      this.closeFromOutsideEsc();
+                      this._changeDetector.detectChanges();
+                    }));
     }
   }