Skip to content

Commit

Permalink
fix(list-key-manager): typehead not handling non-English input (#6463)
Browse files Browse the repository at this point in the history
Since `String.fromCharCode` only maps the key codes to their English letter counterparts, the typeahead option in the `ListKeyManager` won't work for non-English input. These changes switch to using `event.key` and falling back to using `keyCode` and `fromCharCode`.
  • Loading branch information
crisbeto authored and andrewseguin committed Aug 16, 2017
1 parent da7cb2d commit 08a6673
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 29 deletions.
31 changes: 23 additions & 8 deletions src/cdk/a11y/list-key-manager.spec.ts
Expand Up @@ -408,9 +408,9 @@ describe('Key managers', () => {
});

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"
keyManager.onKeydown(createKeyboardEvent('keydown', 79, undefined, 'o')); // types "o"
keyManager.onKeydown(createKeyboardEvent('keydown', 78, undefined, 'n')); // types "n"
keyManager.onKeydown(createKeyboardEvent('keydown', 69, undefined, 'e')); // types "e"

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

Expand All @@ -420,32 +420,47 @@ describe('Key managers', () => {
}));

it('should focus the first item that starts with a letter', fakeAsync(() => {
keyManager.onKeydown(createKeyboardEvent('keydown', 84)); // types "t"
keyManager.onKeydown(createKeyboardEvent('keydown', 84, undefined, 't')); // 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"
keyManager.onKeydown(createKeyboardEvent('keydown', 84, undefined, 't')); // types "t"
keyManager.onKeydown(createKeyboardEvent('keydown', 72, undefined, 'h')); // 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(createKeyboardEvent('keydown', 84, undefined, 't')); // types "t"
keyManager.onKeydown(createKeyboardEvent('keydown', 72, undefined, 'h')); // types "h"
keyManager.onKeydown(fakeKeyEvents.downArrow);

tick(debounceInterval);

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

it('should handle non-English input', fakeAsync(() => {
itemList.items = [
new FakeFocusable('едно'),
new FakeFocusable('две'),
new FakeFocusable('три')
];

const keyboardEvent = createKeyboardEvent('keydown', 68, undefined, 'д');

keyManager.onKeydown(keyboardEvent); // types "д"
tick(debounceInterval);

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

});

});
Expand Down
45 changes: 25 additions & 20 deletions src/cdk/a11y/list-key-manager.ts
Expand Up @@ -7,7 +7,6 @@
*/

import {QueryList} from '@angular/core';
import {Observable} from 'rxjs/Observable';
import {Subject} from 'rxjs/Subject';
import {Subscription} from 'rxjs/Subscription';
import {UP_ARROW, DOWN_ARROW, TAB, A, Z} from '@angular/cdk/keycodes';
Expand All @@ -29,14 +28,20 @@ export class ListKeyManager<T extends ListKeyManagerOption> {
private _activeItemIndex = -1;
private _activeItem: T;
private _wrap = false;
private _nonNavigationKeyStream = new Subject<number>();
private _letterKeyStream = new Subject<string>();
private _typeaheadSubscription: Subscription;

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

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

/**
* Stream that emits any time the TAB key is pressed, so components can react
* when focus is shifted off of the list.
*/
tabOut: Subject<void> = new Subject<void>();

/**
* 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.
Expand All @@ -62,12 +67,11 @@ export class ListKeyManager<T extends ListKeyManagerOption> {
// 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)
.call(doOperator, keyCode => this._pressedInputKeys.push(keyCode))
this._typeaheadSubscription = RxChain.from(this._letterKeyStream)
.call(doOperator, keyCode => this._pressedLetters.push(keyCode))
.call(debounceTime, debounceInterval)
.call(filter, () => this._pressedInputKeys.length > 0)
.call(map, () => String.fromCharCode(...this._pressedInputKeys))
.call(filter, () => this._pressedLetters.length > 0)
.call(map, () => this._pressedLetters.join(''))
.subscribe(inputString => {
const items = this._items.toArray();

Expand All @@ -78,7 +82,7 @@ export class ListKeyManager<T extends ListKeyManagerOption> {
}
}

this._pressedInputKeys = [];
this._pressedLetters = [];
});

return this;
Expand All @@ -101,13 +105,22 @@ export class ListKeyManager<T extends ListKeyManagerOption> {
switch (event.keyCode) {
case DOWN_ARROW: this.setNextItemActive(); break;
case UP_ARROW: this.setPreviousItemActive(); break;
case TAB: this.tabOut.next(); return;
default:
if (event.keyCode >= A && event.keyCode <= Z) {
// Attempt to use the `event.key` which also maps it to the user's keyboard language,
// otherwise fall back to `keyCode` and `fromCharCode` which always resolve to English.
this._letterKeyStream.next(event.key ?
event.key.toLocaleUpperCase() :
String.fromCharCode(event.keyCode));
}

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

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

Expand Down Expand Up @@ -150,14 +163,6 @@ export class ListKeyManager<T extends ListKeyManagerOption> {
this._activeItemIndex = index;
}

/**
* Observable that emits any time the TAB key is pressed, so components can react
* when focus is shifted off of the list.
*/
get tabOut(): Observable<void> {
return filter.call(this._nonNavigationKeyStream, keyCode => keyCode === TAB);
}

/**
* This method sets the active item, given a list of items and the delta between the
* currently active item and the new active item. It will calculate differently
Expand Down
3 changes: 2 additions & 1 deletion src/cdk/testing/event-objects.ts
Expand Up @@ -30,7 +30,7 @@ export function createMouseEvent(type: string, x = 0, y = 0) {
}

/** Dispatches a keydown event from an element. */
export function createKeyboardEvent(type: string, keyCode: number, target?: Element) {
export function createKeyboardEvent(type: string, keyCode: number, target?: Element, key?: string) {
let event = document.createEvent('KeyboardEvent') as any;
// Firefox does not support `initKeyboardEvent`, but supports `initKeyEvent`.
let initEventFn = (event.initKeyEvent || event.initKeyboardEvent).bind(event);
Expand All @@ -42,6 +42,7 @@ export function createKeyboardEvent(type: string, keyCode: number, target?: Elem
// See related bug https://bugs.webkit.org/show_bug.cgi?id=16735
Object.defineProperties(event, {
keyCode: { get: () => keyCode },
key: { get: () => key },
target: { get: () => target }
});

Expand Down

0 comments on commit 08a6673

Please sign in to comment.