SelectPanel2: Improve keyboard navigation from search input #4199
Conversation
🦋 Changeset detectedLatest commit: bcc4bcf The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
size-limit report 📦
|
There was a problem hiding this comment.
Just wanted to ask a broader clarification question before reviewing, what kind of role/semantics are we looking for between the input and the listbox? At first glance, this seems like a case where role="combobox" may be appropriate but wanted to check-in to see since the intent here is to shift the focus to the list box.
Interesting! I can see how that might work. Until now, we've looked at it as a listbox with dynamic content 🤔 cc @ichelsea for clarification though! 
|
|
I'm like 90% sure ARIA in HTML: combobox role. Most comboboxes are associated with a listbox that popups up and is not appearing by default. Also in our case, the list can be accessed independently of the search input where in combobox, the two are inherently connected. The selection of a combobox is typically displayed in the text input as well, which we aren't doing. Multi-select behavior, if it's even supported, isn't expected from assistive technology. |
|
@ichelsea Thank you! That's very helpful @joshblack Back for your review! |
There was a problem hiding this comment.
I was testing it on different stories and the focus doesn't move to the list on arrow down when there are sections in the list. Is that intentional or do we want the same focus model when there are sections as well?
select-panel-arrow-down.mp4
A selectpanel pop up is open and displays two sections with headings and options in each. Pressing the arrow down when the focus is on the search input doesn't do anything.
That's not intentional at all! You've found a bug with groups :D Amazing catch, fixed now! |
siddharthkp
requested review from
joshblack
and removed request for
joshblack
| @@ -161,6 +163,14 @@ const Panel: React.FC<SelectPanelProps> = ({ | |||
| [internalOpen], | |||
| ) | |||
|
|
|||
| // used in SelectPanel.SearchInput | |||
| const moveFocusToList = () => { | |||
| const selector = 'ul[role=listbox] li:not([role=none])' | |||
There was a problem hiding this comment.
The selector looks a bit too fragile, makes me a bit nervous 😬 but the query makes a lot of sense. Can we add tests/stories to make sure we always get the intended first element? (It is not a blocker, feel free to merge the PR)
There was a problem hiding this comment.
Will take it up in a new PR!
* add moveFocusToList * Create fresh-parents-kiss.md * make selector ignore groups
* add moveFocusToList * Create fresh-parents-kiss.md * make selector ignore groups
When the focus is on search input,
When the focus is in the list,
selectpanel-input-keydown.mov