From 1489a3b34ba97f221603d922879c6066dab0c7b9 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Wed, 23 Dec 2020 17:08:49 +0200 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. --- src/material/tooltip/tooltip.spec.ts | 25 ++++++++++++++++++--- src/material/tooltip/tooltip.ts | 33 +++++++++++----------------- 2 files changed, 35 insertions(+), 23 deletions(-) diff --git a/src/material/tooltip/tooltip.spec.ts b/src/material/tooltip/tooltip.spec.ts index d4e7765c28e1..e3992969cdba 100644 --- a/src/material/tooltip/tooltip.spec.ts +++ b/src/material/tooltip/tooltip.spec.ts @@ -639,9 +639,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(); @@ -654,7 +673,7 @@ describe('MatTooltip', () => { tick(0); fixture.detectChanges(); - const event = dispatchKeyboardEvent(buttonElement, 'keydown', ESCAPE); + const event = dispatchKeyboardEvent(document.body, 'keydown', ESCAPE); fixture.detectChanges(); flush(); @@ -667,7 +686,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 28f374f62393..18c88df189eb 100644 --- a/src/material/tooltip/tooltip.ts +++ b/src/material/tooltip/tooltip.ts @@ -286,10 +286,6 @@ export class MatTooltip implements OnDestroy, AfterViewInit { this.touchGestures = _defaultOptions.touchGestures; } } - - _ngZone.runOutsideAngular(() => { - _elementRef.nativeElement.addEventListener('keydown', this._handleKeydown); - }); } ngAfterViewInit() { @@ -323,7 +319,6 @@ export class MatTooltip implements OnDestroy, AfterViewInit { } // Clean up the event listeners set in the constructor - nativeElement.removeEventListener('keydown', this._handleKeydown); this._passiveListeners.forEach(([event, listener]) => { nativeElement.removeEventListener(event, listener, passiveListenerOptions); }); @@ -372,18 +367,6 @@ export class MatTooltip implements OnDestroy, AfterViewInit { 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) { @@ -411,7 +394,7 @@ export class MatTooltip implements OnDestroy, AfterViewInit { } }); - this._overlayRef = this._overlay.create({ + const overlayRef = this._overlayRef = this._overlay.create({ direction: this._dir, positionStrategy: strategy, panelClass: TOOLTIP_PANEL_CLASS, @@ -420,11 +403,21 @@ export class MatTooltip implements OnDestroy, AfterViewInit { this._updatePosition(); - this._overlayRef.detachments() + 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)); + } + }); + + overlayRef.detachments() .pipe(takeUntil(this._destroyed)) .subscribe(() => this._detach()); - return this._overlayRef; + return overlayRef; } /** Detaches the currently-attached tooltip. */