Skip to content

Commit 1bcf8c0

Browse files
committed
refactor(material/button): clean up antipattern (#27157)
The button has a host binding that doesn't actually set the attribute it is bound to, but rather it is used to update the state of the ripple. This is an antipattern since data bindings aren't supposed to have side effects. (cherry picked from commit 28e0ace)
1 parent f5f1d51 commit 1bcf8c0

File tree

2 files changed

+9
-11
lines changed

2 files changed

+9
-11
lines changed

src/material/button/button-base.ts

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
ElementRef,
1515
inject,
1616
NgZone,
17+
OnChanges,
1718
OnDestroy,
1819
OnInit,
1920
} from '@angular/core';
@@ -43,8 +44,6 @@ export const MAT_BUTTON_HOST = {
4344
// wants to target all Material buttons.
4445
'[class.mat-mdc-button-base]': 'true',
4546
[MAT_BUTTON_RIPPLE_UNINITIALIZED]: '',
46-
'[attr.mat-button-disabled]': '_isRippleDisabled()',
47-
'[attr.mat-button-is-fab]': '_isFab',
4847
};
4948

5049
/** List of classes to add to buttons instances based on host attribute selector. */
@@ -95,7 +94,7 @@ export const _MatButtonMixin = mixinColor(
9594
@Directive()
9695
export class MatButtonBase
9796
extends _MatButtonMixin
98-
implements CanDisable, CanColor, CanDisableRipple, AfterViewInit, OnDestroy
97+
implements CanDisable, CanColor, CanDisableRipple, AfterViewInit, OnChanges, OnDestroy
9998
{
10099
private readonly _focusMonitor = inject(FocusMonitor);
101100

@@ -151,6 +150,12 @@ export class MatButtonBase
151150
this._focusMonitor.monitor(this._elementRef, true);
152151
}
153152

153+
ngOnChanges() {
154+
if (this._ripple) {
155+
this._ripple.disabled = this.disableRipple || this.disabled;
156+
}
157+
}
158+
154159
ngOnDestroy() {
155160
this._focusMonitor.stopMonitoring(this._elementRef);
156161
}
@@ -168,12 +173,6 @@ export class MatButtonBase
168173
private _hasHostAttributes(...attributes: string[]) {
169174
return attributes.some(attribute => this._elementRef.nativeElement.hasAttribute(attribute));
170175
}
171-
172-
_isRippleDisabled() {
173-
if (this._ripple) {
174-
this._ripple.disabled = this.disableRipple || this.disabled;
175-
}
176-
}
177176
}
178177

179178
/** Shared inputs by buttons using the `<a>` tag */
@@ -197,8 +196,6 @@ export const MAT_ANCHOR_HOST = {
197196
// wants to target all Material buttons.
198197
'[class.mat-mdc-button-base]': 'true',
199198
[MAT_BUTTON_RIPPLE_UNINITIALIZED]: '',
200-
'[attr.mat-button-disabled]': '_isRippleDisabled()',
201-
'[attr.mat-button-is-fab]': '_isFab',
202199
};
203200

204201
/**

tools/public_api_guard/material/button.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import * as i4 from '@angular/material/core';
1818
import { InjectionToken } from '@angular/core';
1919
import { MatRipple } from '@angular/material/core';
2020
import { NgZone } from '@angular/core';
21+
import { OnChanges } from '@angular/core';
2122
import { OnDestroy } from '@angular/core';
2223
import { OnInit } from '@angular/core';
2324
import { Platform } from '@angular/cdk/platform';

0 commit comments

Comments
 (0)