From 568c3ca90406f1150d2e971fb419d6b038980d62 Mon Sep 17 00:00:00 2001 From: arturovt Date: Sat, 20 Nov 2021 04:25:25 +0200 Subject: [PATCH] perf(material/button): do not run change detection when the anchor is clicked --- .../mdc-button/BUILD.bazel | 1 + .../mdc-button/button-base.ts | 24 ++++++++++------ .../mdc-button/button.spec.ts | 25 ++++++++++++++++- src/material/button/BUILD.bazel | 1 + src/material/button/button.spec.ts | 25 ++++++++++++++++- src/material/button/button.ts | 28 ++++++++++++++++--- tools/public_api_guard/material/button.md | 14 +++++++--- 7 files changed, 99 insertions(+), 19 deletions(-) diff --git a/src/material-experimental/mdc-button/BUILD.bazel b/src/material-experimental/mdc-button/BUILD.bazel index aca24ef76421..48fef69e3ab3 100644 --- a/src/material-experimental/mdc-button/BUILD.bazel +++ b/src/material-experimental/mdc-button/BUILD.bazel @@ -102,6 +102,7 @@ ng_test_library( deps = [ ":mdc-button", "//src/cdk/platform", + "//src/cdk/testing/private", "//src/material-experimental/mdc-core", "//src/material/button", "@npm//@angular/platform-browser", diff --git a/src/material-experimental/mdc-button/button-base.ts b/src/material-experimental/mdc-button/button-base.ts index 82df10547557..58c82feed76e 100644 --- a/src/material-experimental/mdc-button/button-base.ts +++ b/src/material-experimental/mdc-button/button-base.ts @@ -8,7 +8,7 @@ import {BooleanInput} from '@angular/cdk/coercion'; import {Platform} from '@angular/cdk/platform'; -import {Directive, ElementRef, NgZone, ViewChild} from '@angular/core'; +import {Directive, ElementRef, NgZone, OnDestroy, OnInit, ViewChild} from '@angular/core'; import { CanColor, CanDisable, @@ -159,23 +159,29 @@ export const MAT_ANCHOR_HOST = { /** * Anchor button base. */ -@Directive({ - host: { - '(click)': '_haltDisabledEvents($event)', - }, -}) -export class MatAnchorBase extends MatButtonBase { +@Directive() +export class MatAnchorBase extends MatButtonBase implements OnInit, OnDestroy { tabIndex: number; constructor(elementRef: ElementRef, platform: Platform, ngZone: NgZone, animationMode?: string) { super(elementRef, platform, ngZone, animationMode); } - _haltDisabledEvents(event: Event) { + ngOnInit(): void { + this._ngZone.runOutsideAngular(() => { + this._elementRef.nativeElement.addEventListener('click', this._haltDisabledEvents); + }); + } + + ngOnDestroy(): void { + this._elementRef.nativeElement.removeEventListener('click', this._haltDisabledEvents); + } + + _haltDisabledEvents = (event: Event): void => { // A disabled button shouldn't apply any actions if (this.disabled) { event.preventDefault(); event.stopImmediatePropagation(); } - } + }; } diff --git a/src/material-experimental/mdc-button/button.spec.ts b/src/material-experimental/mdc-button/button.spec.ts index 4eccefdff4d6..b19b59c8b9da 100644 --- a/src/material-experimental/mdc-button/button.spec.ts +++ b/src/material-experimental/mdc-button/button.spec.ts @@ -1,8 +1,9 @@ import {waitForAsync, ComponentFixture, TestBed} from '@angular/core/testing'; -import {Component, DebugElement} from '@angular/core'; +import {ApplicationRef, Component, DebugElement} from '@angular/core'; import {By} from '@angular/platform-browser'; import {MatButtonModule, MatButton, MatFabDefaultOptions, MAT_FAB_DEFAULT_OPTIONS} from './index'; import {MatRipple, ThemePalette} from '@angular/material-experimental/mdc-core'; +import {createMouseEvent, dispatchEvent} from '@angular/cdk/testing/private'; describe('MDC-based MatButton', () => { beforeEach( @@ -229,6 +230,28 @@ describe('MDC-based MatButton', () => { .withContext('Expected custom tabindex to be overwritten when disabled.') .toBe('-1'); }); + + describe('change detection behavior', () => { + it('should not run change detection for disabled anchor but should prevent the default behavior and stop event propagation', () => { + const appRef = TestBed.inject(ApplicationRef); + const fixture = TestBed.createComponent(TestApp); + fixture.componentInstance.isDisabled = true; + fixture.detectChanges(); + const anchorElement = fixture.debugElement.query(By.css('a'))!.nativeElement; + + spyOn(appRef, 'tick'); + + const event = createMouseEvent('click'); + spyOn(event, 'preventDefault').and.callThrough(); + spyOn(event, 'stopImmediatePropagation').and.callThrough(); + + dispatchEvent(anchorElement, event); + + expect(appRef.tick).not.toHaveBeenCalled(); + expect(event.preventDefault).toHaveBeenCalled(); + expect(event.stopImmediatePropagation).toHaveBeenCalled(); + }); + }); }); // Ripple tests. diff --git a/src/material/button/BUILD.bazel b/src/material/button/BUILD.bazel index 1302f96a0374..f411fdcf24de 100644 --- a/src/material/button/BUILD.bazel +++ b/src/material/button/BUILD.bazel @@ -52,6 +52,7 @@ ng_test_library( ), deps = [ ":button", + "//src/cdk/testing/private", "//src/material/core", "@npm//@angular/platform-browser", ], diff --git a/src/material/button/button.spec.ts b/src/material/button/button.spec.ts index a0eced1ece92..e4c5bcd179a9 100644 --- a/src/material/button/button.spec.ts +++ b/src/material/button/button.spec.ts @@ -1,8 +1,9 @@ import {waitForAsync, ComponentFixture, TestBed} from '@angular/core/testing'; -import {Component, DebugElement} from '@angular/core'; +import {ApplicationRef, Component, DebugElement} from '@angular/core'; import {By} from '@angular/platform-browser'; import {MatButtonModule, MatButton} from './index'; import {MatRipple, ThemePalette} from '@angular/material/core'; +import {createMouseEvent, dispatchEvent} from '@angular/cdk/testing/private'; describe('MatButton', () => { beforeEach( @@ -243,6 +244,28 @@ describe('MatButton', () => { .withContext('Expected custom tabindex to be overwritten when disabled.') .toBe('-1'); }); + + describe('change detection behavior', () => { + it('should not run change detection for disabled anchor but should prevent the default behavior and stop event propagation', () => { + const appRef = TestBed.inject(ApplicationRef); + const fixture = TestBed.createComponent(TestApp); + fixture.componentInstance.isDisabled = true; + fixture.detectChanges(); + const anchorElement = fixture.debugElement.query(By.css('a'))!.nativeElement; + + spyOn(appRef, 'tick'); + + const event = createMouseEvent('click'); + spyOn(event, 'preventDefault').and.callThrough(); + spyOn(event, 'stopImmediatePropagation').and.callThrough(); + + dispatchEvent(anchorElement, event); + + expect(appRef.tick).not.toHaveBeenCalled(); + expect(event.preventDefault).toHaveBeenCalled(); + expect(event.stopImmediatePropagation).toHaveBeenCalled(); + }); + }); }); // Ripple tests. diff --git a/src/material/button/button.ts b/src/material/button/button.ts index cef104c30d04..e3e4945b794a 100644 --- a/src/material/button/button.ts +++ b/src/material/button/button.ts @@ -19,6 +19,7 @@ import { Inject, Input, AfterViewInit, + NgZone, } from '@angular/core'; import { CanColor, @@ -168,7 +169,6 @@ export class MatButton '[attr.tabindex]': 'disabled ? -1 : (tabIndex || 0)', '[attr.disabled]': 'disabled || null', '[attr.aria-disabled]': 'disabled.toString()', - '(click)': '_haltDisabledEvents($event)', '[class._mat-animation-noopable]': '_animationMode === "NoopAnimations"', '[class.mat-button-disabled]': 'disabled', 'class': 'mat-focus-indicator', @@ -179,7 +179,7 @@ export class MatButton encapsulation: ViewEncapsulation.None, changeDetection: ChangeDetectionStrategy.OnPush, }) -export class MatAnchor extends MatButton { +export class MatAnchor extends MatButton implements AfterViewInit, OnDestroy { /** Tabindex of the button. */ @Input() tabIndex: number; @@ -187,15 +187,35 @@ export class MatAnchor extends MatButton { focusMonitor: FocusMonitor, elementRef: ElementRef, @Optional() @Inject(ANIMATION_MODULE_TYPE) animationMode: string, + /** @breaking-change 14.0.0 _ngZone will be required. */ + @Optional() private _ngZone?: NgZone, ) { super(elementRef, focusMonitor, animationMode); } - _haltDisabledEvents(event: Event) { + override ngAfterViewInit(): void { + super.ngAfterViewInit(); + + /** @breaking-change 14.0.0 _ngZone will be required. */ + if (this._ngZone) { + this._ngZone.runOutsideAngular(() => { + this._elementRef.nativeElement.addEventListener('click', this._haltDisabledEvents); + }); + } else { + this._elementRef.nativeElement.addEventListener('click', this._haltDisabledEvents); + } + } + + override ngOnDestroy(): void { + super.ngOnDestroy(); + this._elementRef.nativeElement.removeEventListener('click', this._haltDisabledEvents); + } + + _haltDisabledEvents = (event: Event): void => { // A disabled button shouldn't apply any actions if (this.disabled) { event.preventDefault(); event.stopImmediatePropagation(); } - } + }; } diff --git a/tools/public_api_guard/material/button.md b/tools/public_api_guard/material/button.md index dc1c58c50c36..26a4b01d6951 100644 --- a/tools/public_api_guard/material/button.md +++ b/tools/public_api_guard/material/button.md @@ -18,18 +18,24 @@ import { FocusOrigin } from '@angular/cdk/a11y'; import * as i0 from '@angular/core'; import * as i2 from '@angular/material/core'; import { MatRipple } from '@angular/material/core'; +import { NgZone } from '@angular/core'; import { OnDestroy } from '@angular/core'; // @public -export class MatAnchor extends MatButton { - constructor(focusMonitor: FocusMonitor, elementRef: ElementRef, animationMode: string); +export class MatAnchor extends MatButton implements AfterViewInit, OnDestroy { + constructor(focusMonitor: FocusMonitor, elementRef: ElementRef, animationMode: string, + _ngZone?: NgZone | undefined); // (undocumented) - _haltDisabledEvents(event: Event): void; + _haltDisabledEvents: (event: Event) => void; + // (undocumented) + ngAfterViewInit(): void; + // (undocumented) + ngOnDestroy(): void; tabIndex: number; // (undocumented) static ɵcmp: i0.ɵɵComponentDeclaration; // (undocumented) - static ɵfac: i0.ɵɵFactoryDeclaration; + static ɵfac: i0.ɵɵFactoryDeclaration; } // @public