From 6422640b038814d9386c3dd668e9d0a262cbe3a8 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Tue, 15 Aug 2017 23:54:08 +0200 Subject: [PATCH] fix(select): don't shift option focus when multi-select value is changed programmatically (#5401) Doesn't shift focus when the value of a multi select is updated programmatically while the panel is still open. Previously focus would end up on the last selected option, because we move focus in a loop. Fixes #5381. --- src/lib/select/select.spec.ts | 25 +++++++++++++++++++++++++ src/lib/select/select.ts | 12 ++++++++---- 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/src/lib/select/select.spec.ts b/src/lib/select/select.spec.ts index ee55075a76fe..fd7f7b32a28c 100644 --- a/src/lib/select/select.spec.ts +++ b/src/lib/select/select.spec.ts @@ -1838,6 +1838,31 @@ describe('MdSelect', () => { expect(fixture.componentInstance.options.toArray()[6].selected).toBe(true); }); + it('should not shift focus when the selected options are updated programmatically ' + + 'in a multi select', () => { + fixture.destroy(); + + const multiFixture = TestBed.createComponent(MultiSelect); + const instance = multiFixture.componentInstance; + + multiFixture.detectChanges(); + select = multiFixture.debugElement.query(By.css('md-select')).nativeElement; + multiFixture.componentInstance.select.open(); + multiFixture.detectChanges(); + + const options = + overlayContainerElement.querySelectorAll('md-option') as NodeListOf; + + options[3].focus(); + expect(document.activeElement).toBe(options[3], 'Expected fourth option to be focused.'); + + multiFixture.componentInstance.control.setValue(['steak-0', 'sushi-7']); + multiFixture.detectChanges(); + + expect(document.activeElement) + .toBe(options[3], 'Expected fourth option to remain focused.'); + }); + it('should not cycle through the options if the control is disabled', () => { const formControl = fixture.componentInstance.control; diff --git a/src/lib/select/select.ts b/src/lib/select/select.ts index 1ce46210c567..58b1ff7ece81 100644 --- a/src/lib/select/select.ts +++ b/src/lib/select/select.ts @@ -687,7 +687,13 @@ export class MdSelect extends _MdSelectMixinBase implements AfterContentInit, On value.forEach((currentValue: any) => this._selectValue(currentValue, isUserInput)); this._sortValues(); } else { - this._selectValue(value, isUserInput); + const correspondingOption = this._selectValue(value, isUserInput); + + // 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. + if (correspondingOption) { + this._keyManager.setActiveItem(this.options.toArray().indexOf(correspondingOption)); + } } this._setValueWidth(); @@ -704,15 +710,13 @@ export class MdSelect extends _MdSelectMixinBase implements AfterContentInit, On * @returns Option that has the corresponding value. */ private _selectValue(value: any, isUserInput = false): MdOption | undefined { - let optionsArray = this.options.toArray(); - let correspondingOption = optionsArray.find(option => { + let correspondingOption = this.options.find(option => { return option.value != null && option.value === value; }); if (correspondingOption) { isUserInput ? correspondingOption._selectViaInteraction() : correspondingOption.select(); this._selectionModel.select(correspondingOption); - this._keyManager.setActiveItem(optionsArray.indexOf(correspondingOption)); } return correspondingOption;