Skip to content

Commit 8dfd2ee

Browse files
crisbetovivian-hu-zz
authored andcommitted
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()`.
1 parent a620fca commit 8dfd2ee

File tree

2 files changed

+17
-16
lines changed

2 files changed

+17
-16
lines changed

src/lib/autocomplete/autocomplete-trigger.ts

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -365,13 +365,7 @@ export class MatAutocompleteTrigger implements ControlValueAccessor, OnDestroy {
365365
event.preventDefault();
366366
}
367367

368-
// Close when pressing ESCAPE or ALT + UP_ARROW, based on the a11y guidelines.
369-
// See: https://www.w3.org/TR/wai-aria-practices-1.1/#textbox-keyboard-interaction
370-
if (this.panelOpen && (keyCode === ESCAPE || (keyCode === UP_ARROW && event.altKey))) {
371-
this._resetActiveItem();
372-
this._closeKeyEventStream.next();
373-
event.stopPropagation();
374-
} else if (this.activeOption && keyCode === ENTER && this.panelOpen) {
368+
if (this.activeOption && keyCode === ENTER && this.panelOpen) {
375369
this.activeOption._selectViaInteraction();
376370
this._resetActiveItem();
377371
event.preventDefault();
@@ -574,6 +568,17 @@ export class MatAutocompleteTrigger implements ControlValueAccessor, OnDestroy {
574568
this._portal = new TemplatePortal(this.autocomplete.template, this._viewContainerRef);
575569
this._overlayRef = this._overlay.create(this._getOverlayConfig());
576570

571+
// Use the `keydownEvents` in order to take advantage of
572+
// the overlay event targeting provided by the CDK overlay.
573+
this._overlayRef.keydownEvents().subscribe(event => {
574+
// Close when pressing ESCAPE or ALT + UP_ARROW, based on the a11y guidelines.
575+
// See: https://www.w3.org/TR/wai-aria-practices-1.1/#textbox-keyboard-interaction
576+
if (event.keyCode === ESCAPE || (event.keyCode === UP_ARROW && event.altKey)) {
577+
this._resetActiveItem();
578+
this._closeKeyEventStream.next();
579+
}
580+
});
581+
577582
if (this._viewportRuler) {
578583
this._viewportSubscription = this._viewportRuler.change().subscribe(() => {
579584
if (this.panelOpen && this._overlayRef) {

src/lib/autocomplete/autocomplete.spec.ts

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
dispatchKeyboardEvent,
99
MockNgZone,
1010
typeInElement,
11+
dispatchEvent,
1112
} from '@angular/cdk/testing';
1213
import {
1314
ChangeDetectionStrategy,
@@ -1043,8 +1044,6 @@ describe('MatAutocomplete', () => {
10431044

10441045
it('should close the panel when pressing escape', fakeAsync(() => {
10451046
const trigger = fixture.componentInstance.trigger;
1046-
const escapeEvent = createKeyboardEvent('keydown', ESCAPE);
1047-
const stopPropagationSpy = spyOn(escapeEvent, 'stopPropagation').and.callThrough();
10481047

10491048
input.focus();
10501049
flush();
@@ -1053,12 +1052,11 @@ describe('MatAutocomplete', () => {
10531052
expect(document.activeElement).toBe(input, 'Expected input to be focused.');
10541053
expect(trigger.panelOpen).toBe(true, 'Expected panel to be open.');
10551054

1056-
trigger._handleKeydown(escapeEvent);
1055+
dispatchKeyboardEvent(document.body, 'keydown', ESCAPE);
10571056
fixture.detectChanges();
10581057

10591058
expect(document.activeElement).toBe(input, 'Expected input to continue to be focused.');
10601059
expect(trigger.panelOpen).toBe(false, 'Expected panel to be closed.');
1061-
expect(stopPropagationSpy).toHaveBeenCalled();
10621060
}));
10631061

10641062
it('should prevent the default action when pressing escape', fakeAsync(() => {
@@ -1080,7 +1078,7 @@ describe('MatAutocomplete', () => {
10801078
expect(document.activeElement).toBe(input, 'Expected input to be focused.');
10811079
expect(trigger.panelOpen).toBe(true, 'Expected panel to be open.');
10821080

1083-
trigger._handleKeydown(upArrowEvent);
1081+
dispatchEvent(document.body, upArrowEvent);
10841082
fixture.detectChanges();
10851083

10861084
expect(document.activeElement).toBe(input, 'Expected input to continue to be focused.');
@@ -1125,7 +1123,7 @@ describe('MatAutocomplete', () => {
11251123
// from crashing when trying to stringify the option if the test fails.
11261124
expect(!!trigger.activeOption).toBe(true, 'Expected to find an active option.');
11271125

1128-
trigger._handleKeydown(createKeyboardEvent('keydown', ESCAPE));
1126+
dispatchKeyboardEvent(document.body, 'keydown', ESCAPE);
11291127
tick();
11301128

11311129
expect(!!trigger.activeOption).toBe(false, 'Expected no active options.');
@@ -1782,10 +1780,8 @@ describe('MatAutocomplete', () => {
17821780
});
17831781

17841782
it('should close the panel when pressing escape', () => {
1785-
const escapeEvent = createKeyboardEvent('keydown', ESCAPE);
1786-
17871783
expect(closingActionSpy).not.toHaveBeenCalled();
1788-
trigger._handleKeydown(escapeEvent);
1784+
dispatchKeyboardEvent(document.body, 'keydown', ESCAPE);
17891785
expect(closingActionSpy).toHaveBeenCalledWith(null);
17901786
});
17911787
});

0 commit comments

Comments
 (0)