accessible-autocomplete
accessible-autocomplete copied to clipboard
Preventing default when you return on the input field with autoselect: false
Using the autoselect: false setting prevents a form from submitting when hitting return while the autocomplete list is visible.
Note: this is when focus is on the input
field.
As far as I understand it, the problem is in this code:
handleEnter (event) {
if (this.state.menuOpen) {
event.preventDefault()
const hasSelectedOption = this.state.selected >= 0
if (hasSelectedOption) {
this.handleOptionClick(event, this.state.selected)
}
}
}
When this.state.menuOpen
is true ("the autocomplete list is visible") but this.state.selected >= 0
is false (you haven't selected anything), it calls event.preventDefault()
which prevents the form from being submitted (which I guess is what the title of this issue implies.)
This is somewhat related to #205: If you do select an option first this.state.selected
is set to something, and then if you backspace a couple of times to get a shorter query and then hit return, the form is submitted, but not with the visible query, but instead with the one you selected in the list of options. Maybe that should be a separate issue, I don't know.
All this with autoselect: false
.
@peterjaric That was what I found as well. Here's a modified version which allows arbitrary text input:
handleEnter (event) {
const hasSelectedOption = this.state.selected >= 0
if (this.state.menuOpen && hasSelectedOption) {
event.preventDefault()
this.handleOptionClick(event, this.state.selected)
}
}
I had been under the assumption that disallowing arbitrary text input was the desired behavior for this plugin. But if others are looking for this as well, I assume further mods would be needed to play nice with features I'm not using. Perhaps there could be a check added to ensure that !this.hasAutoselect()
, for example, or maybe this behavior would have to be enabled via some new setting (for backwards compatibility) like allowAny
.
Thanks @kwaves! @tvararu, what do you think about @kwaves' suggestion(s), is this something that could be incorporated into the code?
I'll tag in @hannalaakso and @nickcolley too, since you have been more active as of late, I hope that's ok?
If you would like this change (or any of the alternatives given above) to be added, I can add any necessary tests, both in code and manual testing with a screen reader.
Hi @kwaves thanks so much for picking this up!
Could you say a little bit more about the user need for making this change? If autoselect: false
and we don’t prevent default when user hasn’t selected an option, then whatever user types in the input will be submitted. Here’s a screen grab (this is using your fork @kwaves), as you can see in the query string the user input which wasn't on the list was submitted.
This could be the desired behaviour depending on use case but the user could also make a mistake and press enter unintentionally. But perhaps we could say it’s the job of the server to look at the user input and return an error if appropriate: for instance if user typed in something that wasn’t on the list, we could either retain what the user submitted and move on, or show user an error message.
But it would be good to understand what the problem is that we’re trying to solve. @kwaves have you seen users needing to submit things that are not on the list?
Also, have you tested your fork with the enhanced select? When I tested this with enhanced select and autoselect: false
the page refreshes but it looks like the user input is not submitted. Sorry I didn’t have time to dig into it but it’s worth checking that the enhanced select would work correctly with this change.
I'm not @kwaves, but I'll chip in with our use case. We're using this component for our search field. The options are fetched from a suggestion endpoint on our search server, but that list is not a complete list of completions as we have tens of thousands of web pages indexed. So the user might type something that gives a lot of suggestions, but not the one she is looking for. In our case we match the prefix of the string, so the input might be "Peter" and the matches for example "Peter Andersson" and "Peter Smith", but none of those is the Peter she is looking for, so she hits Enter - and nothing happens.
If this change breaks another feature, maybe it could be optional, as @kwaves suggested? And include some documentation about it being incompatible with the enhanced select feature?
Hi @hannalaakso!
Like @peterjaric, I'm seeing this for AJAX-powered suggestions within a search field. (So accessible-autocomplete's source
option is set to a function, as I assume he's doing as well.)
Unlike @peterjaric (I think), I usually find myself using this approach to return a single, very long list of keywords rather than pages. And in that case, the reason for AJAX is not query efficiency or dynamic responses (the list doesn't change based on search text), but frontend performance: it means I can defer the loading of a potentially huge list of terms until the user has opened the search tool.
Maybe I'm wrong, but I feel like enhanced select would fundamentally incompatible with a search field use-case. Seems like a progressively-enhanced search field would just be a search field with no suggestions. If anything there could be an enhanceSearchElement/Field/Input/Whatever
function–the search field is rendered server-side, then gets hidden and re-rendered with JS. (Though admittedly that possibility sounds like it would take a lot more dev time than I'm capable of committing to in the near future.)
I'm considering using something like this, and will create a PR if I don't get a "no" here. Maybe the option could be called something else.
handleEnter (event) {
if (this.state.menuOpen) {
// If not using autoselect and not using enhanceSelectElement, check if the current
// value can be submitted without selecting an option from the open menu.
const allowAnyInput = !this.props.autoselect && !this.props.selectElement && this.props.experimentalAllowAnyInput
const hasSelectedOption = this.state.selected >= 0
if (!allowAnyInput || hasSelectedOption) {
event.preventDefault()
}
if (hasSelectedOption) {
this.handleOptionClick(event, this.state.selected)
}
}
}
Docs:
#### `experimentalAllowAnyInput` (default: `false`)
Type: `Boolean`
The autocomplete will submit the entered value even if does not match a value in the list of suggestions when pressing Enter. This option has no effect if `autoselect` is `true` or if `enhanceSelectElement` is used.
Hi @peterjaric thanks for offering to pick this up. I've assigned your proposal for our team to discuss at our next triage session, this should be next week. Thanks for your patience!
Thanks so much for offering to contribute this work @peterjaric 🙌
We’re prioritising bug fixes over new feature development for the accessible autocomplete at the moment. Since new features add complexity to the component, they increase the maintenance burden and the number of support requests raised. For this reason we won’t be able to accept and support your contribution at this time.
Hi @hannalaakso, I will fork it and if you are interested at a later date, I will make a PR then instead!
Thanks for your work on this great library.
Finally I've had some time for this, and I've added the implementation above to the fork at https://github.com/uppsala-university/accessible-autocomplete. Tests for the new feature are lacking, so it's not ready for a PR, but if and when you're interested in this functionality, just ping me here and I'll see to it, if I haven't fixed it in the meantime.
Hi again @hannalaakso, almost one year later. Are you interested in a PR of this feature at this point in time?
Would like to see that happen! Also encountering this bug over here.
Hi @peterjaric ,
Although the GOV.UK Design System team maintains the accessible autocomplete as a standalone component, we’re only able to put in minimal work to support it.
You can read about our plans to maintain this component.
You can also read more about the types of support we can provide.