Skip to content

feat(select): allow focusing items by typing #2907

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 8, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/cdk/a11y/activedescendant-key-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,14 @@
* found in the LICENSE file at https://angular.io/license
*/

import {ListKeyManager, CanDisable} from './list-key-manager';
import {ListKeyManager, ListKeyManagerOption} from './list-key-manager';

/**
* This is the interface for highlightable items (used by the ActiveDescendantKeyManager).
* Each item must know how to style itself as active or inactive and whether or not it is
* currently disabled.
*/
export interface Highlightable extends CanDisable {
export interface Highlightable extends ListKeyManagerOption {
setActiveStyles(): void;
setInactiveStyles(): void;
}
Expand Down
17 changes: 5 additions & 12 deletions src/cdk/a11y/focus-key-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,24 +6,18 @@
* found in the LICENSE file at https://angular.io/license
*/

import {QueryList} from '@angular/core';
import {ListKeyManager, CanDisable} from './list-key-manager';
import {ListKeyManager, ListKeyManagerOption} from './list-key-manager';

/**
* This is the interface for focusable items (used by the FocusKeyManager).
* Each item must know how to focus itself and whether or not it is currently disabled.
* Each item must know how to focus itself, whether or not it is currently disabled
* and be able to supply it's label.
*/
export interface Focusable extends CanDisable {
export interface FocusableOption extends ListKeyManagerOption {
focus(): void;
}


export class FocusKeyManager extends ListKeyManager<Focusable> {

constructor(items: QueryList<Focusable>) {
super(items);
}

export class FocusKeyManager extends ListKeyManager<FocusableOption> {
/**
* This method sets the active item to the item at the specified index.
* It also adds focuses the newly active item.
Expand All @@ -35,5 +29,4 @@ export class FocusKeyManager extends ListKeyManager<Focusable> {
this.activeItem.focus();
}
}

}
129 changes: 97 additions & 32 deletions src/cdk/a11y/list-key-manager.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,10 @@ import {first} from '../rxjs/index';


class FakeFocusable {
constructor(private _label = '') { }
disabled = false;
focus() {}
getLabel() { return this._label; }
}

class FakeHighlightable {
Expand All @@ -20,11 +22,11 @@ class FakeHighlightable {
}

class FakeQueryList<T> extends QueryList<T> {
get length() { return this.items.length; }
items: T[];
toArray() {
return this.items;
}
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); }
}


Expand All @@ -43,7 +45,7 @@ describe('Key managers', () => {
downArrow: createKeyboardEvent('keydown', DOWN_ARROW),
upArrow: createKeyboardEvent('keydown', UP_ARROW),
tab: createKeyboardEvent('keydown', TAB),
unsupported: createKeyboardEvent('keydown', 65) // corresponds to the letter "a"
unsupported: createKeyboardEvent('keydown', 192) // corresponds to the tilde character (~)
};
});

Expand All @@ -52,7 +54,11 @@ describe('Key managers', () => {
let keyManager: ListKeyManager<FakeFocusable>;

beforeEach(() => {
itemList.items = [new FakeFocusable(), new FakeFocusable(), new FakeFocusable()];
itemList.items = [
new FakeFocusable('one'),
new FakeFocusable('two'),
new FakeFocusable('three')
];
keyManager = new ListKeyManager<FakeFocusable>(itemList);

// first item is already focused
Expand Down Expand Up @@ -383,6 +389,65 @@ describe('Key managers', () => {

});

describe('typeahead mode', () => {
const debounceInterval = 300;

beforeEach(() => {
keyManager.withTypeAhead(debounceInterval);
keyManager.setActiveItem(-1);
});

it('should throw if the items do not implement the getLabel method', () => {
const invalidQueryList = new FakeQueryList();

invalidQueryList.items = [{ disabled: false }];

const invalidManager = new ListKeyManager(invalidQueryList);

expect(() => invalidManager.withTypeAhead()).toThrowError(/must implement/);
});

it('should debounce the input key presses', fakeAsync(() => {
keyManager.onKeydown(createKeyboardEvent('keydown', 79)); // types "o"
keyManager.onKeydown(createKeyboardEvent('keydown', 78)); // types "n"
keyManager.onKeydown(createKeyboardEvent('keydown', 69)); // types "e"

expect(keyManager.activeItem).not.toBe(itemList.items[0]);

tick(debounceInterval);

expect(keyManager.activeItem).toBe(itemList.items[0]);
}));

it('should focus the first item that starts with a letter', fakeAsync(() => {
keyManager.onKeydown(createKeyboardEvent('keydown', 84)); // types "t"

tick(debounceInterval);

expect(keyManager.activeItem).toBe(itemList.items[1]);
}));

it('should focus the first item that starts with sequence of letters', fakeAsync(() => {
keyManager.onKeydown(createKeyboardEvent('keydown', 84)); // types "t"
keyManager.onKeydown(createKeyboardEvent('keydown', 72)); // types "h"

tick(debounceInterval);

expect(keyManager.activeItem).toBe(itemList.items[2]);
}));

it('should cancel any pending timers if a navigation key is pressed', fakeAsync(() => {
keyManager.onKeydown(createKeyboardEvent('keydown', 84)); // types "t"
keyManager.onKeydown(createKeyboardEvent('keydown', 72)); // types "h"
keyManager.onKeydown(fakeKeyEvents.downArrow);

tick(debounceInterval);

expect(keyManager.activeItem).toBe(itemList.items[0]);
}));

});

});

describe('FocusKeyManager', () => {
Expand All @@ -400,40 +465,40 @@ describe('Key managers', () => {
spyOn(itemList.items[2], 'focus');
});

it('should focus subsequent items when down arrow is pressed', () => {
keyManager.onKeydown(fakeKeyEvents.downArrow);
it('should focus subsequent items when down arrow is pressed', () => {
keyManager.onKeydown(fakeKeyEvents.downArrow);

expect(itemList.items[0].focus).not.toHaveBeenCalled();
expect(itemList.items[1].focus).toHaveBeenCalledTimes(1);
expect(itemList.items[2].focus).not.toHaveBeenCalled();
expect(itemList.items[0].focus).not.toHaveBeenCalled();
expect(itemList.items[1].focus).toHaveBeenCalledTimes(1);
expect(itemList.items[2].focus).not.toHaveBeenCalled();

keyManager.onKeydown(fakeKeyEvents.downArrow);
expect(itemList.items[0].focus).not.toHaveBeenCalled();
expect(itemList.items[1].focus).toHaveBeenCalledTimes(1);
expect(itemList.items[2].focus).toHaveBeenCalledTimes(1);
});
keyManager.onKeydown(fakeKeyEvents.downArrow);
expect(itemList.items[0].focus).not.toHaveBeenCalled();
expect(itemList.items[1].focus).toHaveBeenCalledTimes(1);
expect(itemList.items[2].focus).toHaveBeenCalledTimes(1);
});

it('should focus previous items when up arrow is pressed', () => {
keyManager.onKeydown(fakeKeyEvents.downArrow);
it('should focus previous items when up arrow is pressed', () => {
keyManager.onKeydown(fakeKeyEvents.downArrow);

expect(itemList.items[0].focus).not.toHaveBeenCalled();
expect(itemList.items[1].focus).toHaveBeenCalledTimes(1);
expect(itemList.items[0].focus).not.toHaveBeenCalled();
expect(itemList.items[1].focus).toHaveBeenCalledTimes(1);

keyManager.onKeydown(fakeKeyEvents.upArrow);
keyManager.onKeydown(fakeKeyEvents.upArrow);

expect(itemList.items[0].focus).toHaveBeenCalledTimes(1);
expect(itemList.items[1].focus).toHaveBeenCalledTimes(1);
});
expect(itemList.items[0].focus).toHaveBeenCalledTimes(1);
expect(itemList.items[1].focus).toHaveBeenCalledTimes(1);
});

it('should allow setting the focused item without calling focus', () => {
expect(keyManager.activeItemIndex)
.toBe(0, `Expected first item of the list to be active.`);
it('should allow setting the focused item without calling focus', () => {
expect(keyManager.activeItemIndex)
.toBe(0, `Expected first item of the list to be active.`);

keyManager.updateActiveItemIndex(1);
expect(keyManager.activeItemIndex)
.toBe(1, `Expected activeItemIndex to update after calling updateActiveItemIndex().`);
expect(itemList.items[1].focus).not.toHaveBeenCalledTimes(1);
});
keyManager.updateActiveItemIndex(1);
expect(keyManager.activeItemIndex)
.toBe(1, `Expected activeItemIndex to update after calling updateActiveItemIndex().`);
expect(itemList.items[1].focus).not.toHaveBeenCalledTimes(1);
});

});

Expand Down
85 changes: 60 additions & 25 deletions src/cdk/a11y/list-key-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,42 +9,83 @@
import {QueryList} from '@angular/core';
import {Observable} from 'rxjs/Observable';
import {Subject} from 'rxjs/Subject';
import {UP_ARROW, DOWN_ARROW, TAB} from '@angular/cdk/keyboard';
import {Subscription} from 'rxjs/Subscription';
import {UP_ARROW, DOWN_ARROW, TAB, A, Z} from '@angular/cdk/keyboard';
import {RxChain, debounceTime, filter, map, doOperator} from '@angular/cdk/rxjs';

/**
* This interface is for items that can be disabled. The type passed into
* ListKeyManager must extend this interface.
* This interface is for items that can be passed to a ListKeyManager.
*/
export interface CanDisable {
export interface ListKeyManagerOption {
disabled?: boolean;
getLabel?(): string;
}

/**
* This class manages keyboard events for selectable lists. If you pass it a query list
* of items, it will set the active item correctly when arrow events occur.
*/
export class ListKeyManager<T extends CanDisable> {
private _activeItemIndex: number = -1;
export class ListKeyManager<T extends ListKeyManagerOption> {
private _activeItemIndex = -1;
private _activeItem: T;
private _tabOut = new Subject<void>();
private _wrap: boolean = false;
private _wrap = false;
private _nonNavigationKeyStream = new Subject<number>();
private _typeaheadSubscription: Subscription;

// Buffer for the letters that the user has pressed when the typeahead option is turned on.
private _pressedInputKeys: number[] = [];

constructor(private _items: QueryList<T>) { }

/**
* Turns on wrapping mode, which ensures that the active item will wrap to
* the other end of list when there are no more items in the given direction.
*
* @returns The ListKeyManager that the method was called on.
*/
withWrap(): this {
this._wrap = true;
return this;
}

/**
* Turns on typeahead mode which allows users to set the active item by typing.
* @param debounceInterval Time to wait after the last keystroke before setting the active item.
*/
withTypeAhead(debounceInterval = 200): this {
if (this._items.length && this._items.some(item => typeof item.getLabel !== 'function')) {
throw Error('ListKeyManager items in typeahead mode must implement the `getLabel` method.');
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add test for this error?


if (this._typeaheadSubscription) {
this._typeaheadSubscription.unsubscribe();
}

// Debounce the presses of non-navigational keys, collect the ones that correspond to letters
// and convert those letters back into a string. Afterwards find the first item that starts
// with that string and select it.
this._typeaheadSubscription = RxChain.from(this._nonNavigationKeyStream)
.call(filter, keyCode => keyCode >= A && keyCode <= Z)
Copy link

@andrewbents andrewbents Aug 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so, it only searches by letters? What if options contain other characters? Also, a lot of languages have more letters, so some of them don't fit in the A-Z range

Copy link
Member Author

@crisbeto crisbeto Aug 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This checks the keyboard codes, not the actual letters. E.g. 68 will correspond to d in English and д in a Bulgarian phonetic keyboard. As for the languages with more letters, nothing has come up yet, but we can always update the range. This seems to be what Closure uses as well.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Russian has 6 letters outside this range, they are on [ ] ; ' , . keys. So, they won't be handled? Also I think non-alphabetic characters should be supported as well

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about numbers? Does it work with numeric values? And when should we expect it to be release?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've submitted #6543 that should handle more or less everything. It'll still fall back to A-Z and 0-9 for browsers that don't support KeyboardEvent.key (should be a very small percentage).

.call(doOperator, keyCode => this._pressedInputKeys.push(keyCode))
.call(debounceTime, debounceInterval)
.call(filter, () => this._pressedInputKeys.length > 0)
.call(map, () => String.fromCharCode(...this._pressedInputKeys))
.subscribe(inputString => {
const items = this._items.toArray();

for (let i = 0; i < items.length; i++) {
if (items[i].getLabel!().toUpperCase().trim().indexOf(inputString) === 0) {
this.setActiveItem(i);
break;
}
}

this._pressedInputKeys = [];
});

return this;
}

/**
* Sets the active item to the item at the index specified.
*
* @param index The index of the item to be set as active.
*/
setActiveItem(index: number): void {
Expand All @@ -58,20 +99,15 @@ export class ListKeyManager<T extends CanDisable> {
*/
onKeydown(event: KeyboardEvent): void {
switch (event.keyCode) {
case DOWN_ARROW:
this.setNextItemActive();
break;
case UP_ARROW:
this.setPreviousItemActive();
break;
case TAB:
// Note that we shouldn't prevent the default action on tab.
this._tabOut.next();
return;
default:
return;
case DOWN_ARROW: this.setNextItemActive(); break;
case UP_ARROW: this.setPreviousItemActive(); break;

// Note that we return here, in order to avoid preventing
// the default action of unsupported keys.
default: this._nonNavigationKeyStream.next(event.keyCode); return;
}

this._pressedInputKeys = [];
event.preventDefault();
}

Expand Down Expand Up @@ -119,7 +155,7 @@ export class ListKeyManager<T extends CanDisable> {
* when focus is shifted off of the list.
*/
get tabOut(): Observable<void> {
return this._tabOut.asObservable();
return filter.call(this._nonNavigationKeyStream, keyCode => keyCode === TAB);
}

/**
Expand Down Expand Up @@ -173,5 +209,4 @@ export class ListKeyManager<T extends CanDisable> {
}
this.setActiveItem(index);
}

}
2 changes: 2 additions & 0 deletions src/cdk/keyboard/keycodes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,5 @@ export const TAB = 9;
export const ESCAPE = 27;
export const BACKSPACE = 8;
export const DELETE = 46;
export const A = 65;
export const Z = 90;
5 changes: 3 additions & 2 deletions src/lib/chips/chip.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import {
forwardRef,
} from '@angular/core';

import {Focusable} from '../core/a11y/focus-key-manager';
import {FocusableOption} from '../core/a11y/focus-key-manager';
import {coerceBooleanProperty} from '@angular/cdk/coercion';
import {CanColor, mixinColor} from '../core/common-behaviors/color';
import {CanDisable, mixinDisabled} from '../core/common-behaviors/disabled';
Expand Down Expand Up @@ -68,7 +68,8 @@ export class MdBasicChip { }
'(blur)': '_hasFocus = false',
}
})
export class MdChip extends _MdChipMixinBase implements Focusable, OnDestroy, CanColor, CanDisable {
export class MdChip extends _MdChipMixinBase implements FocusableOption, OnDestroy, CanColor,
CanDisable {

@ContentChild(forwardRef(() => MdChipRemove)) _chipRemove: MdChipRemove;

Expand Down
Loading