Skip to content

Commit

Permalink
perf(tooltip): avoid triggering change detection for all keydown even…
Browse files Browse the repository at this point in the history
…ts (#17331)

Currently we only care about keydown events on the Escape key, while the tooltip is open, however since we're binding the listener through `host` it'll change detection for all `keydown` events.
  • Loading branch information
crisbeto authored and jelbourn committed Oct 11, 2019
1 parent 35b3226 commit 493c32d
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 13 deletions.
31 changes: 19 additions & 12 deletions src/material/tooltip/tooltip.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,6 @@ export function MAT_TOOLTIP_DEFAULT_OPTIONS_FACTORY(): MatTooltipDefaultOptions
@Directive({
selector: '[matTooltip]',
exportAs: 'matTooltip',
host: {
'(keydown)': '_handleKeydown($event)',
},
})
export class MatTooltip implements OnDestroy, OnInit {
_overlayRef: OverlayRef | null;
Expand Down Expand Up @@ -285,6 +282,10 @@ export class MatTooltip implements OnDestroy, OnInit {
_ngZone.run(() => this.show());
}
});

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

/**
Expand All @@ -299,6 +300,8 @@ export class MatTooltip implements OnDestroy, OnInit {
* Dispose the tooltip when destroyed.
*/
ngOnDestroy() {
const nativeElement = this._elementRef.nativeElement;

clearTimeout(this._touchstartTimeout);

if (this._overlayRef) {
Expand All @@ -307,16 +310,17 @@ export class MatTooltip implements OnDestroy, OnInit {
}

// Clean up the event listeners set in the constructor
nativeElement.removeEventListener('keydown', this._handleKeydown);
this._passiveListeners.forEach((listener, event) => {
this._elementRef.nativeElement.removeEventListener(event, listener, passiveListenerOptions);
nativeElement.removeEventListener(event, listener, passiveListenerOptions);
});
this._passiveListeners.clear();

this._destroyed.next();
this._destroyed.complete();

this._ariaDescriber.removeDescription(this._elementRef.nativeElement, this.message);
this._focusMonitor.stopMonitoring(this._elementRef);
this._ariaDescriber.removeDescription(nativeElement, this.message);
this._focusMonitor.stopMonitoring(nativeElement);
}

/** Shows the tooltip after the delay in ms, defaults to tooltip-delay-show or 0ms if no input */
Expand Down Expand Up @@ -356,12 +360,15 @@ 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 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));
}
}

Expand Down
1 change: 0 additions & 1 deletion tools/public_api_guard/material/tooltip.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ export declare class MatTooltip implements OnDestroy, OnInit {
main: OverlayConnectionPosition;
fallback: OverlayConnectionPosition;
};
_handleKeydown(e: KeyboardEvent): void;
_isTooltipVisible(): boolean;
hide(delay?: number): void;
ngOnDestroy(): void;
Expand Down

0 comments on commit 493c32d

Please sign in to comment.