Skip to content

Commit 1d4c09a

Browse files
committed
refactor(mdc-prototypes): do not mark components dirty in input setters
In order to make our component inputs which use boolean coercion more consistent with other inputs which aren't declared through setters, we should no longer call `markForCheck` within input setters as Angular automatically runs change detection if input values are updated. This breaks the programmatic usage of these inputs as Angular in that case won't be able to run change detection (since it can't detect that the value of the input changes).. though this is a more general problem with the `OnPush` strategy as we can't have consistent behavior for programmatic input updates without switching *every* input to a setter. Instead, in order to still support programmatic input updates, we should expose a method that allows developers to mark the MDC components as dirty. This is necessary so that developers which _really_ need to update programmatically (e.g. through `ViewChild`) can still update the componet to reflect the changes. The benefit is that we don't need to convert every input to a setter w/ markForCheck in order to make the OnPush inputs behavior consistent. Related to COMP-170
1 parent eef132b commit 1d4c09a

File tree

4 files changed

+30
-35
lines changed

4 files changed

+30
-35
lines changed

src/material-experimental/mdc-checkbox/checkbox.spec.ts

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -164,15 +164,6 @@ describe('MatCheckbox', () => {
164164
expect(testComponent.isIndeterminate).toBe(true);
165165
}));
166166

167-
it('should change native element checked when check programmatically', fakeAsync(() => {
168-
expect(inputElement.checked).toBe(false);
169-
170-
checkboxInstance.checked = true;
171-
fixture.detectChanges();
172-
173-
expect(inputElement.checked).toBe(true);
174-
}));
175-
176167
it('should toggle checked state on click', fakeAsync(() => {
177168
expect(checkboxInstance.checked).toBe(false);
178169

@@ -855,11 +846,9 @@ describe('MatCheckbox', () => {
855846
// fire and not result in a changed after checked exception. Related: #12323
856847
inputElement.focus();
857848

858-
// Flush the two nested timeouts from the FocusMonitor that are being created on `focus`.
859-
flush();
860-
861-
checkboxInstance.disabled = true;
849+
fixture.componentInstance.isDisabled = true;
862850
fixture.detectChanges();
851+
863852
flushMicrotasks();
864853
}).not.toThrow();
865854
}));
@@ -1005,11 +994,13 @@ class SingleCheckbox {
1005994

1006995
/** Simple component for testing an MatCheckbox with required ngModel. */
1007996
@Component({
1008-
template: `<mat-checkbox [required]="isRequired" [(ngModel)]="isGood">Be good</mat-checkbox>`,
997+
template: `<mat-checkbox [required]="isRequired" [(ngModel)]="isGood"
998+
[disabled]="isDisabled">Be good</mat-checkbox>`,
1009999
})
10101000
class CheckboxWithNgModel {
10111001
isGood: boolean = false;
10121002
isRequired: boolean = true;
1003+
isDisabled: boolean = false;
10131004
}
10141005

10151006
@Component({
@@ -1054,6 +1045,7 @@ class CheckboxUsingViewChild {
10541045

10551046
set isDisabled(value: boolean) {
10561047
this.checkbox.disabled = value;
1048+
this.checkbox.markForCheck();
10571049
}
10581050
}
10591051

src/material-experimental/mdc-checkbox/checkbox.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,6 @@ export class MatCheckbox implements AfterViewInit, OnDestroy, ControlValueAccess
120120
}
121121
set checked(checked) {
122122
this._checked = coerceBooleanProperty(checked);
123-
this._changeDetectorRef.markForCheck();
124123
}
125124
private _checked = false;
126125

@@ -136,7 +135,6 @@ export class MatCheckbox implements AfterViewInit, OnDestroy, ControlValueAccess
136135
}
137136
set indeterminate(indeterminate) {
138137
this._indeterminate = coerceBooleanProperty(indeterminate);
139-
this._changeDetectorRef.markForCheck();
140138
}
141139
private _indeterminate = false;
142140

@@ -147,7 +145,6 @@ export class MatCheckbox implements AfterViewInit, OnDestroy, ControlValueAccess
147145
}
148146
set disabled(disabled) {
149147
this._disabled = coerceBooleanProperty(disabled);
150-
this._changeDetectorRef.markForCheck();
151148
}
152149
private _disabled = false;
153150

@@ -158,7 +155,6 @@ export class MatCheckbox implements AfterViewInit, OnDestroy, ControlValueAccess
158155
}
159156
set required(required) {
160157
this._required = coerceBooleanProperty(required);
161-
this._changeDetectorRef.markForCheck();
162158
}
163159
private _required = false;
164160

@@ -284,6 +280,7 @@ export class MatCheckbox implements AfterViewInit, OnDestroy, ControlValueAccess
284280
*/
285281
setDisabledState(isDisabled: boolean) {
286282
this.disabled = isDisabled;
283+
this._changeDetectorRef.markForCheck();
287284
}
288285

