From 440055031834f583af9258816478ad7dea47621b Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Tue, 19 Jan 2021 17:49:18 +0100 Subject: [PATCH] fix(material/select): value not updated if the same array is updated and re-assigned `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. --- .../mdc-select/select.spec.ts | 24 +++++++++++++++++++ src/material/select/select.spec.ts | 22 +++++++++++++++++ src/material/select/select.ts | 3 ++- 3 files changed, 48 insertions(+), 1 deletion(-) diff --git a/src/material-experimental/mdc-select/select.spec.ts b/src/material-experimental/mdc-select/select.spec.ts index e999214c5624..208e77eb99d5 100644 --- a/src/material-experimental/mdc-select/select.spec.ts +++ b/src/material-experimental/mdc-select/select.spec.ts @@ -3768,6 +3768,30 @@ describe('MDC-based MatSelect', () => { .toEqual([true, true, true, true, false, false]); })); + it('should update the option selected state if the same array is mutated and passed back in', + fakeAsync(() => { + const value: string[] = []; + trigger.click(); + testInstance.control.setValue(value); + fixture.detectChanges(); + + const optionNodes = + Array.from(overlayContainerElement.querySelectorAll('mat-option')); + const optionInstances = testInstance.options.toArray(); + + expect(optionNodes.some(option => { + return option.classList.contains('mdc-list-item--selected'); + })).toBe(false); + expect(optionInstances.some(option => option.selected)).toBe(false); + + value.push('eggs-5'); + testInstance.control.setValue(value); + fixture.detectChanges(); + + expect(optionNodes[5].classList).toContain('mdc-list-item--selected'); + expect(optionInstances[5].selected).toBe(true); + })); + }); it('should be able to provide default values through an injection token', fakeAsync(() => { diff --git a/src/material/select/select.spec.ts b/src/material/select/select.spec.ts index f868b5df8a7e..2fc6a5c5d6de 100644 --- a/src/material/select/select.spec.ts +++ b/src/material/select/select.spec.ts @@ -4666,6 +4666,28 @@ describe('MatSelect', () => { .toEqual([true, true, true, true, false, false]); })); + it('should update the option selected state if the same array is mutated and passed back in', + fakeAsync(() => { + const value: string[] = []; + trigger.click(); + testInstance.control.setValue(value); + fixture.detectChanges(); + + const optionNodes = + Array.from(overlayContainerElement.querySelectorAll('mat-option')); + const optionInstances = testInstance.options.toArray(); + + expect(optionNodes.some(option => option.classList.contains('mat-selected'))).toBe(false); + expect(optionInstances.some(option => option.selected)).toBe(false); + + value.push('eggs-5'); + testInstance.control.setValue(value); + fixture.detectChanges(); + + expect(optionNodes[5].classList).toContain('mat-selected'); + expect(optionInstances[5].selected).toBe(true); + })); + }); it('should be able to provide default values through an injection token', fakeAsync(() => { diff --git a/src/material/select/select.ts b/src/material/select/select.ts index 118def34469e..afe802bfabb1 100644 --- a/src/material/select/select.ts +++ b/src/material/select/select.ts @@ -402,7 +402,8 @@ export abstract class _MatSelectBase extends _MatSelectMixinBase implements A @Input() get value(): any { return this._value; } set value(newValue: any) { - if (newValue !== this._value) { + // Always re-assign an array, because it might have been mutated. + if (newValue !== this._value || (this._multiple && Array.isArray(newValue))) { if (this.options) { this._setSelectionByValue(newValue); }