From b50ab131c2ea383d75b7ae8ac1dd12de1d4de52a Mon Sep 17 00:00:00 2001 From: crisbeto Date: Sat, 21 Sep 2019 14:56:45 +0200 Subject: [PATCH] fix(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 | 26 ++++++++++---------- tools/public_api_guard/material/tooltip.d.ts | 1 - 3 files changed, 35 insertions(+), 17 deletions(-) diff --git a/src/material/tooltip/tooltip.spec.ts b/src/material/tooltip/tooltip.spec.ts index fabdcd6af7f0..15af6e657635 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(); @@ -668,7 +687,7 @@ describe('MatTooltip', () => { const event = createKeyboardEvent('keydown', ESCAPE); Object.defineProperty(event, 'altKey', {get: () => 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 803efc4ec663..36926fe120e4 100644 --- a/src/material/tooltip/tooltip.ts +++ b/src/material/tooltip/tooltip.ts @@ -114,7 +114,6 @@ export function MAT_TOOLTIP_DEFAULT_OPTIONS_FACTORY(): MatTooltipDefaultOptions exportAs: 'matTooltip', host: { '(longpress)': 'show()', - '(keydown)': '_handleKeydown($event)', '(touchend)': '_handleTouchend()', }, }) @@ -337,15 +336,6 @@ export class MatTooltip implements OnDestroy, OnInit { return !!this._tooltipInstance && this._tooltipInstance.isVisible(); } - /** Handles the keydown events on the host element. */ - _handleKeydown(e: KeyboardEvent) { - if (this._isTooltipVisible() && e.keyCode === ESCAPE && !hasModifierKey(e)) { - e.preventDefault(); - e.stopPropagation(); - this.hide(0); - } - } - /** Handles the touchend events on the host element. */ _handleTouchend() { this.hide(this._defaultOptions.touchendHideDelay); @@ -378,7 +368,7 @@ export class MatTooltip implements OnDestroy, OnInit { } }); - this._overlayRef = this._overlay.create({ + const overlayRef = this._overlayRef = this._overlay.create({ direction: this._dir, positionStrategy: strategy, panelClass: TOOLTIP_PANEL_CLASS, @@ -387,11 +377,21 @@ export class MatTooltip implements OnDestroy, OnInit { 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.hide(0); + } + }); + + overlayRef.detachments() .pipe(takeUntil(this._destroyed)) .subscribe(() => this._detach()); - return this._overlayRef; + return overlayRef; } /** Detaches the currently-attached tooltip. */ diff --git a/tools/public_api_guard/material/tooltip.d.ts b/tools/public_api_guard/material/tooltip.d.ts index 849be20c555b..2c65c59f3701 100644 --- a/tools/public_api_guard/material/tooltip.d.ts +++ b/tools/public_api_guard/material/tooltip.d.ts @@ -34,7 +34,6 @@ export declare class MatTooltip implements OnDestroy, OnInit { main: OverlayConnectionPosition; fallback: OverlayConnectionPosition; }; - _handleKeydown(e: KeyboardEvent): void; _handleTouchend(): void; _isTooltipVisible(): boolean; hide(delay?: number): void;