Skip to content

Commit

Permalink
fix(list-key-manager): maintain selected index when amount of items c…
Browse files Browse the repository at this point in the history
…hanges (#9164)

Currently the `activeItemIndex` and the `activeItem` in the `ListKeyManager` will stay the same if the amount of items changes. This is an issue, because they'll become out of sync and focus could potentially go to a disabled item. With the following changes the key manager will ensure that the `activeItemIndex` and the `activeItem` are in sync and point to the same item.
  • Loading branch information
crisbeto authored and jelbourn committed Jan 24, 2018
1 parent 17e36fe commit 4f65276
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 1 deletion.
14 changes: 14 additions & 0 deletions src/cdk/a11y/list-key-manager.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {ActiveDescendantKeyManager} from './activedescendant-key-manager';
import {FocusKeyManager} from './focus-key-manager';
import {ListKeyManager} from './list-key-manager';
import {FocusOrigin} from './focus-monitor';
import {Subject} from 'rxjs/Subject';


class FakeFocusable {
Expand All @@ -23,11 +24,13 @@ class FakeHighlightable {
}

class FakeQueryList<T> extends QueryList<T> {
changes = new Subject<FakeQueryList<T>>();
items: T[];
get length() { return this.items.length; }
get first() { return this.items[0]; }
toArray() { return this.items; }
some() { return this.items.some.apply(this.items, arguments); }
notifyOnChanges() { this.changes.next(this); }
}


Expand Down Expand Up @@ -71,6 +74,17 @@ describe('Key managers', () => {
spyOn(keyManager, 'setActiveItem').and.callThrough();
});

it('should maintain the active item if the amount of items changes', () => {
expect(keyManager.activeItemIndex).toBe(0);
expect(keyManager.activeItem!.getLabel()).toBe('one');

itemList.items.unshift(new FakeFocusable('zero'));
itemList.notifyOnChanges();

expect(keyManager.activeItemIndex).toBe(1);
expect(keyManager.activeItem!.getLabel()).toBe('one');
});

describe('Key events', () => {

it('should emit tabOut when the tab key is pressed', () => {
Expand Down
13 changes: 12 additions & 1 deletion src/cdk/a11y/list-key-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,18 @@ export class ListKeyManager<T extends ListKeyManagerOption> {
// Buffer for the letters that the user has pressed when the typeahead option is turned on.
private _pressedLetters: string[] = [];

constructor(private _items: QueryList<T>) { }
constructor(private _items: QueryList<T>) {
_items.changes.subscribe((newItems: QueryList<T>) => {
if (this._activeItem) {
const itemArray = newItems.toArray();
const newIndex = itemArray.indexOf(this._activeItem);

if (newIndex > -1 && newIndex !== this._activeItemIndex) {
this._activeItemIndex = newIndex;
}
}
});
}

/**
* Stream that emits any time the TAB key is pressed, so components can react
Expand Down

0 comments on commit 4f65276

Please sign in to comment.