Skip to content

Commit

Permalink
fix(material/select): disabled state out of sync when swapping form g…
Browse files Browse the repository at this point in the history
…roup with a disabled one (#17872)

Fixes the disabled state of a `mat-select` falling out of sync with its form control if the
control's group is swapped out with one that is disabled on init.

Fixes #17860.
  • Loading branch information
crisbeto authored Mar 3, 2022
1 parent 2111bbf commit 723a821
Show file tree
Hide file tree
Showing 3 changed files with 112 additions and 1 deletion.
45 changes: 45 additions & 0 deletions src/material-experimental/mdc-select/select.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import {
} from '@angular/core/testing';
import {
ControlValueAccessor,
FormBuilder,
FormControl,
FormGroup,
FormGroupDirective,
Expand Down Expand Up @@ -123,6 +124,7 @@ describe('MDC-based MatSelect', () => {
SelectWithGroupsAndNgContainer,
SelectWithFormFieldLabel,
SelectWithChangeEvent,
SelectInsideDynamicFormGroup,
]);
}),
);
Expand Down Expand Up @@ -2193,6 +2195,23 @@ describe('MDC-based MatSelect', () => {
.withContext(`Expected select panelOpen property to become true.`)
.toBe(true);
}));

it(
'should keep the disabled state in sync if the form group is swapped and ' +
'disabled at the same time',
fakeAsync(() => {
const fixture = TestBed.createComponent(SelectInsideDynamicFormGroup);
fixture.detectChanges();
const instance = fixture.componentInstance;

expect(instance.select.disabled).toBe(false);

instance.assignGroup(true);
fixture.detectChanges();

expect(instance.select.disabled).toBe(true);
}),
);
});

describe('keyboard scrolling', () => {
Expand Down Expand Up @@ -5036,3 +5055,29 @@ class SelectWithResetOptionAndFormControl {
`,
})
class SelectInNgContainer {}

@Component({
template: `
<form [formGroup]="form">
<mat-form-field>
<mat-select formControlName="control">
<mat-option value="1">One</mat-option>
</mat-select>
</mat-form-field>
</form>
`,
})
class SelectInsideDynamicFormGroup {
@ViewChild(MatSelect) select: MatSelect;
form: FormGroup;

constructor(private _formBuilder: FormBuilder) {
this.assignGroup(false);
}

assignGroup(isDisabled: boolean) {
this.form = this._formBuilder.group({
control: {value: '', disabled: isDisabled},
});
}
}
45 changes: 45 additions & 0 deletions src/material/select/select.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ import {
NG_VALUE_ACCESSOR,
ReactiveFormsModule,
Validators,
FormBuilder,
} from '@angular/forms';
import {ErrorStateMatcher, MatOption, MatOptionSelectionChange} from '@angular/material/core';
import {
Expand Down Expand Up @@ -122,6 +123,7 @@ describe('MatSelect', () => {
SelectWithGroupsAndNgContainer,
SelectWithFormFieldLabel,
SelectWithChangeEvent,
SelectInsideDynamicFormGroup,
]);
}),
);
Expand Down Expand Up @@ -2208,6 +2210,23 @@ describe('MatSelect', () => {
.withContext(`Expected select panelOpen property to become true.`)
.toBe(true);
}));

it(
'should keep the disabled state in sync if the form group is swapped and ' +
'disabled at the same time',
fakeAsync(() => {
const fixture = TestBed.createComponent(SelectInsideDynamicFormGroup);
fixture.detectChanges();
const instance = fixture.componentInstance;

expect(instance.select.disabled).toBe(false);

instance.assignGroup(true);
fixture.detectChanges();

expect(instance.select.disabled).toBe(true);
}),
);
});

describe('animations', () => {
Expand Down Expand Up @@ -6017,3 +6036,29 @@ class SelectWithResetOptionAndFormControl {
`,
})
class SelectInNgContainer {}

@Component({
template: `
<form [formGroup]="form">
<mat-form-field>
<mat-select formControlName="control">
<mat-option value="1">One</mat-option>
</mat-select>
</mat-form-field>
</form>
`,
})
class SelectInsideDynamicFormGroup {
@ViewChild(MatSelect) select: MatSelect;
form: FormGroup;

constructor(private _formBuilder: FormBuilder) {
this.assignGroup(false);
}

assignGroup(isDisabled: boolean) {
this.form = this._formBuilder.group({
control: {value: '', disabled: isDisabled},
});
}
}
23 changes: 22 additions & 1 deletion src/material/select/select.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ import {
ViewEncapsulation,
} from '@angular/core';
import {
AbstractControl,
ControlValueAccessor,
FormGroupDirective,
NgControl,
Expand Down Expand Up @@ -293,6 +294,12 @@ export abstract class _MatSelectBase<C>
/** Current `ariar-labelledby` value for the select trigger. */
private _triggerAriaLabelledBy: string | null = null;

/**
* Keeps track of the previous form control assigned to the select.
* Used to detect if it has changed.
*/
private _previousControl: AbstractControl | null | undefined;

/** Emits whenever the component is destroyed. */
protected readonly _destroy = new Subject<void>();

Expand Down Expand Up @@ -571,6 +578,7 @@ export abstract class _MatSelectBase<C>

ngDoCheck() {
const newAriaLabelledby = this._getTriggerAriaLabelledby();
const ngControl = this.ngControl;

// We have to manage setting the `aria-labelledby` ourselves, because part of its value
// is computed as a result of a content query which can cause this binding to trigger a
Expand All @@ -585,7 +593,20 @@ export abstract class _MatSelectBase<C>
}
}

if (this.ngControl) {
if (ngControl) {
// The disabled state might go out of sync if the form group is swapped out. See #17860.
if (this._previousControl !== ngControl.control) {
if (
this._previousControl !== undefined &&
ngControl.disabled !== null &&
ngControl.disabled !== this.disabled
) {
this.disabled = ngControl.disabled;
}

this._previousControl = ngControl.control;
}

this.updateErrorState();
}
}
Expand Down

0 comments on commit 723a821

Please sign in to comment.