mirror of
https://github.com/angular/angular.git
synced 2026-09-14 13:54:52 +08:00
refactor(mdc-prototypes): do not mark components dirty in input setters (#16129)
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
This commit is contained in:
committed by
Jeremy Elbourn
parent
60c9ebc374
commit
19af6ec2a8
@@ -1,5 +1,5 @@
|
||||
import {dispatchFakeEvent} from '@angular/cdk/testing';
|
||||
import {ChangeDetectionStrategy, Component, DebugElement, Type, ViewChild} from '@angular/core';
|
||||
import {ChangeDetectionStrategy, Component, DebugElement, Type} from '@angular/core';
|
||||
import {
|
||||
ComponentFixture,
|
||||
fakeAsync,
|
||||
@@ -68,6 +68,28 @@ describe('MatCheckbox', () => {
|
||||
expect(inputElement.checked).toBe(false);
|
||||
}));
|
||||
|
||||
|
||||
it('should toggle checkbox ripple disabledness correctly', fakeAsync(() => {
|
||||
const rippleSelector = '.mat-ripple-element:not(.mat-checkbox-persistent-ripple)';
|
||||
|
||||
testComponent.isDisabled = true;
|
||||
fixture.detectChanges();
|
||||
dispatchFakeEvent(labelElement, 'mousedown');
|
||||
dispatchFakeEvent(labelElement, 'mouseup');
|
||||
labelElement.click();
|
||||
expect(checkboxNativeElement.querySelectorAll(rippleSelector).length).toBe(0);
|
||||
|
||||
flush();
|
||||
testComponent.isDisabled = false;
|
||||
fixture.detectChanges();
|
||||
dispatchFakeEvent(labelElement, 'mousedown');
|
||||
dispatchFakeEvent(labelElement, 'mouseup');
|
||||
labelElement.click();
|
||||
expect(checkboxNativeElement.querySelectorAll(rippleSelector).length).toBe(1);
|
||||
|
||||
flush();
|
||||
}));
|
||||
|
||||
it('should add and remove indeterminate state', fakeAsync(() => {
|
||||
expect(inputElement.checked).toBe(false);
|
||||
expect(inputElement.indeterminate).toBe(false);
|
||||
@@ -164,15 +186,6 @@ describe('MatCheckbox', () => {
|
||||
expect(testComponent.isIndeterminate).toBe(true);
|
||||
}));
|
||||
|
||||
it('should change native element checked when check programmatically', fakeAsync(() => {
|
||||
expect(inputElement.checked).toBe(false);
|
||||
|
||||
checkboxInstance.checked = true;
|
||||
fixture.detectChanges();
|
||||
|
||||
expect(inputElement.checked).toBe(true);
|
||||
}));
|
||||
|
||||
it('should toggle checked state on click', fakeAsync(() => {
|
||||
expect(checkboxInstance.checked).toBe(false);
|
||||
|
||||
@@ -698,64 +711,6 @@ describe('MatCheckbox', () => {
|
||||
}));
|
||||
});
|
||||
|
||||
describe('using ViewChild', () => {
|
||||
let checkboxDebugElement: DebugElement;
|
||||
let checkboxNativeElement: HTMLElement;
|
||||
let testComponent: CheckboxUsingViewChild;
|
||||
|
||||
beforeEach(() => {
|
||||
fixture = createComponent(CheckboxUsingViewChild);
|
||||
fixture.detectChanges();
|
||||
|
||||
checkboxDebugElement = fixture.debugElement.query(By.directive(MatCheckbox));
|
||||
checkboxNativeElement = checkboxDebugElement.nativeElement;
|
||||
testComponent = fixture.debugElement.componentInstance;
|
||||
});
|
||||
|
||||
it('should toggle checkbox disabledness correctly', fakeAsync(() => {
|
||||
const checkboxInstance = checkboxDebugElement.componentInstance;
|
||||
const inputElement = <HTMLInputElement>checkboxNativeElement.querySelector('input');
|
||||
expect(checkboxInstance.disabled).toBe(false);
|
||||
expect(inputElement.tabIndex).toBe(0);
|
||||
expect(inputElement.disabled).toBe(false);
|
||||
|
||||
testComponent.isDisabled = true;
|
||||
fixture.detectChanges();
|
||||
|
||||
expect(checkboxInstance.disabled).toBe(true);
|
||||
expect(inputElement.disabled).toBe(true);
|
||||
|
||||
testComponent.isDisabled = false;
|
||||
fixture.detectChanges();
|
||||
|
||||
expect(checkboxInstance.disabled).toBe(false);
|
||||
expect(inputElement.tabIndex).toBe(0);
|
||||
expect(inputElement.disabled).toBe(false);
|
||||
}));
|
||||
|
||||
it('should toggle checkbox ripple disabledness correctly', fakeAsync(() => {
|
||||
const rippleSelector = '.mat-ripple-element:not(.mat-checkbox-persistent-ripple)';
|
||||
const labelElement = checkboxNativeElement.querySelector('label') as HTMLLabelElement;
|
||||
|
||||
testComponent.isDisabled = true;
|
||||
fixture.detectChanges();
|
||||
dispatchFakeEvent(labelElement, 'mousedown');
|
||||
dispatchFakeEvent(labelElement, 'mouseup');
|
||||
labelElement.click();
|
||||
expect(checkboxNativeElement.querySelectorAll(rippleSelector).length).toBe(0);
|
||||
|
||||
flush();
|
||||
testComponent.isDisabled = false;
|
||||
fixture.detectChanges();
|
||||
dispatchFakeEvent(labelElement, 'mousedown');
|
||||
dispatchFakeEvent(labelElement, 'mouseup');
|
||||
labelElement.click();
|
||||
expect(checkboxNativeElement.querySelectorAll(rippleSelector).length).toBe(1);
|
||||
|
||||
flush();
|
||||
}));
|
||||
});
|
||||
|
||||
describe('with multiple checkboxes', () => {
|
||||
beforeEach(() => {
|
||||
fixture = createComponent(MultipleCheckboxes);
|
||||
@@ -855,11 +810,9 @@ describe('MatCheckbox', () => {
|
||||
// fire and not result in a changed after checked exception. Related: #12323
|
||||
inputElement.focus();
|
||||
|
||||
// Flush the two nested timeouts from the FocusMonitor that are being created on `focus`.
|
||||
flush();
|
||||
|
||||
checkboxInstance.disabled = true;
|
||||
fixture.componentInstance.isDisabled = true;
|
||||
fixture.detectChanges();
|
||||
|
||||
flushMicrotasks();
|
||||
}).not.toThrow();
|
||||
}));
|
||||
@@ -1005,11 +958,13 @@ class SingleCheckbox {
|
||||
|
||||
/** Simple component for testing an MatCheckbox with required ngModel. */
|
||||
@Component({
|
||||
template: `<mat-checkbox [required]="isRequired" [(ngModel)]="isGood">Be good</mat-checkbox>`,
|
||||
template: `<mat-checkbox [required]="isRequired" [(ngModel)]="isGood"
|
||||
[disabled]="isDisabled">Be good</mat-checkbox>`,
|
||||
})
|
||||
class CheckboxWithNgModel {
|
||||
isGood: boolean = false;
|
||||
isRequired: boolean = true;
|
||||
isDisabled: boolean = false;
|
||||
}
|
||||
|
||||
@Component({
|
||||
@@ -1043,20 +998,6 @@ class CheckboxWithTabIndex {
|
||||
isDisabled: boolean = false;
|
||||
}
|
||||
|
||||
|
||||
/** Simple test component that accesses MatCheckbox using ViewChild. */
|
||||
@Component({
|
||||
template: `
|
||||
<mat-checkbox></mat-checkbox>`,
|
||||
})
|
||||
class CheckboxUsingViewChild {
|
||||
@ViewChild(MatCheckbox, {static: false}) checkbox: MatCheckbox;
|
||||
|
||||
set isDisabled(value: boolean) {
|
||||
this.checkbox.disabled = value;
|
||||
}
|
||||
}
|
||||
|
||||
/** Simple test component with an aria-label set. */
|
||||
@Component({template: `<mat-checkbox aria-label="Super effective"></mat-checkbox>`})
|
||||
class CheckboxWithAriaLabel {
|
||||
|
||||
@@ -120,7 +120,6 @@ export class MatCheckbox implements AfterViewInit, OnDestroy, ControlValueAccess
|
||||
}
|
||||
set checked(checked) {
|
||||
this._checked = coerceBooleanProperty(checked);
|
||||
this._changeDetectorRef.markForCheck();
|
||||
}
|
||||
private _checked = false;
|
||||
|
||||
@@ -136,7 +135,6 @@ export class MatCheckbox implements AfterViewInit, OnDestroy, ControlValueAccess
|
||||
}
|
||||
set indeterminate(indeterminate) {
|
||||
this._indeterminate = coerceBooleanProperty(indeterminate);
|
||||
this._changeDetectorRef.markForCheck();
|
||||
}
|
||||
private _indeterminate = false;
|
||||
|
||||
@@ -147,7 +145,6 @@ export class MatCheckbox implements AfterViewInit, OnDestroy, ControlValueAccess
|
||||
}
|
||||
set disabled(disabled) {
|
||||
this._disabled = coerceBooleanProperty(disabled);
|
||||
this._changeDetectorRef.markForCheck();
|
||||
}
|
||||
private _disabled = false;
|
||||
|
||||
@@ -158,7 +155,6 @@ export class MatCheckbox implements AfterViewInit, OnDestroy, ControlValueAccess
|
||||
}
|
||||
set required(required) {
|
||||
this._required = coerceBooleanProperty(required);
|
||||
this._changeDetectorRef.markForCheck();
|
||||
}
|
||||
private _required = false;
|
||||
|
||||
@@ -284,6 +280,7 @@ export class MatCheckbox implements AfterViewInit, OnDestroy, ControlValueAccess
|
||||
*/
|
||||
setDisabledState(isDisabled: boolean) {
|
||||
this.disabled = isDisabled;
|
||||
this._changeDetectorRef.markForCheck();
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -1,14 +1,7 @@
|
||||
import {BidiModule, Direction} from '@angular/cdk/bidi';
|
||||
import {dispatchFakeEvent} from '@angular/cdk/testing';
|
||||
import {Component} from '@angular/core';
|
||||
import {
|
||||
ComponentFixture,
|
||||
fakeAsync,
|
||||
flush,
|
||||
flushMicrotasks,
|
||||
TestBed,
|
||||
tick,
|
||||
} from '@angular/core/testing';
|
||||
import {ComponentFixture, fakeAsync, flushMicrotasks, TestBed, tick} from '@angular/core/testing';
|
||||
import {FormControl, FormsModule, NgModel, ReactiveFormsModule} from '@angular/forms';
|
||||
import {By} from '@angular/platform-browser';
|
||||
import {MatSlideToggle, MatSlideToggleChange, MatSlideToggleModule} from './index';
|
||||
@@ -481,11 +474,9 @@ describe('MatSlideToggle with forms', () => {
|
||||
// fire and not result in a changed after checked exception. Related: #12323
|
||||
inputElement.focus();
|
||||
|
||||
// Flush the two nested timeouts from the FocusMonitor that are being created on `focus`.
|
||||
flush();
|
||||
|
||||
slideToggle.disabled = true;
|
||||
fixture.componentInstance.isDisabled = true;
|
||||
fixture.detectChanges();
|
||||
|
||||
flushMicrotasks();
|
||||
}).not.toThrow();
|
||||
}));
|
||||
@@ -496,7 +487,7 @@ describe('MatSlideToggle with forms', () => {
|
||||
// The control should start off with being untouched.
|
||||
expect(slideToggleModel.touched).toBe(false);
|
||||
|
||||
slideToggle.checked = true;
|
||||
testComponent.isChecked = true;
|
||||
fixture.detectChanges();
|
||||
|
||||
expect(slideToggleModel.touched).toBe(false);
|
||||
@@ -736,10 +727,13 @@ class SlideToggleWithForm {
|
||||
}
|
||||
|
||||
@Component({
|
||||
template: `<mat-slide-toggle [(ngModel)]="modelValue"></mat-slide-toggle>`
|
||||
template: `<mat-slide-toggle [(ngModel)]="modelValue" [disabled]="isDisabled"
|
||||
[checked]="isChecked"></mat-slide-toggle>`
|
||||
})
|
||||
class SlideToggleWithModel {
|
||||
modelValue = false;
|
||||
isChecked = false;
|
||||
isDisabled = false;
|
||||
}
|
||||
|
||||
@Component({
|
||||
|
||||
@@ -154,8 +154,6 @@ export class MatSlideToggle implements ControlValueAccessor, AfterViewInit, OnDe
|
||||
if (this._foundation) {
|
||||
this._foundation.setChecked(this._checked);
|
||||
}
|
||||
|
||||
this._changeDetectorRef.markForCheck();
|
||||
}
|
||||
|
||||
/** Whether to disable the ripple on this checkbox. */
|
||||
@@ -179,8 +177,6 @@ export class MatSlideToggle implements ControlValueAccessor, AfterViewInit, OnDe
|
||||
if (this._foundation) {
|
||||
this._foundation.setDisabled(this._disabled);
|
||||
}
|
||||
|
||||
this._changeDetectorRef.markForCheck();
|
||||
}
|
||||
private _disabled = false;
|
||||
|
||||
@@ -268,6 +264,7 @@ export class MatSlideToggle implements ControlValueAccessor, AfterViewInit, OnDe
|
||||
/** Implemented as part of ControlValueAccessor. */
|
||||
writeValue(value: any): void {
|
||||
this.checked = !!value;
|
||||
this._changeDetectorRef.markForCheck();
|
||||
}
|
||||
|
||||
/** Implemented as part of ControlValueAccessor. */
|
||||
|
||||
Reference in New Issue
Block a user