From 8dfd2ee77646073da4c38cdccf790dd127f6594b Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Thu, 4 Oct 2018 18:49:58 +0200 Subject: [PATCH] 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()`. --- 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 1b93e64e8b1..b1ae2efd0c0 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 12a3f16c613..06808691426 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); }); });