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
(WIP) Autocomplete with menu #7181
base: main
Are you sure you want to change the base?
(WIP) Autocomplete with menu #7181
Changes from 17 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
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.
At the moment the keyboard event dispatch handling happens in Menu directly, but it might be good to have those in a hook somewhere. Its a bit tough since we don't have access to the wrapped collection and collection ref at the top level, but potentially could just have the callbackRef expect to receive the collection and collection ref instead of just the id, open to opinions here
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.
something to note with the announcements, hopefully this won't be a problem as browsers/screenreaders begin to handle announcing of aria-activedescendant properly
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.
Is this weird also? Focus on hover happens in the menu (and future other wrapped collections) and thus needs to communicate this change upwards. This felt better that having the menu hooks accept
setFocusedNodeId