Skip to content

Commit

Permalink
fix(select): don't shift option focus when multi-select value is chan…
Browse files Browse the repository at this point in the history
…ged 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.
  • Loading branch information
crisbeto authored and andrewseguin committed Aug 15, 2017
1 parent 9ba5d84 commit 6422640
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 4 deletions.
25 changes: 25 additions & 0 deletions src/lib/select/select.spec.ts
Expand Up @@ -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<HTMLElement>;

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;

Expand Down
12 changes: 8 additions & 4 deletions src/lib/select/select.ts
Expand Up @@ -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();
Expand All @@ -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;
Expand Down

0 comments on commit 6422640

Please sign in to comment.