fix(autocomplete): closing parent overlay when pressing escpe (#13413)

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()`.
angular · Oct 4, 2018 · 8dfd2ee · 8dfd2ee
1 parent a620fca
commit 8dfd2ee
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 16 deletions.
diff --git 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
@@ -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);
     });
   });