Skip to content

Commit

Permalink
fix(select): handle optionSelectionChanges being accessed early (#8830
Browse files Browse the repository at this point in the history
)

Along the same lines #8802, `mat-select` will throw if the `optionSelectionChanges` is accessed before the options are initialized. These changes add a fallback that will replace the observable once the options become available.
  • Loading branch information
crisbeto authored and andrewseguin committed Dec 19, 2017
1 parent 657f649 commit c98321c
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 3 deletions.
50 changes: 50 additions & 0 deletions src/lib/select/select.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,14 @@ import {
FloatLabelType,
MAT_LABEL_GLOBAL_OPTIONS,
MatOption,
MatOptionSelectionChange,
} from '@angular/material/core';
import {MatFormFieldModule} from '@angular/material/form-field';
import {By} from '@angular/platform-browser';
import {NoopAnimationsModule} from '@angular/platform-browser/animations';
import {map} from 'rxjs/operators/map';
import {Subject} from 'rxjs/Subject';
import {Subscription} from 'rxjs/Subscription';
import {MatSelectModule} from './index';
import {MatSelect} from './select';
import {
Expand Down Expand Up @@ -1001,6 +1003,54 @@ describe('MatSelect', () => {
it('should not throw if triggerValue accessed with no selected value', fakeAsync(() => {
expect(() => fixture.componentInstance.select.triggerValue).not.toThrow();
}));

it('should emit to `optionSelectionChanges` when an option is selected', fakeAsync(() => {
trigger.click();
fixture.detectChanges();
flush();

const spy = jasmine.createSpy('option selection spy');
const subscription = fixture.componentInstance.select.optionSelectionChanges.subscribe(spy);
const option = overlayContainerElement.querySelector('mat-option') as HTMLElement;
option.click();
fixture.detectChanges();
flush();

expect(spy).toHaveBeenCalledWith(jasmine.any(MatOptionSelectionChange));

subscription.unsubscribe();
}));

it('should handle accessing `optionSelectionChanges` before the options are initialized',
fakeAsync(() => {
fixture.destroy();
fixture = TestBed.createComponent(BasicSelect);

let spy = jasmine.createSpy('option selection spy');
let subscription: Subscription;

expect(fixture.componentInstance.select.options).toBeFalsy();
expect(() => {
subscription = fixture.componentInstance.select.optionSelectionChanges.subscribe(spy);
}).not.toThrow();

fixture.detectChanges();
trigger = fixture.debugElement.query(By.css('.mat-select-trigger')).nativeElement;

trigger.click();
fixture.detectChanges();
flush();

const option = overlayContainerElement.querySelector('mat-option') as HTMLElement;
option.click();
fixture.detectChanges();
flush();

expect(spy).toHaveBeenCalledWith(jasmine.any(MatOptionSelectionChange));

subscription!.unsubscribe();
}));

});

describe('forms integration', () => {
Expand Down
14 changes: 11 additions & 3 deletions src/lib/select/select.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
import {filter} from 'rxjs/operators/filter';
import {take} from 'rxjs/operators/take';
import {map} from 'rxjs/operators/map';
import {switchMap} from 'rxjs/operators/switchMap';
import {startWith} from 'rxjs/operators/startWith';
import {takeUntil} from 'rxjs/operators/takeUntil';
import {
Expand Down Expand Up @@ -75,6 +76,7 @@ import {MatFormField, MatFormFieldControl} from '@angular/material/form-field';
import {Observable} from 'rxjs/Observable';
import {merge} from 'rxjs/observable/merge';
import {Subject} from 'rxjs/Subject';
import {defer} from 'rxjs/observable/defer';
import {fadeInContent, transformPanel} from './select-animations';
import {
getMatSelectDynamicMultipleError,
Expand Down Expand Up @@ -397,9 +399,15 @@ export class MatSelect extends _MatSelectMixinBase implements AfterContentInit,
private _id: string;

/** Combined stream of all of the child options' change events. */
get optionSelectionChanges(): Observable<MatOptionSelectionChange> {
return merge(...this.options.map(option => option.onSelectionChange));
}
optionSelectionChanges: Observable<MatOptionSelectionChange> = defer(() => {
if (this.options) {
return merge(...this.options.map(option => option.onSelectionChange));
}

return this._ngZone.onStable
.asObservable()
.pipe(take(1), switchMap(() => this.optionSelectionChanges));
});

/** Event emitted when the select has been opened. */
@Output() openedChange: EventEmitter<boolean> = new EventEmitter<boolean>();
Expand Down

0 comments on commit c98321c

Please sign in to comment.