Skip to content

Commit 245caae

Browse files
crisbetojelbourn
authored andcommitted
fix(select): handle async changes to the option label (#9159)
Currently `mat-select` doesn't react to changes in the label of its options, which is problematic, because the option label might be populated at a later point by a pipe or it might have changed while the select is closed. These changes add a `Subject` to the `MatOption` that will emit whenever the label changes and allows the select to react accordingly. Fixes #7923.
1 parent 4f65276 commit 245caae

File tree

3 files changed

+56
-13
lines changed

3 files changed

+56
-13
lines changed

src/lib/core/option/option.ts

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
import {coerceBooleanProperty} from '@angular/cdk/coercion';
1010
import {ENTER, SPACE} from '@angular/cdk/keycodes';
11+
import {Subject} from 'rxjs/Subject';
1112
import {
1213
ChangeDetectionStrategy,
1314
ChangeDetectorRef,
@@ -21,6 +22,7 @@ import {
2122
ViewEncapsulation,
2223
InjectionToken,
2324
Inject,
25+
AfterViewChecked,
2426
} from '@angular/core';
2527
import {MatOptgroup} from './optgroup';
2628

@@ -82,11 +84,12 @@ export const MAT_OPTION_PARENT_COMPONENT =
8284
preserveWhitespaces: false,
8385
changeDetection: ChangeDetectionStrategy.OnPush,
8486
})
85-
export class MatOption {
87+
export class MatOption implements AfterViewChecked {
8688
private _selected = false;
8789
private _active = false;
8890
private _disabled = false;
8991
private _id = `mat-option-${_uniqueIdCounter++}`;
92+
private _mostRecentViewValue = '';
9093

9194
/** Whether the wrapping component is in multiple selection mode. */
9295
get multiple() { return this._parent && this._parent.multiple; }
@@ -111,6 +114,9 @@ export class MatOption {
111114
/** Event emitted when the option is selected or deselected. */
112115
@Output() onSelectionChange = new EventEmitter<MatOptionSelectionChange>();
113116

117+
/** Emits when the state of the option changes and any parents have to be notified. */
118+
_stateChanges = new Subject<void>();
119+
114120
constructor(
115121
private _element: ElementRef,
116122
private _changeDetectorRef: ChangeDetectorRef,
@@ -220,6 +226,22 @@ export class MatOption {
220226
return this._element.nativeElement;
221227
}
222228

229+
ngAfterViewChecked() {
230+
// Since parent components could be using the option's label to display the selected values
231+
// (e.g. `mat-select`) and they don't have a way of knowing if the option's label has changed
232+
// we have to check for changes in the DOM ourselves and dispatch an event. These checks are
233+
// relatively cheap, however we still limit them only to selected options in order to avoid
234+
// hitting the DOM too often.
235+
if (this._selected) {
236+
const viewValue = this.viewValue;
237+
238+
if (viewValue !== this._mostRecentViewValue) {
239+
this._mostRecentViewValue = viewValue;
240+
this._stateChanges.next();
241+
}
242+
}
243+
}
244+
223245
/** Emits the selection change event. */
224246
private _emitSelectionChangeEvent(isUserInput = false): void {
225247
this.onSelectionChange.emit(new MatOptionSelectionChange(this, isUserInput));

src/lib/select/select.spec.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1097,6 +1097,18 @@ describe('MatSelect', () => {
10971097
.toBe(fixture.componentInstance.options.last);
10981098
}));
10991099

1100+
it('should update the trigger when the selected option label is changed', fakeAsync(() => {
1101+
fixture.componentInstance.control.setValue('pizza-1');
1102+
fixture.detectChanges();
1103+
1104+
expect(trigger.textContent!.trim()).toBe('Pizza');
1105+
1106+
fixture.componentInstance.foods[1].viewValue = 'Calzone';
1107+
fixture.detectChanges();
1108+
1109+
expect(trigger.textContent!.trim()).toBe('Calzone');
1110+
}));
1111+
11001112
it('should not select disabled options', fakeAsync(() => {
11011113
trigger.click();
11021114
fixture.detectChanges();

src/lib/select/select.ts

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -833,7 +833,6 @@ export class MatSelect extends _MatSelectMixinBase implements AfterContentInit,
833833
private _initKeyManager() {
834834
this._keyManager = new ActiveDescendantKeyManager<MatOption>(this.options).withTypeAhead();
835835
this._keyManager.tabOut.pipe(takeUntil(this._destroy)).subscribe(() => this.close());
836-
837836
this._keyManager.change.pipe(takeUntil(this._destroy)).subscribe(() => {
838837
if (this._panelOpen && this.panel) {
839838
this._scrollActiveOptionIntoView();
@@ -845,17 +844,27 @@ export class MatSelect extends _MatSelectMixinBase implements AfterContentInit,
845844

846845
/** Drops current option subscriptions and IDs and resets from scratch. */
847846
private _resetOptions(): void {
848-
this.optionSelectionChanges.pipe(
849-
takeUntil(merge(this._destroy, this.options.changes)),
850-
filter(event => event.isUserInput)
851-
).subscribe(event => {
852-
this._onSelect(event.source);
853-
854-
if (!this.multiple && this._panelOpen) {
855-
this.close();
856-
this.focus();
857-
}
858-
});
847+
const changedOrDestroyed = merge(this.options.changes, this._destroy);
848+
849+
this.optionSelectionChanges
850+
.pipe(takeUntil(changedOrDestroyed), filter(event => event.isUserInput))
851+
.subscribe(event => {
852+
this._onSelect(event.source);
853+
854+
if (!this.multiple && this._panelOpen) {
855+
this.close();
856+
this.focus();
857+
}
858+
});
859+
860+
// Listen to changes in the internal state of the options and react accordingly.
861+
// Handles cases like the labels of the selected options changing.
862+
merge(...this.options.map(option => option._stateChanges))
863+
.pipe(takeUntil(changedOrDestroyed))
864+
.subscribe(() => {
865+
this._changeDetectorRef.markForCheck();
866+
this.stateChanges.next();
867+
});
859868

860869
this._setOptionIds();
861870
}

0 commit comments

Comments
 (0)