From 444c0867073bc6e1a6b93316610fda3b6a3f8390 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Tue, 14 Dec 2021 17:29:15 +0100 Subject: [PATCH] fix(material/tooltip): not closing if escape is pressed while trigger isn't focused Currently the tooltip's `keydown` handler is on the trigger, which means that it won't fire if the trigger doesn't have focus. This could happen when a tooltip was opened by hovering over the trigger. These changes use the `OverlayKeyboardDispatcher` to handle the events instead. Fixes #14278. --- .../mdc-tooltip/tooltip.spec.ts | 25 +++++++++++++++-- src/material/tooltip/tooltip.spec.ts | 25 +++++++++++++++-- src/material/tooltip/tooltip.ts | 28 ++++++++----------- 3 files changed, 55 insertions(+), 23 deletions(-) diff --git a/src/material-experimental/mdc-tooltip/tooltip.spec.ts b/src/material-experimental/mdc-tooltip/tooltip.spec.ts index 4c2462d62bd0..272e70e165ef 100644 --- a/src/material-experimental/mdc-tooltip/tooltip.spec.ts +++ b/src/material-experimental/mdc-tooltip/tooltip.spec.ts @@ -697,9 +697,28 @@ describe('MDC-based MatTooltip', () => { expect(overlayContainerElement.textContent).toContain(initialTooltipMessage); })); + it('should hide when pressing escape', fakeAsync(() => { + tooltipDirective.show(); + tick(0); + fixture.detectChanges(); + tick(500); + + expect(tooltipDirective._isTooltipVisible()).toBe(true); + expect(overlayContainerElement.textContent).toContain(initialTooltipMessage); + + dispatchKeyboardEvent(document.body, 'keydown', ESCAPE); + tick(0); + fixture.detectChanges(); + tick(500); + fixture.detectChanges(); + + expect(tooltipDirective._isTooltipVisible()).toBe(false); + expect(overlayContainerElement.textContent).toBe(''); + })); + it('should not throw when pressing ESCAPE', fakeAsync(() => { expect(() => { - dispatchKeyboardEvent(buttonElement, 'keydown', ESCAPE); + dispatchKeyboardEvent(document.body, 'keydown', ESCAPE); fixture.detectChanges(); }).not.toThrow(); @@ -712,7 +731,7 @@ describe('MDC-based MatTooltip', () => { tick(0); fixture.detectChanges(); - const event = dispatchKeyboardEvent(buttonElement, 'keydown', ESCAPE); + const event = dispatchKeyboardEvent(document.body, 'keydown', ESCAPE); fixture.detectChanges(); flush(); @@ -725,7 +744,7 @@ describe('MDC-based MatTooltip', () => { fixture.detectChanges(); const event = createKeyboardEvent('keydown', ESCAPE, undefined, {alt: true}); - dispatchEvent(buttonElement, event); + dispatchEvent(document.body, event); fixture.detectChanges(); flush(); diff --git a/src/material/tooltip/tooltip.spec.ts b/src/material/tooltip/tooltip.spec.ts index d784e06b7984..c8118c52de15 100644 --- a/src/material/tooltip/tooltip.spec.ts +++ b/src/material/tooltip/tooltip.spec.ts @@ -693,9 +693,28 @@ describe('MatTooltip', () => { expect(overlayContainerElement.textContent).toContain(initialTooltipMessage); })); + it('should hide when pressing escape', fakeAsync(() => { + tooltipDirective.show(); + tick(0); + fixture.detectChanges(); + tick(500); + + expect(tooltipDirective._isTooltipVisible()).toBe(true); + expect(overlayContainerElement.textContent).toContain(initialTooltipMessage); + + dispatchKeyboardEvent(document.body, 'keydown', ESCAPE); + tick(0); + fixture.detectChanges(); + tick(500); + fixture.detectChanges(); + + expect(tooltipDirective._isTooltipVisible()).toBe(false); + expect(overlayContainerElement.textContent).toBe(''); + })); + it('should not throw when pressing ESCAPE', fakeAsync(() => { expect(() => { - dispatchKeyboardEvent(buttonElement, 'keydown', ESCAPE); + dispatchKeyboardEvent(document.body, 'keydown', ESCAPE); fixture.detectChanges(); }).not.toThrow(); @@ -708,7 +727,7 @@ describe('MatTooltip', () => { tick(0); fixture.detectChanges(); - const event = dispatchKeyboardEvent(buttonElement, 'keydown', ESCAPE); + const event = dispatchKeyboardEvent(document.body, 'keydown', ESCAPE); fixture.detectChanges(); flush(); @@ -721,7 +740,7 @@ describe('MatTooltip', () => { fixture.detectChanges(); const event = createKeyboardEvent('keydown', ESCAPE, undefined, {alt: true}); - dispatchEvent(buttonElement, event); + dispatchEvent(document.body, event); fixture.detectChanges(); flush(); diff --git a/src/material/tooltip/tooltip.ts b/src/material/tooltip/tooltip.ts index 55a735e9aaad..b4b46ec87a9c 100644 --- a/src/material/tooltip/tooltip.ts +++ b/src/material/tooltip/tooltip.ts @@ -295,10 +295,6 @@ export abstract class _MatTooltipBase this._updatePosition(this._overlayRef); } }); - - _ngZone.runOutsideAngular(() => { - _elementRef.nativeElement.addEventListener('keydown', this._handleKeydown); - }); } ngAfterViewInit() { @@ -333,7 +329,6 @@ export abstract class _MatTooltipBase } // Clean up the event listeners set in the constructor - nativeElement.removeEventListener('keydown', this._handleKeydown); this._passiveListeners.forEach(([event, listener]) => { nativeElement.removeEventListener(event, listener, passiveListenerOptions); }); @@ -389,18 +384,6 @@ export abstract class _MatTooltipBase return !!this._tooltipInstance && this._tooltipInstance.isVisible(); } - /** - * Handles the keydown events on the host element. - * Needs to be an arrow function so that we can use it in addEventListener. - */ - private _handleKeydown = (event: KeyboardEvent) => { - if (this._isTooltipVisible() && event.keyCode === ESCAPE && !hasModifierKey(event)) { - event.preventDefault(); - event.stopPropagation(); - this._ngZone.run(() => this.hide(0)); - } - }; - /** Create the overlay config and position strategy */ private _createOverlay(): OverlayRef { if (this._overlayRef) { @@ -451,6 +434,17 @@ export abstract class _MatTooltipBase .pipe(takeUntil(this._destroyed)) .subscribe(() => this._tooltipInstance?._handleBodyInteraction()); + this._overlayRef + .keydownEvents() + .pipe(takeUntil(this._destroyed)) + .subscribe(event => { + if (this._isTooltipVisible() && event.keyCode === ESCAPE && !hasModifierKey(event)) { + event.preventDefault(); + event.stopPropagation(); + this._ngZone.run(() => this.hide(0)); + } + }); + return this._overlayRef; }