-
Notifications
You must be signed in to change notification settings - Fork 6.8k
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
feat(select): allow focusing items by typing #2907
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,42 +9,77 @@ | |
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 ListKeyManagerItem { | ||
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 ListKeyManagerItem> { | ||
private _activeItemIndex = -1; | ||
private _activeItem: T; | ||
private _tabOut = new Subject<void>(); | ||
private _wrap: boolean = false; | ||
private _wrap = false; | ||
private _pressedInputKeys: number[] = []; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add description for |
||
private _nonNavigationKeyStream = new Subject<number>(); | ||
private _typeaheadSubscription: Subscription; | ||
|
||
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.'); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add test for this error? |
||
|
||
if (this._typeaheadSubscription) { | ||
this._typeaheadSubscription.unsubscribe(); | ||
} | ||
|
||
this._typeaheadSubscription = RxChain.from(this._nonNavigationKeyStream) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would be good to add a description of how you're using |
||
.call(filter, keyCode => keyCode >= A && keyCode <= Z) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
.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 activeItemIndex = this._items.toArray().findIndex(item => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IE11 doesn't have There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It gets added by There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
return item.getLabel!().toUpperCase().trim().startsWith(inputString); | ||
}); | ||
|
||
if (activeItemIndex > -1) { | ||
this.setActiveItem(activeItemIndex); | ||
} | ||
|
||
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 { | ||
|
@@ -58,20 +93,12 @@ 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; | ||
default: this._nonNavigationKeyStream.next(event.keyCode); return; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add back comment about using |
||
} | ||
|
||
this._pressedInputKeys = []; | ||
event.preventDefault(); | ||
} | ||
|
||
|
@@ -119,7 +146,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); | ||
} | ||
|
||
/** | ||
|
@@ -173,5 +200,4 @@ export class ListKeyManager<T extends CanDisable> { | |
} | ||
this.setActiveItem(index); | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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, ListKeyManagerItem} 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 Focusable extends ListKeyManagerItem { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe make this something like |
||
focus(): void; | ||
} | ||
|
||
|
||
export class FocusKeyManager extends ListKeyManager<Focusable> { | ||
|
||
constructor(items: QueryList<Focusable>) { | ||
super(items); | ||
} | ||
|
||
/** | ||
* This method sets the active item to the item at the specified index. | ||
* It also adds focuses the newly active item. | ||
|
@@ -35,5 +29,4 @@ export class FocusKeyManager extends ListKeyManager<Focusable> { | |
this.activeItem.focus(); | ||
} | ||
} | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe
Item
->Option
? I think every time we use this it's for thelistbox
+option
combo