From 39e59283a8daea105b7e73c71acd398c2d9455bd Mon Sep 17 00:00:00 2001
From: crisbeto <crisbeto@abv.bg>
Date: Wed, 3 Oct 2018 22:03:10 +0200
Subject: [PATCH] fix(autocomplete): closing parent overlay when pressing escpe

Currently we only listen for keydown events on the autocomplete trigger, however because the escape handler doesn't go through the `OverlayKeyboardDispatcher`, it means parent overlay will still receive the event, causing unintended side effects like the parent dialog closing when escape is pressed on an autocomplete. These changes switch the closing actions to go through the `OverlayRef.keydownEvents()`.
---
 src/lib/autocomplete/autocomplete-trigger.ts | 19 ++++++++++++-------
 src/lib/autocomplete/autocomplete.spec.ts    | 14 +++++---------
 2 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/src/lib/autocomplete/autocomplete-trigger.ts b/src/lib/autocomplete/autocomplete-trigger.ts
index 1b93e64e8b1f..b1ae2efd0c00 100644
--- a/src/lib/autocomplete/autocomplete-trigger.ts
+++ b/src/lib/autocomplete/autocomplete-trigger.ts
@@ -365,13 +365,7 @@ export class MatAutocompleteTrigger implements ControlValueAccessor, OnDestroy {
       event.preventDefault();
     }
 
-    // Close when pressing ESCAPE or ALT + UP_ARROW, based on the a11y guidelines.
-    // See: https://www.w3.org/TR/wai-aria-practices-1.1/#textbox-keyboard-interaction
-    if (this.panelOpen && (keyCode === ESCAPE || (keyCode === UP_ARROW && event.altKey))) {
-      this._resetActiveItem();
-      this._closeKeyEventStream.next();
-      event.stopPropagation();
-    } else if (this.activeOption && keyCode === ENTER && this.panelOpen) {
+    if (this.activeOption && keyCode === ENTER && this.panelOpen) {
       this.activeOption._selectViaInteraction();
       this._resetActiveItem();
       event.preventDefault();
@@ -574,6 +568,17 @@ export class MatAutocompleteTrigger implements ControlValueAccessor, OnDestroy {
       this._portal = new TemplatePortal(this.autocomplete.template, this._viewContainerRef);
       this._overlayRef = this._overlay.create(this._getOverlayConfig());
 
+      // Use the `keydownEvents` in order to take advantage of
+      // the overlay event targeting provided by the CDK overlay.
+      this._overlayRef.keydownEvents().subscribe(event => {
+        // Close when pressing ESCAPE or ALT + UP_ARROW, based on the a11y guidelines.
+        // See: https://www.w3.org/TR/wai-aria-practices-1.1/#textbox-keyboard-interaction
+        if (event.keyCode === ESCAPE || (event.keyCode === UP_ARROW && event.altKey)) {
+          this._resetActiveItem();
+          this._closeKeyEventStream.next();
+        }
+      });
+
       if (this._viewportRuler) {
         this._viewportSubscription = this._viewportRuler.change().subscribe(() => {
           if (this.panelOpen && this._overlayRef) {
diff --git a/src/lib/autocomplete/autocomplete.spec.ts b/src/lib/autocomplete/autocomplete.spec.ts
index 12a3f16c613a..068086914261 100644
--- a/src/lib/autocomplete/autocomplete.spec.ts
+++ b/src/lib/autocomplete/autocomplete.spec.ts
@@ -8,6 +8,7 @@ import {
   dispatchKeyboardEvent,
   MockNgZone,
   typeInElement,
+  dispatchEvent,
 } from '@angular/cdk/testing';
 import {
   ChangeDetectionStrategy,
@@ -1043,8 +1044,6 @@ describe('MatAutocomplete', () => {
 
     it('should close the panel when pressing escape', fakeAsync(() => {
       const trigger = fixture.componentInstance.trigger;
-      const escapeEvent = createKeyboardEvent('keydown', ESCAPE);
-      const stopPropagationSpy = spyOn(escapeEvent, 'stopPropagation').and.callThrough();
 
       input.focus();
       flush();
@@ -1053,12 +1052,11 @@ describe('MatAutocomplete', () => {
       expect(document.activeElement).toBe(input, 'Expected input to be focused.');
       expect(trigger.panelOpen).toBe(true, 'Expected panel to be open.');
 
-      trigger._handleKeydown(escapeEvent);
+      dispatchKeyboardEvent(document.body, 'keydown', ESCAPE);
       fixture.detectChanges();
 
       expect(document.activeElement).toBe(input, 'Expected input to continue to be focused.');
       expect(trigger.panelOpen).toBe(false, 'Expected panel to be closed.');
-      expect(stopPropagationSpy).toHaveBeenCalled();
     }));
 
     it('should prevent the default action when pressing escape', fakeAsync(() => {
@@ -1080,7 +1078,7 @@ describe('MatAutocomplete', () => {
       expect(document.activeElement).toBe(input, 'Expected input to be focused.');
       expect(trigger.panelOpen).toBe(true, 'Expected panel to be open.');
 
-      trigger._handleKeydown(upArrowEvent);
+      dispatchEvent(document.body, upArrowEvent);
       fixture.detectChanges();
 
       expect(document.activeElement).toBe(input, 'Expected input to continue to be focused.');
@@ -1125,7 +1123,7 @@ describe('MatAutocomplete', () => {
       // from crashing when trying to stringify the option if the test fails.
       expect(!!trigger.activeOption).toBe(true, 'Expected to find an active option.');
 
-      trigger._handleKeydown(createKeyboardEvent('keydown', ESCAPE));
+      dispatchKeyboardEvent(document.body, 'keydown', ESCAPE);
       tick();
 
       expect(!!trigger.activeOption).toBe(false, 'Expected no active options.');
@@ -1782,10 +1780,8 @@ describe('MatAutocomplete', () => {
     });
 
     it('should close the panel when pressing escape', () => {
-      const escapeEvent = createKeyboardEvent('keydown', ESCAPE);
-
       expect(closingActionSpy).not.toHaveBeenCalled();
-      trigger._handleKeydown(escapeEvent);
+      dispatchKeyboardEvent(document.body, 'keydown', ESCAPE);
       expect(closingActionSpy).toHaveBeenCalledWith(null);
     });
   });