Skip to content

Commit

Permalink
fix(material/tooltip): not closing if escape is pressed while trigger…
Browse files Browse the repository at this point in the history
… isn't focused (#14434)

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.

(cherry picked from commit 08aed71)
  • Loading branch information
crisbeto authored and andrewseguin committed Jan 13, 2022
1 parent fe39b55 commit e01e579
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 23 deletions.
25 changes: 22 additions & 3 deletions src/material-experimental/mdc-tooltip/tooltip.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -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();

Expand All @@ -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();

Expand Down
25 changes: 22 additions & 3 deletions src/material/tooltip/tooltip.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -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();

Expand All @@ -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();

Expand Down
28 changes: 11 additions & 17 deletions src/material/tooltip/tooltip.ts
Original file line number Diff line number Diff line change
Expand Up @@ -314,10 +314,6 @@ export abstract class _MatTooltipBase<T extends _TooltipComponentBase>
this._updatePosition(this._overlayRef);
}
});

_ngZone.runOutsideAngular(() => {
_elementRef.nativeElement.addEventListener('keydown', this._handleKeydown);
});
}

ngAfterViewInit() {
Expand Down Expand Up @@ -352,7 +348,6 @@ export abstract class _MatTooltipBase<T extends _TooltipComponentBase>
}

// Clean up the event listeners set in the constructor
nativeElement.removeEventListener('keydown', this._handleKeydown);
this._passiveListeners.forEach(([event, listener]) => {
nativeElement.removeEventListener(event, listener, passiveListenerOptions);
});
Expand Down Expand Up @@ -408,18 +403,6 @@ export abstract class _MatTooltipBase<T extends _TooltipComponentBase>
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) {
Expand Down Expand Up @@ -470,6 +453,17 @@ export abstract class _MatTooltipBase<T extends _TooltipComponentBase>
.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;
}

Expand Down

0 comments on commit e01e579

Please sign in to comment.