Skip to content

fix(select): handle async changes to the option label #9159

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 24, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 23 additions & 1 deletion src/lib/core/option/option.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -21,6 +22,7 @@ import {
ViewEncapsulation,
InjectionToken,
Inject,
AfterViewChecked,
} from '@angular/core';
import {MatOptgroup} from './optgroup';

Expand Down Expand Up @@ -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; }
Expand All @@ -111,6 +114,9 @@ export class MatOption {
/** Event emitted when the option is selected or deselected. */
@Output() onSelectionChange = new EventEmitter<MatOptionSelectionChange>();

/** Emits when the state of the option changes and any parents have to be notified. */
_stateChanges = new Subject<void>();

constructor(
private _element: ElementRef,
private _changeDetectorRef: ChangeDetectorRef,
Expand Down Expand Up @@ -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));
Expand Down
12 changes: 12 additions & 0 deletions src/lib/select/select.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
33 changes: 21 additions & 12 deletions src/lib/select/select.ts
Original file line number Diff line number Diff line change
Expand Up @@ -833,7 +833,6 @@ export class MatSelect extends _MatSelectMixinBase implements AfterContentInit,
private _initKeyManager() {
this._keyManager = new ActiveDescendantKeyManager<MatOption>(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();
Expand All @@ -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();
}
Expand Down