From 4988d09088b12dd975accd47987de1f4a378a31e Mon Sep 17 00:00:00 2001 From: crisbeto Date: Mon, 25 Nov 2019 21:33:02 +0100 Subject: [PATCH] fix(select): don't handle open key presses while the user is typing We support typing into a select to skip to an item, as well as pressing space to open the select, however if the user is typing something that has a space in it, the select will open and interrupt the user. These changes add some logic so that we don't trigger the panel while the user is typing. Fixes #17774. --- .../a11y/key-manager/list-key-manager.spec.ts | 12 +++++++++ src/cdk/a11y/key-manager/list-key-manager.ts | 7 +++++- src/material/select/select.spec.ts | 25 +++++++++++++++++++ src/material/select/select.ts | 12 ++++++--- tools/public_api_guard/cdk/a11y.d.ts | 1 + 5 files changed, 52 insertions(+), 5 deletions(-) diff --git a/src/cdk/a11y/key-manager/list-key-manager.spec.ts b/src/cdk/a11y/key-manager/list-key-manager.spec.ts index 5d124a65a0ee..3915cdc4794d 100644 --- a/src/cdk/a11y/key-manager/list-key-manager.spec.ts +++ b/src/cdk/a11y/key-manager/list-key-manager.spec.ts @@ -818,6 +818,18 @@ describe('Key managers', () => { expect(keyManager.activeItem).toBe(itemList.items[1]); })); + it('should expose whether the user is currently typing', fakeAsync(() => { + expect(keyManager.isTyping()).toBe(false); + + keyManager.onKeydown(createKeyboardEvent('keydown', 79, 'o')); // types "o" + + expect(keyManager.isTyping()).toBe(true); + + tick(debounceInterval); + + expect(keyManager.isTyping()).toBe(false); + })); + }); }); diff --git a/src/cdk/a11y/key-manager/list-key-manager.ts b/src/cdk/a11y/key-manager/list-key-manager.ts index cc4335c5abc4..b7440157fb76 100644 --- a/src/cdk/a11y/key-manager/list-key-manager.ts +++ b/src/cdk/a11y/key-manager/list-key-manager.ts @@ -147,7 +147,7 @@ export class ListKeyManager { // and convert those letters back into a string. Afterwards find the first item that starts // with that string and select it. this._typeaheadSubscription = this._letterKeyStream.pipe( - tap(keyCode => this._pressedLetters.push(keyCode)), + tap(letter => this._pressedLetters.push(letter)), debounceTime(debounceInterval), filter(() => this._pressedLetters.length > 0), map(() => this._pressedLetters.join('')) @@ -274,6 +274,11 @@ export class ListKeyManager { return this._activeItem; } + /** Gets whether the user is currently typing into the manager using the typeahead feature. */ + isTyping(): boolean { + return this._pressedLetters.length > 0; + } + /** Sets the active item to the first enabled item in the list. */ setFirstItemActive(): void { this._setActiveItemByIndex(0, 1); diff --git a/src/material/select/select.spec.ts b/src/material/select/select.spec.ts index d1c749f8dd3e..63e752ad5d0d 100644 --- a/src/material/select/select.spec.ts +++ b/src/material/select/select.spec.ts @@ -511,6 +511,31 @@ describe('MatSelect', () => { 'Expected value from sixth option to have been set on the model.'); })); + it('should not open the select when pressing space while typing', fakeAsync(() => { + const selectInstance = fixture.componentInstance.select; + + fixture.componentInstance.typeaheadDebounceInterval = DEFAULT_TYPEAHEAD_DEBOUNCE_INTERVAL; + fixture.detectChanges(); + + expect(selectInstance.panelOpen).toBe(false, 'Expected select to be closed on init.'); + + dispatchEvent(select, createKeyboardEvent('keydown', 80, 'p')); + tick(DEFAULT_TYPEAHEAD_DEBOUNCE_INTERVAL / 2); + fixture.detectChanges(); + + dispatchKeyboardEvent(select, 'keydown', SPACE); + fixture.detectChanges(); + + expect(selectInstance.panelOpen).toBe(false, + 'Expected select to remain closed after space was pressed.'); + + tick(DEFAULT_TYPEAHEAD_DEBOUNCE_INTERVAL / 2); + fixture.detectChanges(); + + expect(selectInstance.panelOpen).toBe(false, + 'Expected select to be closed when the timer runs out.'); + })); + it('should be able to customize the typeahead debounce interval', fakeAsync(() => { const formControl = fixture.componentInstance.control; const options = fixture.componentInstance.options.toArray(); diff --git a/src/material/select/select.ts b/src/material/select/select.ts index 5b6b8c8b1ae4..7146732731e6 100644 --- a/src/material/select/select.ts +++ b/src/material/select/select.ts @@ -721,7 +721,8 @@ export class MatSelect extends _MatSelectMixinBase implements AfterContentInit, const manager = this._keyManager; // Open the select on ALT + arrow key to match the native event.preventDefault(); this.close(); - } else if ((keyCode === ENTER || keyCode === SPACE) && manager.activeItem && + // Don't do anything in this case if the user is typing, + // because the typing sequence can include the space key. + } else if (!isTyping && (keyCode === ENTER || keyCode === SPACE) && manager.activeItem && !hasModifierKey(event)) { event.preventDefault(); manager.activeItem._selectViaInteraction(); - } else if (this._multiple && keyCode === A && event.ctrlKey) { + } else if (!isTyping && this._multiple && keyCode === A && event.ctrlKey) { event.preventDefault(); const hasDeselectedOptions = this.options.some(opt => !opt.disabled && !opt.selected); diff --git a/tools/public_api_guard/cdk/a11y.d.ts b/tools/public_api_guard/cdk/a11y.d.ts index 642fb1b3fe71..effd635d5608 100644 --- a/tools/public_api_guard/cdk/a11y.d.ts +++ b/tools/public_api_guard/cdk/a11y.d.ts @@ -144,6 +144,7 @@ export declare class ListKeyManager { change: Subject; tabOut: Subject; constructor(_items: QueryList | T[]); + isTyping(): boolean; onKeydown(event: KeyboardEvent): void; setActiveItem(index: number): void; setActiveItem(item: T): void;