Skip to content

Commit

Permalink
fix(chips): programmatically selected chip stealing focus (#7978)
Browse files Browse the repository at this point in the history
Currently chips that are selected programmatically (e.g. by setting the of `ngModel`) will moves focus to themselves, causing the page the scroll down. This can be observed in the demo app where the bottom chip instances have a preselected value which shifts focus to the bottom. These changes fix the issue by only moving focus if the selection was a result of a user interaction.
  • Loading branch information
crisbeto authored and andrewseguin committed Nov 2, 2017
1 parent c8df2c1 commit 8168667
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 2 deletions.
11 changes: 11 additions & 0 deletions src/lib/chips/chip-list.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -577,6 +577,17 @@ describe('MatChipList', () => {
expect(falsyFixture.componentInstance.chips.first.selected)
.toBe(true, 'Expected first option to be selected');
});

it('should not focus the active chip when the value is set programmatically', () => {
const chipArray = fixture.componentInstance.chips.toArray();

spyOn(chipArray[4], 'focus').and.callThrough();

fixture.componentInstance.control.setValue('chips-4');
fixture.detectChanges();

expect(chipArray[4].focus).not.toHaveBeenCalled();
});
});

describe('multiple selection', () => {
Expand Down
10 changes: 8 additions & 2 deletions src/lib/chips/chip-list.ts
Original file line number Diff line number Diff line change
Expand Up @@ -527,7 +527,6 @@ export class MatChipList implements MatFormFieldControl<any>, ControlValueAccess
private _isInputEmpty(element: HTMLElement): boolean {
if (element && element.nodeName.toLowerCase() === 'input') {
let input = element as HTMLInputElement;

return !input.value;
}

Expand All @@ -547,7 +546,14 @@ export class MatChipList implements MatFormFieldControl<any>, ControlValueAccess
// Shift focus to the active item. Note that we shouldn't do this in multiple
// mode, because we don't know what chip the user interacted with last.
if (correspondingChip) {
this._keyManager.setActiveItem(this.chips.toArray().indexOf(correspondingChip));
const correspondingChipIndex = this.chips.toArray().indexOf(correspondingChip);

if (isUserInput) {
this._keyManager.setActiveItem(correspondingChipIndex);
} else {
this._keyManager.updateActiveItemIndex(correspondingChipIndex);
}

}
}
}
Expand Down

0 comments on commit 8168667

Please sign in to comment.