Skip to content

Commit d35278b

Browse files
committed
fix(select): handle async changes to the option label
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 c3d7cd9 commit d35278b

File tree

3 files changed

+55
-12
lines changed

3 files changed

+55
-12
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

@@ -81,11 +83,12 @@ export const MAT_OPTION_PARENT_COMPONENT =
8183
preserveWhitespaces: false,
8284
changeDetection: ChangeDetectionStrategy.OnPush,
8385
})
84-
export class MatOption {
86+
export class MatOption implements AfterViewChecked {
8587
private _selected = false;
8688
private _active = false;
8789
private _disabled = false;
8890
private _id = `mat-option-${_uniqueIdCounter++}`;
91+
private _mostRecentViewValue = '';
8992

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

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

228+
ngAfterViewChecked() {
229+
// Since parent components could be using the option's label to display the selected values
230+
// (e.g. `mat-select`) and they don't have a way of knowing if the option's label has changed
231+
// we have to check for changes in the DOM ourselves and dispatch an event. These checks are
232+
// relatively cheap, however we still limit them only to selected options in order to avoid
233+
// hitting the DOM too often.
234+
if (this._selected) {
235+
const viewValue = this.viewValue;
236+
237+
if (viewValue !== this._mostRecentViewValue) {
238+
this._mostRecentViewValue = viewValue;
239+
this._stateChanges.next();
240+
}
241+
}
242+
}
243+
222244
/** Emits the selection change event. */
223245
private _emitSelectionChangeEvent(isUserInput = false): void {
224246
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
@@ -997,6 +997,18 @@ describe('MatSelect', () => {
997997
.toBe(fixture.componentInstance.options.last);
998998
}));
999999

1000+
it('should update the trigger when the selected option label is changed', fakeAsync(() => {
1001+
fixture.componentInstance.control.setValue('pizza-1');
1002+
fixture.detectChanges();
1003+
1004+
expect(trigger.textContent!.trim()).toBe('Pizza');
1005+
1006+
fixture.componentInstance.foods[1].viewValue = 'Calzone';
1007+
fixture.detectChanges();
1008+
1009+
expect(trigger.textContent!.trim()).toBe('Calzone');
1010+
}));
1011+
10001012
it('should not select disabled options', fakeAsync(() => {
10011013
trigger.click();
10021014
fixture.detectChanges();

src/lib/select/select.ts

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -817,7 +817,6 @@ export class MatSelect extends _MatSelectMixinBase implements AfterContentInit,
817817
private _initKeyManager() {
818818
this._keyManager = new ActiveDescendantKeyManager<MatOption>(this.options).withTypeAhead();
819819
this._keyManager.tabOut.pipe(takeUntil(this._destroy)).subscribe(() => this.close());
820-
821820
this._keyManager.change.pipe(takeUntil(this._destroy)).subscribe(() => {
822821
if (this._panelOpen && this.panel) {
823822
this._scrollActiveOptionIntoView();
@@ -829,16 +828,26 @@ export class MatSelect extends _MatSelectMixinBase implements AfterContentInit,
829828

830829
/** Drops current option subscriptions and IDs and resets from scratch. */
831830
private _resetOptions(): void {
832-
this.optionSelectionChanges.pipe(
833-
takeUntil(merge(this._destroy, this.options.changes)),
834-
filter(event => event.isUserInput)
835-
).subscribe(event => {
836-
this._onSelect(event.source);
837-
838-
if (!this.multiple) {
839-
this.close();
840-
}
841-
});
831+
const changedOrDestroyed = merge(this.options.changes, this._destroy);
832+
833+
this.optionSelectionChanges
834+
.pipe(takeUntil(changedOrDestroyed), filter(event => event.isUserInput))
835+
.subscribe(event => {
836+
this._onSelect(event.source);
837+
838+
if (!this.multiple) {
839+
this.close();
840+
}
841+
});
842+
843+
// Listen to changes in the internal state of the options and react accordingly.
844+
// Handles cases like the labels of the selected options changing.
845+
merge(...this.options.map(option => option._stateChanges))
846+
.pipe(takeUntil(changedOrDestroyed))
847+
.subscribe(() => {
848+
this._changeDetectorRef.markForCheck();
849+
this.stateChanges.next();
850+
});
842851

843852
this._setOptionIds();
844853
}

0 commit comments

Comments
 (0)