Skip to content

Commit ce1b517

Browse files
stevenyxujelbourn
authored andcommitted
fix(checkbox): update MatCheckbox disabled setter to trigger change detection (#11098)
This is related to #11056, which is a similar fix for MatRadioButton. The MatCheckbox component instance may be accessed directly by a parent component using ViewChild/ContentChild. Since MatCheckbox uses OnPush change detection setting the disabled property directly currently doesn't cause change detection leading to a stale or broken state. Accessing it through a template binding does correctly trigger change detection on MatCheckbox (there are already test cases for this method). We need to add a manual call to markForCheck, or else MatCheckbox will never get kicked if disabled is set directly on the component instance. Related open Angular bug: angular/angular#20611
1 parent 33b5df4 commit ce1b517

File tree

2 files changed

+86
-4
lines changed

2 files changed

+86
-4
lines changed

src/lib/checkbox/checkbox.spec.ts

Lines changed: 71 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import {ComponentFixture, fakeAsync, TestBed, tick, flush} from '@angular/core/testing';
22
import {FormControl, FormsModule, NgModel, ReactiveFormsModule} from '@angular/forms';
3-
import {Component, DebugElement} from '@angular/core';
3+
import {Component, DebugElement, ViewChild} from '@angular/core';
44
import {By} from '@angular/platform-browser';
55
import {dispatchFakeEvent} from '@angular/cdk/testing';
66
import {MatCheckbox, MatCheckboxChange, MatCheckboxModule} from './index';
@@ -28,6 +28,7 @@ describe('MatCheckbox', () => {
2828
CheckboxWithFormControl,
2929
CheckboxWithoutLabel,
3030
CheckboxWithTabindexAttr,
31+
CheckboxUsingViewChild,
3132
]
3233
});
3334

@@ -786,7 +787,6 @@ describe('MatCheckbox', () => {
786787
});
787788

