Add ComboboxControl and story #25442
Conversation
youknowriad
added
Needs Design Feedback
youknowriad
requested review from
adamsilverstein,
afercia and
enriquesanchez
|
Size Change: +2.93 kB (0%) Total Size: 1.17 MB
ℹ️ View Unchanged
|
|
@youknowriad Nice! I'll give this a test. Related is #25420 where I try using the FormTokenField for the Author selector. Working on that I was thinking we might go this direction. |
| suggestions={ availableContinents } | ||
| onChange={ ( tokens ) => setSelectedContinent( tokens ) } | ||
| onInputChange={ searchContinents } | ||
| placeholder="Type a continent" |
There was a problem hiding this comment.
I believe it's fine here, it's just a story.
|
Overall this worked really well in my testing, nice work! A couple of questions/comments:
Steps to reproduce:
|
this was inherited as is from FormTokenField so I'll have to check.
No it doesn't support a loading state, it's just like the "tags" input available today, it updates the suggestions when ready. I can see how some sense of "progress/loading" can be useful. |
|
@youknowriad I'm getting a
|
|
@enriquesanchez I can't reproduce your error 🤔. Do you have |
youknowriad
force-pushed
the
try/autocomplete-combobox
branch
from
594d41f to
13c23be
Compare
There was a problem hiding this comment.
Great work @youknowriad!
Tested it quickly with VoiceOver and NVDA. Aside from the points I mention below, which are not specific to screen readers, it's working well on VoiceOver. On NVDA, though, I noticed that "x results found" is announced twice the first time the combo box popup opens. Also, the combo box popup is announced just as list because it doesn't have an aria-label attribute.
Besides that, I found these interactions a bit weird. Not sure if they're expected or not:
-
Having to type two characters to see the suggestions is kind of annoying. I would expect it to show suggestions right away when I click on the input. It seems to be intentional though, so maybe I'm missing some context.
-
When you navigate to an option, there's no way to go back to the state where no option is selected. Usually, pressing ArrowUp on the first suggestion would unselect it so only the input would have focus. Typing additional characters on the input should also unselect any option.
-
I couldn't find a way to clear the input value.
-
Weird bug: type "oced", then delete "d". The popup will open, close and open again.
-
When I select an option, I expect the popup to close. But it keeps open.
-
When you have multiple items and the popup has a scrollbar, moving through suggestions don't affect scroll position. But it looks like it's a simple fix (just passing a prop to
SuggestionsList).
FYI, there's a Combobox component in the works on Reakit. Here's an example: https://codesandbox.io/s/reakit-controlled-combobox-o1457
It's not quite ready yet. And it's still not optimized for big lists, but I've been exploring solutions for it, and it looks quite promising.
I'm currently testing with the Block Inserter.
|
Thanks @youknowriad, also for the ping in the Slack accessibility channel. I tested the story extensively on:
It works as expected, meaning that the behavior and interaction aren't different from the existing Tags combobox. As mentioned in #25267 (comment) the advantage of this implementation is that it sticks to the ARIA 1.0 combobox pattern. Downshift implements the ARIA 1.1 combobox pattern, which is different from the ARIA 1.0 one. The ARIA 1.1 pattern is buggy and undergoing revision. It doesn't work with Safari + VoiceOver. I guess one more plus of this implementation would be reducing the usage of (maybe entirely removing?) Downshift. That said, I noticed some of the things reported also by @diegohaz but they happen also on the existing Tags combobox so they're unrelated to this exploration. Specifically:
Yes, twice with NVDA and multiple times with JAWS 2019. Maybe the aria-live message is sent multiple times or the component does a full re-render. Regarding other considerations:
I don't think screen readers would ever announce the aria-label of the listbox because the real focus stays on the input field and it's not possible to really navigate to the listbox. However, if we want to be strict and follow the ARIA Authoring Practices examples, it should preferably use an
I don't think we would want to hammer the REST API for just one typed character. Also, I'm not sure it would be a good user experience. In core, the de facto standard for many "live search" fields is a minimum of two characters.
I'm not sure that's the expected interaction described in the ARIA Authoring Practices. Arrow Up and Down should loop through the suggestions. To clear the input field users can just delete what they entered. Optionally the delete key can be used to clear the field in one go, see:
However, I wouldn't recommend to implement this optional behavior. IMHO it's best to keep things closer to a more native behavior.
Not sure I understand this point. When typing additional characters and there's no match the suggestions listbox disappears, as expected.
Hm, I just used the delete key and I think that the expected behavior for all input fields? Also selecting the entered text and cutting works. Btw, all these points are unrelated to this exploration. If there are things to fix (most notably the multiple announcement of the search results) these should be addressed separately. |
In my tests, NVDA does announce the listbox label when the virtual focus is moved onto the suggestions for the first time. VoiceOver announces it if you navigate using VO + ArrowRight.
I see! I didn't assume that suggestions would be populated by a REST API as it seems to be a very generic component. The way I was expecting it was something similar to the Google Search combo box, which should be familiar to most parts of the internet. That's also how the WAI-ARIA example works. For network requests, I think it would be better to rely on a sort of debounce method instead.
In the example, it does loop through suggestions without focusing on the combobox. But the pattern describes it differently:
I mean this behavior:
The WAI-ARIA pattern also covers this:
That's also how I've seen this working on other sites.
Looks like the problem happens only when you already have selected an item.
I'm sorry! I wasn't aware of the context on this PR, so I just tested everything related to |
Thanks @youknowriad! I think that was my issue 😃 Tested on VoiceOver and works as expected most of the time. The component feels a bit buggy but I assume that's because it's still a WIP?
It'd be great to be able to test with a larger set of results to get a better feel of the interaction with more results. |
|
@haz I think I understand the issues you raise about the value reset behavior, it's fixed for me in the last commit. |
I agree with this as well, I feel it shouldn't be the responsibility of the component itself but more the caller. |
|
Thanks for the reviews everyone, I believe some of the issues are related to race conditions, refactoring the component to use hooks should make these easier to reason about and might solve some of these. I'm happy to see that in general, this is behavior properly in terms of a11y. I'll start the refactoring and polish and I'd like to leave the existing issues that are common with tags input to a separate PR (I'll create an issue for these later). |
I'm not sure why your NVDA is in "virtual focus" mode because as soon as you land on the input field it should switch to "focus mode". In focus mode, you're only editing the field so you can't navigate to the listbox.
@diegohaz I think this comes from the ARIA Authoring Practices 1.2, which are still a "Working Draft". As said above, the ARIA 1.2 combobox pattern is undergoing revision and we should not take this pattern into consideration. The ARIA Authoring Practices 1.2 reflect the ARIA 1.2 spec. Instead, we should stick to the markup and interaction of the 1.0 pattern. Same applies for some of your following points, that I think refers to 1.2 The whole purpose of this PR is to stick to the ARIA 1.0 pattern so it's important to pick up the correct specification and refer to the correct Authoring Practices details and examples. |
|
@diegohaz I noticed your emoji on my comment, is my fix incorrect? (I'm talking about the fact to type extra characters or empty the field to unset the value) |
|
@youknowriad It's not me! I think that's because you mentioned the wrong Haz. 😅 |
|
Sorry @haz wrong ping :P |
|
Aye, I haven't done WP hacking in many years now. Carry on... ;) |
NVDA does switch to focus mode. By "virtual focus" (which is the term MDN uses), I mean "visual focus" or the focus that is defined by the I can record a GIF later, but the reproduction is pretty simple. I've upgraded NVDA recently, so that might be the reason.
As far as I know, the problems around the combobox 1.1 patterns are related to the document structure, not the recommended keyboard interactions. The combobox 1.0 pattern has also structural problems because of the use of Both Authoring Practices 1.1 and 1.2 recommend the same interactions regarding ArrowUp and ArrowDown. I couldn't find an equivalent document for 1.0 aside from the examples, which just describe how the examples work. This is not a big deal for one-dimensional comboboxes as we should be able to move the caret on the input using horizontal arrow keys. But this becomes a bigger problem on tree/grid comboboxes where horizontal arrow keys move the visual focus within the popup. Without a clear way to return visual focus to the input, users can't move the caret. It's important to standardize as much as possible the interaction model across different types of comboboxes so we don't confuse users. That's why I recommend we follow 1.1 and 1.2 recommendations here.
The only other reference I made to the Authoring Practices are related to printable characters. It's still valid if you consider 1.0. Pinging @jeryj who has done a lot of research on this topic. |
It is possible, however the point is that we should not look at 1.2 as the combobox pattern is undergoing substantial revision. After some more digging, it appears there's something not well described in the ARIA Authoring Practices. On one hand, the expected interaction for the listbox https://www.w3.org/TR/wai-aria-practices-1.1/#listbox-popup-keyboard-interaction uses the words "... either ... or ..." thus suggesting that authors can choose between one of the following two options:
But then, in the examples, there's one more pattern described 🎉 and it's the "looping" behavior: https://www.w3.org/TR/wai-aria-practices-1.1/examples/combobox/aria1.1pattern/listbox-combo.html "Listbox Popup" section Down Arrow
Up Arrow
Home
End
So the W3C could improve clarity but there are actually 3 patterns that are valid. The "wrapping" (looping) behavior is valid and, to me, is the most intuitive one. Since this is already implemented, I don't see a good reason to change it. |
|
@youknowriad Tested again with NVDA, and now also with JAWS. I can confirm that the following issues have been fixed with the latest commits:
Regarding the use of the |
|
yeah, I was surprised too, I saw it used on PRs but apparently it was not used anywhere. |
youknowriad
requested review from
ajitbohra and
chrisvanpatten
as code owners
youknowriad
changed the title
|
I think we're reaching a good state for initial merge. WDYT. I'd like to move forward and use this in PostAuthor and ParentPage |
|
This is so much better, @youknowriad 👍 Tested it again and all the buginness and issues with Up and Down I found before are gone. I just came across one issue: When selecting an option from the results, VoiceOver announces "1 result found", as if its performing a search with the selected option:
Is this expected? I understand this is doing a live search and it's why this is happening, but it threw me off a little. |
|
@enriquesanchez I think it's fixed with a few other tweaks too. |
youknowriad
added
the
[Type] New API
|
@youknowriad Excellent work here! I will work on using this for the author/parent page selectors. |
This PR tries to add a Combobox/Autocomplete component to be used for PostAuthor/PageParent (selects where the number of entries can be very high). The idea is to replace the Downshift based one ultimately.
This component is just an adaptation of the existing FormTokenField component for a single value selection.
npm run storybook:devThanks for your help.