diff --git a/src/cdk/collections/selection.spec.ts b/src/cdk/collections/selection.spec.ts index 452f59b4c1cd..36cfa8337b09 100644 --- a/src/cdk/collections/selection.spec.ts +++ b/src/cdk/collections/selection.spec.ts @@ -82,6 +82,14 @@ describe('SelectionModel', () => { expect(model.selected).toEqual([1, 2, 3]); }); + + it('should sort values if `selected` has not been accessed before', () => { + model = new SelectionModel(true, [2, 3, 1]); + + // Important: don't assert `selected` before sorting so the getter isn't invoked + model.sort(); + expect(model.selected).toEqual([1, 2, 3]); + }); }); describe('onChange event', () => { diff --git a/src/cdk/collections/selection.ts b/src/cdk/collections/selection.ts index d04471d91ec5..46df9f6811f2 100644 --- a/src/cdk/collections/selection.ts +++ b/src/cdk/collections/selection.ts @@ -13,7 +13,7 @@ import {Subject} from 'rxjs'; */ export class SelectionModel { /** Currently-selected values. */ - private _selection: Set = new Set(); + private _selection = new Set(); /** Keeps track of the deselected options that haven't been emitted by the change event. */ private _deselectedToEmit: T[] = []; @@ -111,8 +111,8 @@ export class SelectionModel { * Sorts the selected values based on a predicate function. */ sort(predicate?: (a: T, b: T) => number): void { - if (this._multiple && this._selected) { - this._selected.sort(predicate); + if (this._multiple && this.selected) { + this._selected!.sort(predicate); } } diff --git a/src/lib/core/option/option.spec.ts b/src/lib/core/option/option.spec.ts index 9eceeb530e2f..d1b4949d3b0b 100644 --- a/src/lib/core/option/option.spec.ts +++ b/src/lib/core/option/option.spec.ts @@ -27,6 +27,50 @@ describe('MatOption component', () => { subscription.unsubscribe(); }); + it('should not emit to `onSelectionChange` if selecting an already-selected option', () => { + const fixture = TestBed.createComponent(OptionWithDisable); + fixture.detectChanges(); + + const optionInstance: MatOption = + fixture.debugElement.query(By.directive(MatOption)).componentInstance; + + optionInstance.select(); + expect(optionInstance.selected).toBe(true); + + const spy = jasmine.createSpy('selection change spy'); + const subscription = optionInstance.onSelectionChange.subscribe(spy); + + optionInstance.select(); + fixture.detectChanges(); + + expect(optionInstance.selected).toBe(true); + expect(spy).not.toHaveBeenCalled(); + + subscription.unsubscribe(); + }); + + it('should not emit to `onSelectionChange` if deselecting an unselected option', () => { + const fixture = TestBed.createComponent(OptionWithDisable); + fixture.detectChanges(); + + const optionInstance: MatOption = + fixture.debugElement.query(By.directive(MatOption)).componentInstance; + + optionInstance.deselect(); + expect(optionInstance.selected).toBe(false); + + const spy = jasmine.createSpy('selection change spy'); + const subscription = optionInstance.onSelectionChange.subscribe(spy); + + optionInstance.deselect(); + fixture.detectChanges(); + + expect(optionInstance.selected).toBe(false); + expect(spy).not.toHaveBeenCalled(); + + subscription.unsubscribe(); + }); + describe('ripples', () => { let fixture: ComponentFixture; let optionDebugElement: DebugElement; diff --git a/src/lib/core/option/option.ts b/src/lib/core/option/option.ts index 75c468d49b25..3b0339c1804d 100644 --- a/src/lib/core/option/option.ts +++ b/src/lib/core/option/option.ts @@ -145,16 +145,20 @@ export class MatOption implements AfterViewChecked, OnDestroy { /** Selects the option. */ select(): void { - this._selected = true; - this._changeDetectorRef.markForCheck(); - this._emitSelectionChangeEvent(); + if (!this._selected) { + this._selected = true; + this._changeDetectorRef.markForCheck(); + this._emitSelectionChangeEvent(); + } } /** Deselects the option. */ deselect(): void { - this._selected = false; - this._changeDetectorRef.markForCheck(); - this._emitSelectionChangeEvent(); + if (this._selected) { + this._selected = false; + this._changeDetectorRef.markForCheck(); + this._emitSelectionChangeEvent(); + } } /** Sets focus onto this option. */ diff --git a/src/lib/select/select.spec.ts b/src/lib/select/select.spec.ts index 0238805cc9d5..72f3debc0cdd 100644 --- a/src/lib/select/select.spec.ts +++ b/src/lib/select/select.spec.ts @@ -86,7 +86,7 @@ describe('MatSelect', () => { * overall test time. * @param declarations Components to declare for this block */ - function configureMatSelectTestingModule(declarations) { + function configureMatSelectTestingModule(declarations: any[]) { TestBed.configureTestingModule({ imports: [ MatFormFieldModule, @@ -1096,6 +1096,23 @@ describe('MatSelect', () => { .toBe(fixture.componentInstance.options.first); })); + it('should be able to select an option using the MatOption API', fakeAsync(() => { + trigger.click(); + fixture.detectChanges(); + flush(); + + const optionInstances = fixture.componentInstance.options.toArray(); + const optionNodes: NodeListOf = + overlayContainerElement.querySelectorAll('mat-option'); + + optionInstances[1].select(); + fixture.detectChanges(); + + expect(optionNodes[1].classList).toContain('mat-selected'); + expect(optionInstances[1].selected).toBe(true); + expect(fixture.componentInstance.select.selected).toBe(optionInstances[1]); + })); + it('should deselect other options when one is selected', fakeAsync(() => { trigger.click(); fixture.detectChanges(); diff --git a/src/lib/select/select.ts b/src/lib/select/select.ts index 4286e4119869..914a6b1feae3 100644 --- a/src/lib/select/select.ts +++ b/src/lib/select/select.ts @@ -468,13 +468,18 @@ export class MatSelect extends _MatSelectMixinBase implements AfterContentInit, } ngOnInit() { - this._selectionModel = new SelectionModel(this.multiple, undefined, false); + this._selectionModel = new SelectionModel(this.multiple); this.stateChanges.next(); } ngAfterContentInit() { this._initKeyManager(); + this._selectionModel.onChange!.pipe(takeUntil(this._destroy)).subscribe(event => { + event.added.forEach(option => option.select()); + event.removed.forEach(option => option.deselect()); + }); + this.options.changes.pipe(startWith(null), takeUntil(this._destroy)).subscribe(() => { this._resetOptions(); this._initializeSelection(); @@ -753,19 +758,18 @@ export class MatSelect extends _MatSelectMixinBase implements AfterContentInit, * Sets the selected option based on a value. If no option can be * found with the designated value, the select trigger is cleared. */ - private _setSelectionByValue(value: any | any[], isUserInput = false): void { + private _setSelectionByValue(value: any | any[]): void { if (this.multiple && value) { if (!Array.isArray(value)) { throw getMatSelectNonArrayValueError(); } - this._clearSelection(); - value.forEach((currentValue: any) => this._selectValue(currentValue, isUserInput)); + this._selectionModel.clear(); + value.forEach((currentValue: any) => this._selectValue(currentValue)); this._sortValues(); } else { - this._clearSelection(); - - const correspondingOption = this._selectValue(value, isUserInput); + this._selectionModel.clear(); + const correspondingOption = this._selectValue(value); // Shift focus to the active item. Note that we shouldn't do this in multiple // mode, because we don't know what option the user interacted with last. @@ -781,7 +785,7 @@ export class MatSelect extends _MatSelectMixinBase implements AfterContentInit, * Finds and selects and option based on its value. * @returns Option that has the corresponding value. */ - private _selectValue(value: any, isUserInput = false): MatOption | undefined { + private _selectValue(value: any): MatOption | undefined { const correspondingOption = this.options.find((option: MatOption) => { try { // Treat null as a special reset value. @@ -796,29 +800,12 @@ export class MatSelect extends _MatSelectMixinBase implements AfterContentInit, }); if (correspondingOption) { - isUserInput ? correspondingOption._selectViaInteraction() : correspondingOption.select(); this._selectionModel.select(correspondingOption); - this.stateChanges.next(); } return correspondingOption; } - - /** - * Clears the select trigger and deselects every option in the list. - * @param skip Option that should not be deselected. - */ - private _clearSelection(skip?: MatOption): void { - this._selectionModel.clear(); - this.options.forEach(option => { - if (option !== skip) { - option.deselect(); - } - }); - this.stateChanges.next(); - } - /** Sets up a key manager to listen to keyboard events on the overlay panel. */ private _initKeyManager() { this._keyManager = new ActiveDescendantKeyManager(this.options) @@ -846,16 +833,14 @@ export class MatSelect extends _MatSelectMixinBase implements AfterContentInit, private _resetOptions(): void { const changedOrDestroyed = merge(this.options.changes, this._destroy); - this.optionSelectionChanges - .pipe(takeUntil(changedOrDestroyed), filter(event => event.isUserInput)) - .subscribe(event => { - this._onSelect(event.source); + this.optionSelectionChanges.pipe(takeUntil(changedOrDestroyed)).subscribe(event => { + this._onSelect(event.source, event.isUserInput); - if (!this.multiple && this._panelOpen) { - this.close(); - this.focus(); - } - }); + if (event.isUserInput && !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. @@ -870,51 +855,42 @@ export class MatSelect extends _MatSelectMixinBase implements AfterContentInit, } /** Invoked when an option is clicked. */ - private _onSelect(option: MatOption): void { + private _onSelect(option: MatOption, isUserInput: boolean): void { const wasSelected = this._selectionModel.isSelected(option); - // TODO(crisbeto): handle blank/null options inside multi-select. - if (this.multiple) { - this._selectionModel.toggle(option); - this.stateChanges.next(); - wasSelected ? option.deselect() : option.select(); - this._keyManager.setActiveItem(option); - this._sortValues(); - - // In case the user select the option with their mouse, we - // want to restore focus back to the trigger, in order to - // prevent the select keyboard controls from clashing with - // the ones from `mat-option`. - this.focus(); + if (option.value == null) { + this._selectionModel.clear(); + this._propagateChanges(option.value); } else { - this._clearSelection(option.value == null ? undefined : option); - - if (option.value == null) { - this._propagateChanges(option.value); - } else { - this._selectionModel.select(option); - this.stateChanges.next(); + option.selected ? this._selectionModel.select(option) : this._selectionModel.deselect(option); + + // TODO(crisbeto): handle blank/null options inside multi-select. + if (this.multiple) { + this._sortValues(); + + if (isUserInput) { + this._keyManager.setActiveItem(option); + // In case the user selected the option with their mouse, we + // want to restore focus back to the trigger, in order to + // prevent the select keyboard controls from clashing with + // the ones from `mat-option`. + this.focus(); + } } } if (wasSelected !== this._selectionModel.isSelected(option)) { this._propagateChanges(); } - } - /** - * Sorts the model values, ensuring that they keep the same - * order that they have in the panel. - */ - private _sortValues(): void { - if (this._multiple) { - this._selectionModel.clear(); + this.stateChanges.next(); + } - this.options.forEach(option => { - if (option.selected) { - this._selectionModel.select(option); - } - }); + /** Sorts the selected values in the selected based on their order in the panel. */ + private _sortValues() { + if (this.multiple) { + const options = this.options.toArray(); + this._selectionModel.sort((a, b) => options.indexOf(a) - options.indexOf(b)); this.stateChanges.next(); } }