From 08a66734dbf6d9b92aa6b46709e29c66f4e6bf3b Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Wed, 16 Aug 2017 22:15:13 +0200 Subject: [PATCH] fix(list-key-manager): typehead not handling non-English input (#6463) 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`. --- src/cdk/a11y/list-key-manager.spec.ts | 31 +++++++++++++----- src/cdk/a11y/list-key-manager.ts | 45 +++++++++++++++------------ src/cdk/testing/event-objects.ts | 3 +- 3 files changed, 50 insertions(+), 29 deletions(-) diff --git a/src/cdk/a11y/list-key-manager.spec.ts b/src/cdk/a11y/list-key-manager.spec.ts index 462366554ece..bba6ecf9d65d 100644 --- a/src/cdk/a11y/list-key-manager.spec.ts +++ b/src/cdk/a11y/list-key-manager.spec.ts @@ -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]); @@ -420,7 +420,7 @@ 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); @@ -428,8 +428,8 @@ describe('Key managers', () => { })); 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); @@ -437,8 +437,8 @@ describe('Key managers', () => { })); 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); @@ -446,6 +446,21 @@ describe('Key managers', () => { 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]); + })); + }); }); diff --git a/src/cdk/a11y/list-key-manager.ts b/src/cdk/a11y/list-key-manager.ts index b347585b6699..1ccbedc7ce80 100644 --- a/src/cdk/a11y/list-key-manager.ts +++ b/src/cdk/a11y/list-key-manager.ts @@ -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'; @@ -29,14 +28,20 @@ export class ListKeyManager { private _activeItemIndex = -1; private _activeItem: T; private _wrap = false; - private _nonNavigationKeyStream = new Subject(); + private _letterKeyStream = new Subject(); 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) { } + /** + * Stream that emits any time the TAB key is pressed, so components can react + * when focus is shifted off of the list. + */ + tabOut: Subject = new Subject(); + /** * 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. @@ -62,12 +67,11 @@ export class ListKeyManager { // 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(); @@ -78,7 +82,7 @@ export class ListKeyManager { } } - this._pressedInputKeys = []; + this._pressedLetters = []; }); return this; @@ -101,13 +105,22 @@ export class ListKeyManager { 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(); } @@ -150,14 +163,6 @@ export class ListKeyManager { 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 { - 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 diff --git a/src/cdk/testing/event-objects.ts b/src/cdk/testing/event-objects.ts index 6b47eb916633..1d88d7996df2 100644 --- a/src/cdk/testing/event-objects.ts +++ b/src/cdk/testing/event-objects.ts @@ -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); @@ -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 } });