From 1d7e4763f82386202a311fe23ac9b3edb229efe8 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Tue, 11 Jul 2023 13:54:02 +0200 Subject: [PATCH] fix(material/autocomplete): blocking events to other overlays when there are no results The autocomplete trigger attaches an overlay even if there are no options in the list. It also subscribes to keydown events and clicks while the panel is open which block events from reaching other overlays. This means that if the user focuses an autocomplete input with no options, the events from it won't reach other overlays. These changes resolve the issue by subcribing and unsubscribing from the event streams depending on the visibility state of the panel. Fixes #26479. --- .../autocomplete/autocomplete-trigger.ts | 95 +++++++++++-------- .../autocomplete/autocomplete.spec.ts | 15 ++- 2 files changed, 72 insertions(+), 38 deletions(-) diff --git a/src/material/autocomplete/autocomplete-trigger.ts b/src/material/autocomplete/autocomplete-trigger.ts index 52d08ca44019..67eb0a8489cc 100644 --- a/src/material/autocomplete/autocomplete-trigger.ts +++ b/src/material/autocomplete/autocomplete-trigger.ts @@ -106,6 +106,8 @@ export abstract class _MatAutocompleteTriggerBase private _componentDestroyed = false; private _autocompleteDisabled = false; private _scrollStrategy: () => ScrollStrategy; + private _keydownSubscription: Subscription | null; + private _outsideClickSubscription: Subscription | null; /** Old value of the native input. Used to work around issues with the `input` event on IE. */ private _previousValue: string | number | null; @@ -286,6 +288,8 @@ export abstract class _MatAutocompleteTriggerBase this._closingActionsSubscription.unsubscribe(); } + this._updatePanelState(); + // Note that in some cases this can end up being called after the component is destroyed. // Add a check to ensure that we don't try to run change detection on a destroyed view. if (!this._componentDestroyed) { @@ -545,7 +549,7 @@ export abstract class _MatAutocompleteTriggerBase this._zone.run(() => { const wasOpen = this.panelOpen; this._resetActiveItem(); - this.autocomplete._setVisibility(); + this._updatePanelState(); this._changeDetectorRef.detectChanges(); if (this.panelOpen) { @@ -655,7 +659,6 @@ export abstract class _MatAutocompleteTriggerBase }); overlayRef = this._overlay.create(this._getOverlayConfig()); this._overlayRef = overlayRef; - this._handleOverlayEvents(overlayRef); this._viewportSubscription = this._viewportRuler.change().subscribe(() => { if (this.panelOpen && overlayRef) { overlayRef.updateSize({width: this._getPanelWidth()}); @@ -674,9 +677,9 @@ export abstract class _MatAutocompleteTriggerBase const wasOpen = this.panelOpen; - this.autocomplete._setVisibility(); this.autocomplete._isOpen = this._overlayAttached = true; this.autocomplete._setColor(this._formField?.color); + this._updatePanelState(); this._applyModalPanelOwnership(); @@ -687,6 +690,58 @@ export abstract class _MatAutocompleteTriggerBase } } + /** Handles keyboard events coming from the overlay panel. */ + private _handlePanelKeydown = (event: KeyboardEvent) => { + // 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 && !hasModifierKey(event)) || + (event.keyCode === UP_ARROW && hasModifierKey(event, 'altKey')) + ) { + // If the user had typed something in before we autoselected an option, and they decided + // to cancel the selection, restore the input value to the one they had typed in. + if (this._pendingAutoselectedOption) { + this._updateNativeInputValue(this._valueBeforeAutoSelection ?? ''); + this._pendingAutoselectedOption = null; + } + this._closeKeyEventStream.next(); + this._resetActiveItem(); + // We need to stop propagation, otherwise the event will eventually + // reach the input itself and cause the overlay to be reopened. + event.stopPropagation(); + event.preventDefault(); + } + }; + + /** Updates the panel's visibility state and any trigger state tied to id. */ + private _updatePanelState() { + this.autocomplete._setVisibility(); + + // Note that here we subscribe and unsubscribe based on the panel's visiblity state, + // because the act of subscribing will prevent events from reaching other overlays and + // we don't want to block the events if there are no options. + if (this.panelOpen) { + const overlayRef = this._overlayRef!; + + if (!this._keydownSubscription) { + // Use the `keydownEvents` in order to take advantage of + // the overlay event targeting provided by the CDK overlay. + this._keydownSubscription = overlayRef.keydownEvents().subscribe(this._handlePanelKeydown); + } + + if (!this._outsideClickSubscription) { + // Subscribe to the pointer events stream so that it doesn't get picked up by other overlays. + // TODO(crisbeto): we should switch `_getOutsideClickStream` eventually to use this stream, + // but the behvior isn't exactly the same and it ends up breaking some internal tests. + this._outsideClickSubscription = overlayRef.outsidePointerEvents().subscribe(); + } + } else { + this._keydownSubscription?.unsubscribe(); + this._outsideClickSubscription?.unsubscribe(); + this._keydownSubscription = this._outsideClickSubscription = null; + } + } + private _getOverlayConfig(): OverlayConfig { return new OverlayConfig({ positionStrategy: this._getOverlayPosition(), @@ -835,40 +890,6 @@ export abstract class _MatAutocompleteTriggerBase } } - /** Handles keyboard events coming from the overlay panel. */ - private _handleOverlayEvents(overlayRef: OverlayRef) { - // Use the `keydownEvents` in order to take advantage of - // the overlay event targeting provided by the CDK overlay. - 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 && !hasModifierKey(event)) || - (event.keyCode === UP_ARROW && hasModifierKey(event, 'altKey')) - ) { - // If the user had typed something in before we autoselected an option, and they decided - // to cancel the selection, restore the input value to the one they had typed in. - if (this._pendingAutoselectedOption) { - this._updateNativeInputValue(this._valueBeforeAutoSelection ?? ''); - this._pendingAutoselectedOption = null; - } - - this._closeKeyEventStream.next(); - this._resetActiveItem(); - - // We need to stop propagation, otherwise the event will eventually - // reach the input itself and cause the overlay to be reopened. - event.stopPropagation(); - event.preventDefault(); - } - }); - - // Subscribe to the pointer events stream so that it doesn't get picked up by other overlays. - // TODO(crisbeto): we should switch `_getOutsideClickStream` eventually to use this stream, - // but the behvior isn't exactly the same and it ends up breaking some internal tests. - overlayRef.outsidePointerEvents().subscribe(); - } - /** * Track which modal we have modified the `aria-owns` attribute of. When the combobox trigger is * inside an aria-modal, we apply aria-owns to the parent modal with the `id` of the options diff --git a/src/material/autocomplete/autocomplete.spec.ts b/src/material/autocomplete/autocomplete.spec.ts index 4e2f4b885a40..e8e7842d009d 100644 --- a/src/material/autocomplete/autocomplete.spec.ts +++ b/src/material/autocomplete/autocomplete.spec.ts @@ -570,7 +570,7 @@ describe('MDC-based MatAutocomplete', () => { expect(input.hasAttribute('aria-haspopup')).toBe(false); }); - it('should close the panel when pressing escape', fakeAsync(() => { + it('should reopen the panel when clicking on the input', fakeAsync(() => { const trigger = fixture.componentInstance.trigger; input.focus(); @@ -2577,6 +2577,19 @@ describe('MDC-based MatAutocomplete', () => { dispatchKeyboardEvent(document.body, 'keydown', ESCAPE); expect(closingActionSpy).toHaveBeenCalledWith(null); }); + + it('should not prevent escape key propagation when there are no options', () => { + fixture.componentInstance.filteredStates = fixture.componentInstance.states = []; + fixture.detectChanges(); + zone.simulateZoneExit(); + + const event = createKeyboardEvent('keydown', ESCAPE); + spyOn(event, 'stopPropagation').and.callThrough(); + dispatchEvent(document.body, event); + fixture.detectChanges(); + + expect(event.stopPropagation).not.toHaveBeenCalled(); + }); }); describe('without matInput', () => {