From 1efa47d7790d58c6396bc4d8530716a473de503e Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Tue, 29 Nov 2022 16:06:05 +0100 Subject: [PATCH] fix(material/select): changed after checked error if option label changes Fixes a "changed after checked" error that is thrown if the label of a selected option changes dynamically. This is alternate approach to #14797 which was tricky to land, because it introduced an extra timeout. Fixes #14793. --- src/material/core/option/option.ts | 5 ++++- src/material/legacy-select/select.spec.ts | 16 +++++++++++++++- src/material/select/select.spec.ts | 16 +++++++++++++++- src/material/select/select.ts | 5 ++++- 4 files changed, 38 insertions(+), 4 deletions(-) diff --git a/src/material/core/option/option.ts b/src/material/core/option/option.ts index 6a9fe92f2fce..48d4d8b0b907 100644 --- a/src/material/core/option/option.ts +++ b/src/material/core/option/option.ts @@ -225,8 +225,11 @@ export class _MatOptionBase implements FocusableOption, AfterViewChecke const viewValue = this.viewValue; if (viewValue !== this._mostRecentViewValue) { + if (this._mostRecentViewValue) { + this._stateChanges.next(); + } + this._mostRecentViewValue = viewValue; - this._stateChanges.next(); } } } diff --git a/src/material/legacy-select/select.spec.ts b/src/material/legacy-select/select.spec.ts index 2752dbd17e9c..bb98cf8661c0 100644 --- a/src/material/legacy-select/select.spec.ts +++ b/src/material/legacy-select/select.spec.ts @@ -1831,6 +1831,19 @@ describe('MatSelect', () => { expect(trigger.textContent!.trim()).toBe('Calzone'); })); + it('should update the trigger value if the text as a result of an expression change', fakeAsync(() => { + fixture.componentInstance.control.setValue('pizza-1'); + fixture.detectChanges(); + + expect(trigger.textContent!.trim()).toBe('Pizza'); + + fixture.componentInstance.capitalize = true; + fixture.detectChanges(); + fixture.checkNoChanges(); + + expect(trigger.textContent!.trim()).toBe('PIZZA'); + })); + it('should not select disabled options', fakeAsync(() => { trigger.click(); fixture.detectChanges(); @@ -5202,7 +5215,7 @@ describe('MatSelect', () => { [panelClass]="panelClass" [disableRipple]="disableRipple" [typeaheadDebounceInterval]="typeaheadDebounceInterval"> - {{ food.viewValue }} + {{ capitalize ? food.viewValue.toUpperCase() : food.viewValue }} {{ hint }} @@ -5233,6 +5246,7 @@ class BasicSelect { panelClass = ['custom-one', 'custom-two']; disableRipple: boolean; typeaheadDebounceInterval: number; + capitalize = false; @ViewChild(MatLegacySelect, {static: true}) select: MatLegacySelect; @ViewChildren(MatLegacyOption) options: QueryList; diff --git a/src/material/select/select.spec.ts b/src/material/select/select.spec.ts index 783ca58e60c2..6fd926e909b0 100644 --- a/src/material/select/select.spec.ts +++ b/src/material/select/select.spec.ts @@ -1879,6 +1879,19 @@ describe('MDC-based MatSelect', () => { expect(trigger.textContent!.trim()).toBe('Calzone'); })); + it('should update the trigger value if the text as a result of an expression change', fakeAsync(() => { + fixture.componentInstance.control.setValue('pizza-1'); + fixture.detectChanges(); + + expect(trigger.textContent!.trim()).toBe('Pizza'); + + fixture.componentInstance.capitalize = true; + fixture.detectChanges(); + fixture.checkNoChanges(); + + expect(trigger.textContent!.trim()).toBe('PIZZA'); + })); + it('should not select disabled options', fakeAsync(() => { trigger.click(); fixture.detectChanges(); @@ -4410,7 +4423,7 @@ describe('MDC-based MatSelect', () => { [panelClass]="panelClass" [disableRipple]="disableRipple" [typeaheadDebounceInterval]="typeaheadDebounceInterval"> - {{ food.viewValue }} + {{ capitalize ? food.viewValue.toUpperCase() : food.viewValue }} {{ hint }} @@ -4442,6 +4455,7 @@ class BasicSelect { panelClass = ['custom-one', 'custom-two']; disableRipple: boolean; typeaheadDebounceInterval: number; + capitalize = false; @ViewChild(MatSelect, {static: true}) select: MatSelect; @ViewChildren(MatOption) options: QueryList; diff --git a/src/material/select/select.ts b/src/material/select/select.ts index b797d350cf6b..8fd4921c3568 100644 --- a/src/material/select/select.ts +++ b/src/material/select/select.ts @@ -961,7 +961,10 @@ export abstract class _MatSelectBase merge(...this.options.map(option => option._stateChanges)) .pipe(takeUntil(changedOrDestroyed)) .subscribe(() => { - this._changeDetectorRef.markForCheck(); + // `_stateChanges` can fire as a result of a change in the label's DOM value which may + // be the result of an expression changing. We have to use `detectChanges` in order + // to avoid "changed after checked" errors (see #14793). + this._changeDetectorRef.detectChanges(); this.stateChanges.next(); }); }