Skip to content

Commit 444c086

Browse files
committed
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.
1 parent 97ec228 commit 444c086

File tree

3 files changed

+55
-23
lines changed

3 files changed

+55
-23
lines changed

src/material-experimental/mdc-tooltip/tooltip.spec.ts

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -697,9 +697,28 @@ describe('MDC-based MatTooltip', () => {
697697
expect(overlayContainerElement.textContent).toContain(initialTooltipMessage);
698698
}));
699699

700+
it('should hide when pressing escape', fakeAsync(() => {
701+
tooltipDirective.show();
702+
tick(0);
703+
fixture.detectChanges();
704+
tick(500);
705+
706+
expect(tooltipDirective._isTooltipVisible()).toBe(true);
707+
expect(overlayContainerElement.textContent).toContain(initialTooltipMessage);
708+
709+
dispatchKeyboardEvent(document.body, 'keydown', ESCAPE);
710+
tick(0);
711+
fixture.detectChanges();
712+
tick(500);
713+
fixture.detectChanges();
714+
715+
expect(tooltipDirective._isTooltipVisible()).toBe(false);
716+
expect(overlayContainerElement.textContent).toBe('');
717+
}));
718+
700719
it('should not throw when pressing ESCAPE', fakeAsync(() => {
701720
expect(() => {
702-
dispatchKeyboardEvent(buttonElement, 'keydown', ESCAPE);
721+
dispatchKeyboardEvent(document.body, 'keydown', ESCAPE);
703722
fixture.detectChanges();
704723
}).not.toThrow();
705724

@@ -712,7 +731,7 @@ describe('MDC-based MatTooltip', () => {
712731
tick(0);
713732
fixture.detectChanges();
714733

715-
const event = dispatchKeyboardEvent(buttonElement, 'keydown', ESCAPE);
734+
const event = dispatchKeyboardEvent(document.body, 'keydown', ESCAPE);
716735
fixture.detectChanges();
717736
flush();
718737

@@ -725,7 +744,7 @@ describe('MDC-based MatTooltip', () => {
725744
fixture.detectChanges();
726745

727746
const event = createKeyboardEvent('keydown', ESCAPE, undefined, {alt: true});
728-
dispatchEvent(buttonElement, event);
747+
dispatchEvent(document.body, event);
729748
fixture.detectChanges();
730749
flush();
731750

src/material/tooltip/tooltip.spec.ts

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -693,9 +693,28 @@ describe('MatTooltip', () => {
693693
expect(overlayContainerElement.textContent).toContain(initialTooltipMessage);
694694
}));
695695

696+
it('should hide when pressing escape', fakeAsync(() => {
697+
tooltipDirective.show();
698+
tick(0);
699+
fixture.detectChanges();
700+
tick(500);
701+
702+
expect(tooltipDirective._isTooltipVisible()).toBe(true);
703+
expect(overlayContainerElement.textContent).toContain(initialTooltipMessage);
704+
705+
dispatchKeyboardEvent(document.body, 'keydown', ESCAPE);
706+
tick(0);
707+
fixture.detectChanges();
708+
tick(500);
709+
fixture.detectChanges();
710+
711+
expect(tooltipDirective._isTooltipVisible()).toBe(false);
712+
expect(overlayContainerElement.textContent).toBe('');
713+
}));
714+
696715
it('should not throw when pressing ESCAPE', fakeAsync(() => {
697716
expect(() => {
698-
dispatchKeyboardEvent(buttonElement, 'keydown', ESCAPE);
717+
dispatchKeyboardEvent(document.body, 'keydown', ESCAPE);
699718
fixture.detectChanges();
700719
}).not.toThrow();
701720

@@ -708,7 +727,7 @@ describe('MatTooltip', () => {
708727
tick(0);
709728
fixture.detectChanges();
710729

711-
const event = dispatchKeyboardEvent(buttonElement, 'keydown', ESCAPE);
730+
const event = dispatchKeyboardEvent(document.body, 'keydown', ESCAPE);
712731
fixture.detectChanges();
713732
flush();
714733

@@ -721,7 +740,7 @@ describe('MatTooltip', () => {
721740
fixture.detectChanges();
722741

723742
const event = createKeyboardEvent('keydown', ESCAPE, undefined, {alt: true});
724-
dispatchEvent(buttonElement, event);
743+
dispatchEvent(document.body, event);
725744
fixture.detectChanges();
726745
flush();
727746

src/material/tooltip/tooltip.ts

Lines changed: 11 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -295,10 +295,6 @@ export abstract class _MatTooltipBase<T extends _TooltipComponentBase>
295295
this._updatePosition(this._overlayRef);
296296
}
297297
});
298-
299-
_ngZone.runOutsideAngular(() => {
300-
_elementRef.nativeElement.addEventListener('keydown', this._handleKeydown);
301-
});
302298
}
303299

304300
ngAfterViewInit() {
@@ -333,7 +329,6 @@ export abstract class _MatTooltipBase<T extends _TooltipComponentBase>
333329
}
334330

335331
// Clean up the event listeners set in the constructor
336-
nativeElement.removeEventListener('keydown', this._handleKeydown);
337332
this._passiveListeners.forEach(([event, listener]) => {
338333
nativeElement.removeEventListener(event, listener, passiveListenerOptions);
339334
});
@@ -389,18 +384,6 @@ export abstract class _MatTooltipBase<T extends _TooltipComponentBase>
389384
return !!this._tooltipInstance && this._tooltipInstance.isVisible();
390385
}
391386

392-
/**
393-
* Handles the keydown events on the host element.
394-
* Needs to be an arrow function so that we can use it in addEventListener.
395-
*/
396-
private _handleKeydown = (event: KeyboardEvent) => {
397-
if (this._isTooltipVisible() && event.keyCode === ESCAPE && !hasModifierKey(event)) {
398-
event.preventDefault();
399-
event.stopPropagation();
400-
this._ngZone.run(() => this.hide(0));
401-
}
402-
};
403-
404387
/** Create the overlay config and position strategy */
405388
private _createOverlay(): OverlayRef {
406389
if (this._overlayRef) {
@@ -451,6 +434,17 @@ export abstract class _MatTooltipBase<T extends _TooltipComponentBase>
451434
.pipe(takeUntil(this._destroyed))
452435
.subscribe(() => this._tooltipInstance?._handleBodyInteraction());
453436

437+
this._overlayRef
438+
.keydownEvents()
439+
.pipe(takeUntil(this._destroyed))
440+
.subscribe(event => {
441+
if (this._isTooltipVisible() && event.keyCode === ESCAPE && !hasModifierKey(event)) {
442+
event.preventDefault();
443+
event.stopPropagation();
444+
this._ngZone.run(() => this.hide(0));
445+
}
446+
});
447+
454448
return this._overlayRef;
455449
}
456450

0 commit comments

Comments
 (0)