Skip to content
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

Regression: using defaultValue shows invalid option as a valid suggestion and opens menu unnecessarily #495

Open
edwardhorsford opened this issue Jun 11, 2021 · 2 comments
Labels
🐛 bug Something isn't working the way it should (including incorrect wording in documentation)

Comments

@edwardhorsford
Copy link
Contributor

edwardhorsford commented Jun 11, 2021

Version v2.0.0 appears to have introduced a regression with default values.

If the autocomplete is rendered with a default value, when the autocomplete is focused, a menu is expanded with that value displayed - this happens regardless of if that value is valid or not.

I think these lines are what's at issue.

The regressions are:

  • No menu should be shown when focusing the field
  • The option shown is copied from the defaultValue - but this may not be a valid option

The correct behaviour is that focusing a field with a value does not show the menu. And especially if that value is invalid (such as returning to the page after a validation failure, no invalid option should be shown. The reason we don't show the menu is that if it's a correct option, there should basically only be one option - it's pointless to show. The common behaviour is to clear the input / search again - showing a menu with a single option is confusing and unhelpful.

In my example videos, the autocomplete is enhancing a select. French language is one of the available options, Franch is invalid.

Current autocomplete with invalid option:

An invalid option (that is not in the source select) is shown as a selectable option.
autocomplete-suggesting-invalid-value

Current autocomplete with valid option:

A menu is shown which is pointless - the autocomplete as originally designed intended that this not happen.
autocomplete-suggesting-unnecessary-menu

Correct behaviour in 1.6.1 and prior:

No menu is shown on focus, only when typing - the invalid option is also not shown.
autocomplete-suggesting-invalid-value-previous

This impacts our service when we show validation errors - we want to pre-fill the autocomplete with what the user typed, but we do not want it suggested:
autocomplete-invalid-suggested-option

Fixes

There's possibly some discussion to be had about whether the menu should open if there's valid options - where there's only one valid result, I think the original functionality of not opening the menu is better. If there's more than one valid result, possibly a menu would be better.

However - we definitely shouldn't be showing an invalid 'defaultValue' as a selectable thing. Users of this component should be able to put whatever string they like in defaultValue without it being suggested unless it matches one of the input items.

@edwardhorsford
Copy link
Contributor Author

My (hacky) solution over at #424 is to stop using defaultValue entirely. Instead use javascript to directly set the value of the input after the autocomplete has initialised.

@edwardhorsford
Copy link
Contributor Author

edwardhorsford commented Nov 15, 2021

Frustratingly you can't just use js to set the value of the input to an empty string '' - as in 'show all values' mode this will expand the menu and you can't then collapse it.

This is a problem when using the accessibleAutocomplete.enhanceSelectElement() mode - as if you don't provide a default value (doing so triggers the above bug), then it will set the default value to the first item from the underlying select. In our case the first item is a disabled option Please select... - we don't pass this as a value to the autocomplete though, and it shouldn't get rendered by it.

What it looks like when the kit picks up a value from the select that it shouldn't:
Screenshot 2021-11-15 at 13 23 34

My solution for now will be to remove all usages of the enhanceSelectElement() mode. We'll still enhance from selects, but we'll do it manually with the 'regular' autocomplete, not this mode. This means that we can control what we pass as defaultValue.

@vanitabarrett vanitabarrett added the 🐛 bug Something isn't working the way it should (including incorrect wording in documentation) label Jan 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working the way it should (including incorrect wording in documentation)
Projects
None yet
Development

No branches or pull requests

2 participants