Skip to content

Commit cfbbbfe

Browse files
authored
fix(material/select): value not updated if the same array is updated and re-assigned (#21625)
`mat-select` has an internal check that doesn't re-assign a value if the reference is the same. This was meant primarily for primitives, but it ends up breaking the behavior for a multi-select where the value is always an array. These changes skip the check for a multi-select receiving a new array value. Fixes #21583.
1 parent aa1eb49 commit cfbbbfe

File tree

3 files changed

+48
-1
lines changed

3 files changed

+48
-1
lines changed

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

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3768,6 +3768,30 @@ describe('MDC-based MatSelect', () => {
37683768
.toEqual([true, true, true, true, false, false]);
37693769
}));
37703770

3771+
it('should update the option selected state if the same array is mutated and passed back in',
3772+
fakeAsync(() => {
3773+
const value: string[] = [];
3774+
trigger.click();
3775+
testInstance.control.setValue(value);
3776+
fixture.detectChanges();
3777+
3778+
const optionNodes =
3779+
Array.from<HTMLElement>(overlayContainerElement.querySelectorAll('mat-option'));
3780+
const optionInstances = testInstance.options.toArray();
3781+
3782+
expect(optionNodes.some(option => {
3783+
return option.classList.contains('mdc-list-item--selected');
3784+
})).toBe(false);
3785+
expect(optionInstances.some(option => option.selected)).toBe(false);
3786+
3787+
value.push('eggs-5');
3788+
testInstance.control.setValue(value);
3789+
fixture.detectChanges();
3790+
3791+
expect(optionNodes[5].classList).toContain('mdc-list-item--selected');
3792+
expect(optionInstances[5].selected).toBe(true);
3793+
}));
3794+
37713795
});
37723796

37733797
it('should be able to provide default values through an injection token', fakeAsync(() => {

src/material/select/select.spec.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4666,6 +4666,28 @@ describe('MatSelect', () => {
46664666
.toEqual([true, true, true, true, false, false]);
46674667
}));
46684668

4669+
it('should update the option selected state if the same array is mutated and passed back in',
4670+
fakeAsync(() => {
4671+
const value: string[] = [];
4672+
trigger.click();
4673+
testInstance.control.setValue(value);
4674+
fixture.detectChanges();
4675+
4676+
const optionNodes =
4677+
Array.from<HTMLElement>(overlayContainerElement.querySelectorAll('mat-option'));
4678+
const optionInstances = testInstance.options.toArray();
4679+
4680+
expect(optionNodes.some(option => option.classList.contains('mat-selected'))).toBe(false);
4681+
expect(optionInstances.some(option => option.selected)).toBe(false);
4682+
4683+
value.push('eggs-5');
4684+
testInstance.control.setValue(value);
4685+
fixture.detectChanges();
4686+
4687+
expect(optionNodes[5].classList).toContain('mat-selected');
4688+
expect(optionInstances[5].selected).toBe(true);
4689+
}));
4690+
46694691
});
46704692

46714693
it('should be able to provide default values through an injection token', fakeAsync(() => {

src/material/select/select.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -402,7 +402,8 @@ export abstract class _MatSelectBase<C> extends _MatSelectMixinBase implements A
402402
@Input()
403403
get value(): any { return this._value; }
404404
set value(newValue: any) {
405-
if (newValue !== this._value) {
405+
// Always re-assign an array, because it might have been mutated.
406+
if (newValue !== this._value || (this._multiple && Array.isArray(newValue))) {
406407
if (this.options) {
407408
this._setSelectionByValue(newValue);
408409
}

0 commit comments

Comments
 (0)