788789
describe('with native tabindex attribute', () => {
789-
790790
it('should properly detect native tabindex attribute', fakeAsync(() => {
791791
fixture = TestBed.createComponent(CheckboxWithTabindexAttr);
792792
fixture.detectChanges();
@@ -799,6 +799,61 @@ describe('MatCheckbox', () => {
799799
}));
800800
});
801801

802+
describe('using ViewChild', () => {
803+
let checkboxDebugElement: DebugElement;
804+
let checkboxNativeElement: HTMLElement;
805+
let testComponent: CheckboxUsingViewChild;
806+
807+
beforeEach(() => {
808+
fixture = TestBed.createComponent(CheckboxUsingViewChild);
809+
fixture.detectChanges();
810+
811+
checkboxDebugElement = fixture.debugElement.query(By.directive(MatCheckbox));
812+
checkboxNativeElement = checkboxDebugElement.nativeElement;
813+
testComponent = fixture.debugElement.componentInstance;
814+
});
815+
816+
it('should toggle checkbox disabledness correctly', () => {
817+
const checkboxInstance = checkboxDebugElement.componentInstance;
818+
const inputElement = <HTMLInputElement>checkboxNativeElement.querySelector('input');
819+
expect(checkboxInstance.disabled).toBe(false);
820+
expect(checkboxNativeElement.classList).not.toContain('mat-checkbox-disabled');
821+
expect(inputElement.tabIndex).toBe(0);
822+
expect(inputElement.disabled).toBe(false);
823+
824+
testComponent.isDisabled = true;
825+
fixture.detectChanges();
826+
827+
expect(checkboxInstance.disabled).toBe(true);
828+
expect(checkboxNativeElement.classList).toContain('mat-checkbox-disabled');
829+
expect(inputElement.disabled).toBe(true);
830+
831+
testComponent.isDisabled = false;
832+
fixture.detectChanges();
833+
834+
expect(checkboxInstance.disabled).toBe(false);
835+
expect(checkboxNativeElement.classList).not.toContain('mat-checkbox-disabled');
836+
expect(inputElement.tabIndex).toBe(0);
837+
expect(inputElement.disabled).toBe(false);
838+
});
839+
840+
it('should toggle checkbox ripple disabledness correctly', () => {
841+
const labelElement = checkboxNativeElement.querySelector('label') as HTMLLabelElement;
842+
843+
testComponent.isDisabled = true;
844+
fixture.detectChanges();
845+
dispatchFakeEvent(labelElement, 'mousedown');
846+
dispatchFakeEvent(labelElement, 'mouseup');
847+
expect(checkboxNativeElement.querySelectorAll('.mat-ripple-element').length).toBe(0);
848+
849+
testComponent.isDisabled = false;
850+
fixture.detectChanges();
851+
dispatchFakeEvent(labelElement, 'mousedown');
852+
dispatchFakeEvent(labelElement, 'mouseup');
853+
expect(checkboxNativeElement.querySelectorAll('.mat-ripple-element').length).toBe(1);
854+
});
855+
});
856+
802857
describe('with multiple checkboxes', () => {
803858
beforeEach(() => {
804859
fixture = TestBed.createComponent(MultipleCheckboxes);
@@ -1116,6 +1171,20 @@ class CheckboxWithTabIndex {
11161171
isDisabled: boolean = false;
11171172
}
11181173

1174+
1175+
/** Simple test component that accesses MatCheckbox using ViewChild. */
1176+
@Component({
1177+
template: `
1178+
<mat-checkbox></mat-checkbox>`,
1179+
})
1180+
class CheckboxUsingViewChild {
1181+
@ViewChild(MatCheckbox) checkbox;
1182+
1183+
set isDisabled(value: boolean) {
1184+
this.checkbox.disabled = value;
1185+
}
1186+
}
1187+
11191188
/** Simple test component with an aria-label set. */
11201189
@Component({
11211190
template: `<mat-checkbox aria-label="Super effective"></mat-checkbox>`

src/lib/checkbox/checkbox.ts

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ export const _MatCheckboxMixinBase =
110110
'[class.mat-checkbox-label-before]': 'labelPosition == "before"',
111111
},
112112
providers: [MAT_CHECKBOX_CONTROL_VALUE_ACCESSOR],
113-
inputs: ['disabled', 'disableRipple', 'color', 'tabIndex'],
113+
inputs: ['disableRipple', 'color', 'tabIndex'],
114114
encapsulation: ViewEncapsulation.None,
115115
changeDetection: ChangeDetectionStrategy.OnPush
116116
})
@@ -213,6 +213,20 @@ export class MatCheckbox extends _MatCheckboxMixinBase implements ControlValueAc
213213
}
214214
private _checked: boolean = false;
215215

216+
/**
217+
* Whether the checkbox is disabled. This fully overrides the implementation provided by
218+
* mixinDisabled, but the mixin is still required because mixinTabIndex requires it.
219+
*/
220+
@Input()
221+
get disabled() { return this._disabled; }
222+
set disabled(value: any) {
223+
if (value != this.disabled) {
224+
this._disabled = value;
225+
this._changeDetectorRef.markForCheck();
226+
}
227+
}
228+
private _disabled: boolean = false;
229+
216230
/**
217231
* Whether the checkbox is indeterminate. This is also known as "mixed" mode and can be used to
218232
* represent a checkbox with three states, e.g. a checkbox that represents a nested list of
@@ -267,7 +281,6 @@ export class MatCheckbox extends _MatCheckboxMixinBase implements ControlValueAc
267281
// Implemented as part of ControlValueAccessor.
268282
setDisabledState(isDisabled: boolean) {
269283
this.disabled = isDisabled;
270-
this._changeDetectorRef.markForCheck();
271284
}
272285

273286
_getAriaChecked(): 'true' | 'false' | 'mixed' {

0 commit comments

Comments
 (0)