289286
/**
@@ -305,6 +302,14 @@ export class MatCheckbox implements AfterViewInit, OnDestroy, ControlValueAccess
305302
this.checked = !this.checked;
306303
}
307304

305+
/**
306+
* Marks the component for check. This can be called after inputs have been updated
307+
* programmatically and the component should be updated to reflect the changes.
308+
*/
309+
markForCheck() {
310+
this._changeDetectorRef.markForCheck();
311+
}
312+
308313
/** Triggers the checkbox ripple. */
309314
_activateRipple() {
310315
if (!this.disabled && !this.disableRipple && this._animationMode != 'NoopAnimations') {

src/material-experimental/mdc-slide-toggle/slide-toggle.spec.ts

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,7 @@
11
import {BidiModule, Direction} from '@angular/cdk/bidi';
22
import {dispatchFakeEvent} from '@angular/cdk/testing';
33
import {Component} from '@angular/core';
4-
import {
5-
ComponentFixture,
6-
fakeAsync,
7-
flush,
8-
flushMicrotasks,
9-
TestBed,
10-
tick,
11-
} from '@angular/core/testing';
4+
import {ComponentFixture, fakeAsync, flushMicrotasks, TestBed, tick} from '@angular/core/testing';
125
import {FormControl, FormsModule, NgModel, ReactiveFormsModule} from '@angular/forms';
136
import {By} from '@angular/platform-browser';
147
import {MatSlideToggle, MatSlideToggleChange, MatSlideToggleModule} from './index';
@@ -481,11 +474,9 @@ describe('MatSlideToggle with forms', () => {
481474
// fire and not result in a changed after checked exception. Related: #12323
482475
inputElement.focus();
483476

484-
// Flush the two nested timeouts from the FocusMonitor that are being created on `focus`.
485-
flush();
486-
487-
slideToggle.disabled = true;
477+
fixture.componentInstance.isDisabled = true;
488478
fixture.detectChanges();
479+
489480
flushMicrotasks();
490481
}).not.toThrow();
491482
}));
@@ -497,6 +488,7 @@ describe('MatSlideToggle with forms', () => {
497488
expect(slideToggleModel.touched).toBe(false);
498489

499490
slideToggle.checked = true;
491+
slideToggle.markForCheck();
500492
fixture.detectChanges();
501493

502494
expect(slideToggleModel.touched).toBe(false);
@@ -736,10 +728,11 @@ class SlideToggleWithForm {
736728
}
737729

738730
@Component({
739-
template: `<mat-slide-toggle [(ngModel)]="modelValue"></mat-slide-toggle>`
731+
template: `<mat-slide-toggle [(ngModel)]="modelValue" [disabled]="isDisabled"></mat-slide-toggle>`
740732
})
741733
class SlideToggleWithModel {
742734
modelValue = false;
735+
isDisabled = false;
743736
}
744737

745738
@Component({

src/material-experimental/mdc-slide-toggle/slide-toggle.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -154,8 +154,6 @@ export class MatSlideToggle implements ControlValueAccessor, AfterViewInit, OnDe
154154
if (this._foundation) {
155155
this._foundation.setChecked(this._checked);
156156
}
157-
158-
this._changeDetectorRef.markForCheck();
159157
}
160158

161159
/** Whether to disable the ripple on this checkbox. */
@@ -179,8 +177,6 @@ export class MatSlideToggle implements ControlValueAccessor, AfterViewInit, OnDe
179177
if (this._foundation) {
180178
this._foundation.setDisabled(this._disabled);
181179
}
182-
183-
this._changeDetectorRef.markForCheck();
184180
}
185181
private _disabled = false;
186182

@@ -268,6 +264,7 @@ export class MatSlideToggle implements ControlValueAccessor, AfterViewInit, OnDe
268264
/** Implemented as part of ControlValueAccessor. */
269265
writeValue(value: any): void {
270266
this.checked = !!value;
267+
this._changeDetectorRef.markForCheck();
271268
}
272269

273270
/** Implemented as part of ControlValueAccessor. */
@@ -297,6 +294,14 @@ export class MatSlideToggle implements ControlValueAccessor, AfterViewInit, OnDe
297294
this._onChange(this.checked);
298295
}
299296

297+
/**
298+
* Marks the component for check. This can be called after inputs have been updated
299+
* programmatically and the component should be updated to reflect the changes.
300+
*/
301+
markForCheck() {
302+
this._changeDetectorRef.markForCheck();
303+
}
304+
300305
/** Handles blur events on the native input. */
301306
_onBlur() {
302307
// When a focused element becomes disabled, the browser *immediately* fires a blur event.

0 commit comments

Comments
 (0)