Skip to content

Commit

Permalink
perf(material/button): do not run change detection when the anchor is…
Browse files Browse the repository at this point in the history
… clicked (#23992)

(cherry picked from commit 41f0244)
  • Loading branch information
arturovt authored and andrewseguin committed Jan 13, 2022
1 parent eae436f commit 4972dc5
Show file tree
Hide file tree
Showing 7 changed files with 99 additions and 19 deletions.
1 change: 1 addition & 0 deletions src/material-experimental/mdc-button/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
24 changes: 15 additions & 9 deletions src/material-experimental/mdc-button/button-base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
*/

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,
Expand Down Expand Up @@ -155,23 +155,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();
}
}
};
}
25 changes: 24 additions & 1 deletion src/material-experimental/mdc-button/button.spec.ts
Original file line number Diff line number Diff line change
@@ -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(
Expand Down Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions src/material/button/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ ng_test_library(
),
deps = [
":button",
"//src/cdk/testing/private",
"//src/material/core",
"@npm//@angular/platform-browser",
],
Expand Down
25 changes: 24 additions & 1 deletion src/material/button/button.spec.ts
Original file line number Diff line number Diff line change
@@ -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(
Expand Down Expand Up @@ -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.
Expand Down
28 changes: 24 additions & 4 deletions src/material/button/button.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
Inject,
Input,
AfterViewInit,
NgZone,
} from '@angular/core';
import {
CanColor,
Expand Down Expand Up @@ -164,7 +165,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',
Expand All @@ -175,23 +175,43 @@ 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;

constructor(
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();
}
}
};
}
14 changes: 10 additions & 4 deletions tools/public_api_guard/material/button.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,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<MatAnchor, "a[mat-button], a[mat-raised-button], a[mat-icon-button], a[mat-fab], a[mat-mini-fab], a[mat-stroked-button], a[mat-flat-button]", ["matButton", "matAnchor"], { "disabled": "disabled"; "disableRipple": "disableRipple"; "color": "color"; "tabIndex": "tabIndex"; }, {}, never, ["*"]>;
// (undocumented)
static ɵfac: i0.ɵɵFactoryDeclaration<MatAnchor, [null, null, { optional: true; }]>;
static ɵfac: i0.ɵɵFactoryDeclaration<MatAnchor, [null, null, { optional: true; }, { optional: true; }]>;
}

// @public
Expand Down

0 comments on commit 4972dc5

Please sign in to comment.