refactor(overlay): address PR comments

Author: wnvko

CHANGELOG.md

@@ -8,9 +8,9 @@ All notable changes for each version of this project will be documented in this
 - `igxCombo`
     - **Behavioral Change** - Change default positioning strategy from `ConnectedPositioningStrategy` to `AutoPositionStrategy`. The [`Auto`](https://www.infragistics.com/products/ignite-ui-angular/angular/components/overlay_position.html#auto) strategy will initially try to show the element like the Connected strategy does. If the element goes out of the viewport Auto will flip the starting point and the direction, i.e. if the direction is 'bottom', it will switch it to 'top' and so on. If after flipping direction the content goes out of the view, auto strategy will revert to initial start point and direction and will push the content into the view. Note after pushing the content it may hide the combo's input.
 - `IgxOverlay`
-    - **Behavioral Change** - Added new property `closeOnEsc` in `OverlaySettings`. Should overlay close or not on escape keypress can now be controlled via this new property. By default `closeOnEsc` is set to `false`.
+    - **Behavioral Change** - Added new property `closeOnEscape` in `OverlaySettings`. Should overlay close or not on escape keypress can now be controlled via this new property. By default `closeOnEscape` is set to `false`.
 - `igxDialog`
-    - Added `closeOnEscapeKey` - with it, the dialog can be allowed or prevented from closing when `Esc` is pressed.
+    - Added `closeOnEscape` - with it, the dialog can be allowed or prevented from closing when `Esc` is pressed.
 - `IgxNavbar`:
     - **Breaking Changes** - The `igx-action-icon` has been renamed to `igx-navbar-action`. It should get renamed in your components via `ng update`;
 - `igxGrid`

projects/igniteui-angular/src/lib/date-picker/date-picker.component.ts

@@ -784,7 +784,7 @@ export class IgxDatePickerComponent implements IDatePicker, ControlValueAccessor
         this._modalOverlaySettings = {
             closeOnOutsideClick: true,
             modal: true,
-            closeOnEsc: true,
+            closeOnEscape: true,
             outlet: this.outlet
         };
 

projects/igniteui-angular/src/lib/dialog/dialog.component.ts

@@ -73,24 +73,36 @@ export class IgxDialogComponent implements IToggleView, OnInit, OnDestroy, After
     @Input()
     public id = `igx-dialog-${DIALOG_ID++}`;
 
+    /**
+     * Controls whether the dialog should be shown as modal. Defaults to `true`
+     * ```html
+     * <igx-dialog [isModal]="false" ></igx-dialog>
+     * ```
+     */
     @Input()
-    get isModal() {
+    public get isModal() {
         return this._isModal;
     }
 
-    set isModal(val: boolean) {
+    public set isModal(val: boolean) {
         this._overlayDefaultSettings.modal = val;
         this._isModal = val;
     }
 
+    /**
+     * Controls whether the dialog should close when `Esc` key is pressed. Defaults to `true`
+     * ```html
+     * <igx-dialog [closeOnEscape]="false" ></igx-dialog>
+     * ```
+     */
     @Input()
-    get closeOnEscapeKey() {
-        return this._closeOnEscapeKey;
+    public get closeOnEscape() {
+        return this._closeOnEscape;
     }
 
-    set closeOnEscapeKey(val: boolean) {
-        this._overlayDefaultSettings.closeOnEsc = val;
-        this._closeOnEscapeKey = val;
+    public set closeOnEscape(val: boolean) {
+        this._overlayDefaultSettings.closeOnEscape = val;
+        this._closeOnEscape = val;
     }
 
     /**
@@ -311,7 +323,7 @@ export class IgxDialogComponent implements IToggleView, OnInit, OnDestroy, After
 
     private _overlayDefaultSettings: OverlaySettings;
     private _closeOnOutsideSelect = false;
-    private _closeOnEscapeKey = true;
+    private _closeOnEscape = true;
     private _isModal = true;
     protected destroy$ = new Subject<boolean>();
 
@@ -415,7 +427,7 @@ export class IgxDialogComponent implements IToggleView, OnInit, OnDestroy, After
             positionStrategy: new GlobalPositionStrategy(this._positionSettings),
             scrollStrategy: new NoOpScrollStrategy(),
             modal: this.isModal,
-            closeOnEsc: this._closeOnEscapeKey,
+            closeOnEscape: this._closeOnEscape,
             closeOnOutsideClick: this.closeOnOutsideSelect
         };
     }

projects/igniteui-angular/src/lib/services/overlay/overlay.spec.ts

@@ -2718,7 +2718,7 @@ describe('igxOverlay', () => {
             const fixture = TestBed.createComponent(EmptyPageComponent);
             const overlay = fixture.componentInstance.overlay;
             const overlaySettings: OverlaySettings = {
-                closeOnEsc: true,
+                closeOnEscape: true,
             };
 
             overlay.show(overlay.attach(SimpleDynamicComponent), overlaySettings);
@@ -2738,7 +2738,7 @@ describe('igxOverlay', () => {
             const fixture = TestBed.createComponent(EmptyPageComponent);
             const overlay = fixture.componentInstance.overlay;
             const overlaySettings: OverlaySettings = {
-                closeOnEsc: false
+                closeOnEscape: false
             };
 
             overlay.show(overlay.attach(SimpleDynamicComponent), overlaySettings);
@@ -2757,9 +2757,9 @@ describe('igxOverlay', () => {
         it('Should close the opened overlays consecutively on esc keypress', fakeAsync(() => {
             const fixture = TestBed.createComponent(EmptyPageComponent);
             const overlay = fixture.componentInstance.overlay;
-            overlay.show(overlay.attach(SimpleDynamicComponent), { closeOnEsc: true });
+            overlay.show(overlay.attach(SimpleDynamicComponent), { closeOnEscape: true });
             tick();
-            overlay.show(overlay.attach(SimpleDynamicComponent), { closeOnEsc: true });
+            overlay.show(overlay.attach(SimpleDynamicComponent), { closeOnEscape: true });
             tick();
 
             const overlayDiv = document.getElementsByClassName(CLASS_OVERLAY_MAIN)[0];
@@ -2777,11 +2777,11 @@ describe('igxOverlay', () => {
         it('Should not close the opened overlays consecutively on esc keypress', fakeAsync(() => {
             const fixture = TestBed.createComponent(EmptyPageComponent);
             const overlay = fixture.componentInstance.overlay;
-            overlay.show(overlay.attach(SimpleDynamicComponent), { closeOnEsc: true });
+            overlay.show(overlay.attach(SimpleDynamicComponent), { closeOnEscape: true });
             tick();
-            overlay.show(overlay.attach(SimpleDynamicComponent), { closeOnEsc: false });
+            overlay.show(overlay.attach(SimpleDynamicComponent), { closeOnEscape: false });
             tick();
-            overlay.show(overlay.attach(SimpleDynamicComponent), { closeOnEsc: true });
+            overlay.show(overlay.attach(SimpleDynamicComponent), { closeOnEscape: true });
             tick();
 
             const overlayDiv = document.getElementsByClassName(CLASS_OVERLAY_MAIN)[0];
@@ -2801,7 +2801,7 @@ describe('igxOverlay', () => {
             const fixture = TestBed.createComponent(EmptyPageComponent);
             const overlay = fixture.componentInstance.overlay;
             const overlaySettings: OverlaySettings = {
-                closeOnEsc: true,
+                closeOnEscape: true,
             };
 
             overlay.show(overlay.attach(SimpleDynamicComponent), overlaySettings);

projects/igniteui-angular/src/lib/services/overlay/overlay.ts

@@ -1,15 +1,5 @@
+import { AnimationAnimateRefMetadata, AnimationBuilder, AnimationMetadataType, AnimationReferenceMetadata } from '@angular/animations';
 import { DOCUMENT } from '@angular/common';
-import { GlobalPositionStrategy } from './position/global-position-strategy';
-import { NoOpScrollStrategy } from './scroll/NoOpScrollStrategy';
-import {
-    OverlaySettings,
-    OverlayEventArgs,
-    OverlayInfo,
-    OverlayAnimationEventArgs,
-    OverlayCancelableEventArgs,
-    OverlayClosingEventArgs
-} from './utilities';
-
 import {
     ApplicationRef,
     ComponentFactory,
@@ -20,17 +10,22 @@ import {
     Inject,
     Injectable,
     Injector,
-    Type,
-    OnDestroy,
     NgModuleRef,
-    NgZone
+    NgZone, OnDestroy, Type
 } from '@angular/core';
-import { AnimationBuilder, AnimationReferenceMetadata, AnimationMetadataType, AnimationAnimateRefMetadata } from '@angular/animations';
-import { fromEvent, Subject } from 'rxjs';
+import { fromEvent, Subject, Subscription } from 'rxjs';
 import { filter, takeUntil } from 'rxjs/operators';
 import { IAnimationParams } from '../../animations/main';
 import { showMessage } from '../../core/deprecateDecorators';
-import { FromEventTarget } from 'rxjs/internal/observable/fromEvent';
+import { GlobalPositionStrategy } from './position/global-position-strategy';
+import { NoOpScrollStrategy } from './scroll/NoOpScrollStrategy';
+import {
+    OverlayAnimationEventArgs,
+    OverlayCancelableEventArgs,
+    OverlayClosingEventArgs, OverlayEventArgs,
+    OverlayInfo, OverlaySettings
+} from './utilities';
+
 
 let warningShown = false;
 
@@ -44,14 +39,15 @@ export class IgxOverlayService implements OnDestroy {
     private _overlayInfos: OverlayInfo[] = [];
     private _overlayElement: HTMLElement;
     private _document: Document;
+    private _keyPressEventListener: Subscription;
     private destroy$ = new Subject<boolean>();
 
     private _defaultSettings: OverlaySettings = {
         positionStrategy: new GlobalPositionStrategy(),
         scrollStrategy: new NoOpScrollStrategy(),
         modal: true,
         closeOnOutsideClick: true,
-        closeOnEsc: false
+        closeOnEscape: false
     };
 
     /**
@@ -322,7 +318,7 @@ export class IgxOverlayService implements OnDestroy {
         }
 
         this.addOutsideClickListener(info);
-        this.addResizeHandler(info.id);
+        this.addResizeHandler();
         this.addCloseOnEscapeListener();
 
         if (info.settings.modal) {
@@ -359,7 +355,8 @@ export class IgxOverlayService implements OnDestroy {
         //  TODO: synchronize where these are added/attached and where removed/detached
         info.settings.scrollStrategy.detach();
         this.removeOutsideClickListener(info);
-        this.removeResizeHandler(info.id);
+        this.removeResizeHandler();
+        this.removeCloseOnEscapeListener();
 
         const child: HTMLElement = info.elementRef.nativeElement;
         if (info.settings.modal) {
@@ -706,7 +703,7 @@ export class IgxOverlayService implements OnDestroy {
         }
     }
 
-    private addResizeHandler(id: string) {
+    private addResizeHandler() {
         const closingOverlaysCount =
             this._overlayInfos
                 .filter(o => o.closeAnimationPlayer && o.closeAnimationPlayer.hasStarted())
@@ -716,7 +713,7 @@ export class IgxOverlayService implements OnDestroy {
         }
     }
 
-    private removeResizeHandler(id: string) {
+    private removeResizeHandler() {
         const closingOverlaysCount =
             this._overlayInfos
                 .filter(o => o.closeAnimationPlayer && o.closeAnimationPlayer.hasStarted())
@@ -730,18 +727,27 @@ export class IgxOverlayService implements OnDestroy {
         //  if all overlays minus closing overlays equals one add the handler
         if (this._overlayInfos.length - this._overlayInfos.filter(x => x.closeAnimationPlayer
             && x.closeAnimationPlayer.hasStarted()).length === 1) {
-            fromEvent(this._document, 'keydown').pipe(
-                filter((ev: KeyboardEvent) => ev.key === 'Escape' || ev.key === 'Esc'),
-                takeUntil(this.destroy$)
+            this._keyPressEventListener = fromEvent(this._document, 'keydown').pipe(
+                filter((ev: KeyboardEvent) => ev.key === 'Escape' || ev.key === 'Esc')
             ).subscribe(() => {
                 const targetOverlay = this._overlayInfos[this._overlayInfos.length - 1];
-                if (targetOverlay.settings.closeOnEsc) {
+                if (targetOverlay.settings.closeOnEscape) {
                     this.hide(targetOverlay.id);
                 }
             });
         }
     }
 
+    private removeCloseOnEscapeListener() {
+        const closingOverlaysCount =
+            this._overlayInfos
+                .filter(o => o.closeAnimationPlayer && o.closeAnimationPlayer.hasStarted())
+                .length;
+        if (this._overlayInfos.length - closingOverlaysCount === 1 && !this._keyPressEventListener.closed) {
+            this._keyPressEventListener.unsubscribe();
+        }
+    }
+
     /** @hidden */
     public repositionAll = () => {
         for (let i = this._overlayInfos.length; i--;) {

projects/igniteui-angular/src/lib/services/overlay/utilities.ts

@@ -59,7 +59,7 @@ export interface OverlaySettings {
     /** Set if the overlay should close on outside click */
     closeOnOutsideClick?: boolean;
     /** Set if the overlay should close when `Esc` key is pressed */
-    closeOnEsc?: boolean;
+    closeOnEscape?: boolean;
     /** Set the outlet container to attach the overlay to */
     outlet?: IgxOverlayOutletDirective | ElementRef;
     /**