Skip to content

Commit 6422640

Browse files
crisbetoandrewseguin
authored andcommitted
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.
1 parent 9ba5d84 commit 6422640

File tree

2 files changed

+33
-4
lines changed

2 files changed

+33
-4
lines changed

src/lib/select/select.spec.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1838,6 +1838,31 @@ describe('MdSelect', () => {
18381838
expect(fixture.componentInstance.options.toArray()[6].selected).toBe(true);
18391839
});
18401840

1841+
it('should not shift focus when the selected options are updated programmatically ' +
1842+
'in a multi select', () => {
1843+
fixture.destroy();
1844+
1845+
const multiFixture = TestBed.createComponent(MultiSelect);
1846+
const instance = multiFixture.componentInstance;
1847+
1848+
multiFixture.detectChanges();
1849+
select = multiFixture.debugElement.query(By.css('md-select')).nativeElement;
1850+
multiFixture.componentInstance.select.open();
1851+
multiFixture.detectChanges();
1852+
1853+
const options =
1854+
overlayContainerElement.querySelectorAll('md-option') as NodeListOf<HTMLElement>;
1855+
1856+
options[3].focus();
1857+
expect(document.activeElement).toBe(options[3], 'Expected fourth option to be focused.');
1858+
1859+
multiFixture.componentInstance.control.setValue(['steak-0', 'sushi-7']);
1860+
multiFixture.detectChanges();
1861+
1862+
expect(document.activeElement)
1863+
.toBe(options[3], 'Expected fourth option to remain focused.');
1864+
});
1865+
18411866
it('should not cycle through the options if the control is disabled', () => {
18421867
const formControl = fixture.componentInstance.control;
18431868

src/lib/select/select.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -687,7 +687,13 @@ export class MdSelect extends _MdSelectMixinBase implements AfterContentInit, On
687687
value.forEach((currentValue: any) => this._selectValue(currentValue, isUserInput));
688688
this._sortValues();
689689
} else {
690-
this._selectValue(value, isUserInput);
690+
const correspondingOption = this._selectValue(value, isUserInput);
691+
692+
// Shift focus to the active item. Note that we shouldn't do this in multiple
693+
// mode, because we don't know what option the user interacted with last.
694+
if (correspondingOption) {
695+
this._keyManager.setActiveItem(this.options.toArray().indexOf(correspondingOption));
696+
}
691697
}
692698

693699
this._setValueWidth();
@@ -704,15 +710,13 @@ export class MdSelect extends _MdSelectMixinBase implements AfterContentInit, On
704710
* @returns Option that has the corresponding value.
705711
*/
706712
private _selectValue(value: any, isUserInput = false): MdOption | undefined {
707-
let optionsArray = this.options.toArray();
708-
let correspondingOption = optionsArray.find(option => {
713+
let correspondingOption = this.options.find(option => {
709714
return option.value != null && option.value === value;
710715
});
711716

712717
if (correspondingOption) {
713718
isUserInput ? correspondingOption._selectViaInteraction() : correspondingOption.select();
714719
this._selectionModel.select(correspondingOption);
715-
this._keyManager.setActiveItem(optionsArray.indexOf(correspondingOption));
716720
}
717721

718722
return correspondingOption;

0 commit comments

Comments
 (0)