From c46cd331e05121c25f1f36686cb11658512b43c4 Mon Sep 17 00:00:00 2001 From: crisbeto Date: Fri, 12 Jan 2018 21:11:12 +0100 Subject: [PATCH] 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. --- src/lib/core/option/option.ts | 24 +++++++++++++++++++++++- src/lib/select/select.spec.ts | 12 ++++++++++++ src/lib/select/select.ts | 33 +++++++++++++++++++++------------ 3 files changed, 56 insertions(+), 13 deletions(-) diff --git a/src/lib/core/option/option.ts b/src/lib/core/option/option.ts index 40d1dfbca8fc..040582960a3f 100644 --- a/src/lib/core/option/option.ts +++ b/src/lib/core/option/option.ts @@ -8,6 +8,7 @@ import {coerceBooleanProperty} from '@angular/cdk/coercion'; import {ENTER, SPACE} from '@angular/cdk/keycodes'; +import {Subject} from 'rxjs/Subject'; import { ChangeDetectionStrategy, ChangeDetectorRef, @@ -21,6 +22,7 @@ import { ViewEncapsulation, InjectionToken, Inject, + AfterViewChecked, } from '@angular/core'; import {MatOptgroup} from './optgroup'; @@ -82,11 +84,12 @@ export const MAT_OPTION_PARENT_COMPONENT = preserveWhitespaces: false, changeDetection: ChangeDetectionStrategy.OnPush, }) -export class MatOption { +export class MatOption implements AfterViewChecked { private _selected = false; private _active = false; private _disabled = false; private _id = `mat-option-${_uniqueIdCounter++}`; + private _mostRecentViewValue = ''; /** Whether the wrapping component is in multiple selection mode. */ get multiple() { return this._parent && this._parent.multiple; } @@ -111,6 +114,9 @@ export class MatOption { /** Event emitted when the option is selected or deselected. */ @Output() onSelectionChange = new EventEmitter(); + /** Emits when the state of the option changes and any parents have to be notified. */ + _stateChanges = new Subject(); + constructor( private _element: ElementRef, private _changeDetectorRef: ChangeDetectorRef, @@ -220,6 +226,22 @@ export class MatOption { return this._element.nativeElement; } + ngAfterViewChecked() { + // Since parent components could be using the option's label to display the selected values + // (e.g. `mat-select`) and they don't have a way of knowing if the option's label has changed + // we have to check for changes in the DOM ourselves and dispatch an event. These checks are + // relatively cheap, however we still limit them only to selected options in order to avoid + // hitting the DOM too often. + if (this._selected) { + const viewValue = this.viewValue; + + if (viewValue !== this._mostRecentViewValue) { + this._mostRecentViewValue = viewValue; + this._stateChanges.next(); + } + } + } + /** Emits the selection change event. */ private _emitSelectionChangeEvent(isUserInput = false): void { this.onSelectionChange.emit(new MatOptionSelectionChange(this, isUserInput)); diff --git a/src/lib/select/select.spec.ts b/src/lib/select/select.spec.ts index 3029a24f6db0..168b7d376066 100644 --- a/src/lib/select/select.spec.ts +++ b/src/lib/select/select.spec.ts @@ -1097,6 +1097,18 @@ describe('MatSelect', () => { .toBe(fixture.componentInstance.options.last); })); + it('should update the trigger when the selected option label is changed', fakeAsync(() => { + fixture.componentInstance.control.setValue('pizza-1'); + fixture.detectChanges(); + + expect(trigger.textContent!.trim()).toBe('Pizza'); + + fixture.componentInstance.foods[1].viewValue = 'Calzone'; + fixture.detectChanges(); + + expect(trigger.textContent!.trim()).toBe('Calzone'); + })); + it('should not select disabled options', fakeAsync(() => { trigger.click(); fixture.detectChanges(); diff --git a/src/lib/select/select.ts b/src/lib/select/select.ts index b87327b0ddc8..0d80bc6f5364 100644 --- a/src/lib/select/select.ts +++ b/src/lib/select/select.ts @@ -833,7 +833,6 @@ export class MatSelect extends _MatSelectMixinBase implements AfterContentInit, private _initKeyManager() { this._keyManager = new ActiveDescendantKeyManager(this.options).withTypeAhead(); this._keyManager.tabOut.pipe(takeUntil(this._destroy)).subscribe(() => this.close()); - this._keyManager.change.pipe(takeUntil(this._destroy)).subscribe(() => { if (this._panelOpen && this.panel) { this._scrollActiveOptionIntoView(); @@ -845,17 +844,27 @@ export class MatSelect extends _MatSelectMixinBase implements AfterContentInit, /** Drops current option subscriptions and IDs and resets from scratch. */ private _resetOptions(): void { - this.optionSelectionChanges.pipe( - takeUntil(merge(this._destroy, this.options.changes)), - filter(event => event.isUserInput) - ).subscribe(event => { - this._onSelect(event.source); - - if (!this.multiple && this._panelOpen) { - this.close(); - this.focus(); - } - }); + const changedOrDestroyed = merge(this.options.changes, this._destroy); + + this.optionSelectionChanges + .pipe(takeUntil(changedOrDestroyed), filter(event => event.isUserInput)) + .subscribe(event => { + this._onSelect(event.source); + + if (!this.multiple && this._panelOpen) { + this.close(); + this.focus(); + } + }); + + // Listen to changes in the internal state of the options and react accordingly. + // Handles cases like the labels of the selected options changing. + merge(...this.options.map(option => option._stateChanges)) + .pipe(takeUntil(changedOrDestroyed)) + .subscribe(() => { + this._changeDetectorRef.markForCheck(); + this.stateChanges.next(); + }); this._setOptionIds(); }