-
Notifications
You must be signed in to change notification settings - Fork 1.5k
fix(select): Select component keyboard type-ahead #10006
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
base: main
Are you sure you want to change the base?
Changes from all commits
ee5846b
f422165
0595fd9
f5fe02e
49a56ea
db10874
f2edf21
9bc3aa6
b924c37
763a0f9
196509f
3750344
7e76e34
4e1ee69
7315623
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 |
|---|---|---|
|
|
@@ -370,7 +370,7 @@ export class ListKeyboardDelegate<T> implements KeyboardDelegate { | |
| } | ||
|
|
||
| let collection = this.collection; | ||
| let key = fromKey || this.getFirstKey(); | ||
| let key = fromKey != null ? this.getNextKey(fromKey) : this.getFirstKey(); | ||
|
Member
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 think it'd be better if we can handle this in
Author
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. Good catch. This affects 4 keyboard delegates + 2 callers (~9 files total). Centralizing in useTypeSelect eliminates duplication is fine. This could affect lot of files. It's fine to you? |
||
| while (key != null) { | ||
| let item = collection.getItem(key); | ||
| if (!item) { | ||
|
|
||
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.
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.
Added