Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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: Add Autocomplete wrapper for Menu and ListBox #7181
base: main
Are you sure you want to change the base?
feat: Add Autocomplete wrapper for Menu and ListBox #7181
Changes from 40 commits
c33ac96
a185473
e6fa7c1
61aab83
66fecc8
779309b
78de7f8
e0f098a
5914d11
12480fd
ad8942a
70c2d0f
9812c3c
8478937
3fbd35f
e15d999
368a677
82b3120
423b73c
6c498f1
fe5bfbe
477ca7f
9a58613
574bd67
ae7a00f
7b11c5d
04e8777
79c9064
b20b626
28afe21
2d0a15f
c87a507
682357f
3871e7b
e3c3c35
04d9a8b
0757832
fd709a2
9eede0c
6bd5270
150bd7e
c13544b
0d68f68
d8c5259
00f67ad
1a26d79
435c6dc
df03339
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Any chance this can be done in an event (e.g. onChange) rather than an effect? Seems like that would be more explicit about the intent (the user typed something, not a result of some external state update).
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.
when I changed this to
onChange
in an attempt to have the input value be managed at the input level instead, I ran into the problem wherefocusFirstItem
was firing before the collection filtered itself in response and thus was getting the incorrect activedescendant id. I'll try this again now that the input text is managed at the top level againThere 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.
yep, if I use
onChange
here thefocusFirstItem
happens before the collection is filtered in response to the change in input value.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.
wonder if it might make sense to change focus on hover by default for the wrapped collection
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.
a bit strange to return refs from a hook, but I guess in this case we need a callback ref so not sure how else we'd handle it...