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

[Autocomplete] freeSolo is not actually usable #18113

Closed
2 tasks done
dantman opened this issue Oct 31, 2019 · 12 comments · Fixed by #18285
Closed
2 tasks done

[Autocomplete] freeSolo is not actually usable #18113

dantman opened this issue Oct 31, 2019 · 12 comments · Fixed by #18285
Assignees
Labels
component: autocomplete This is the name of the generic UI component, not the React module! new feature New feature or request

Comments

@dantman
Copy link
Contributor

dantman commented Oct 31, 2019

  • The issue is present in the latest release.
  • I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

If you use an Autocomplete with freeSolo the onChange is never fired, unless you click one one of the predefined autocomplete items or clear the field. So you can't actually use it like a search field, etc... because the component using it never gets the field's value.

Additionally even if you type out one of the predefined items, onChange is not fired unless you actually click/select the autocomplete item. This is mildly strange for normal Autocomplete where it clears the text but feels completely broken when using freeSolo where the text is not cleared and you expect the value has changed.

Expected Behavior 🤔

In freeSolo mode onChange should fire on every input key press like a normal TextField does.

Additionally, outside of freeSolo Autocomplete should probably fire onChange when you type out one of the options and blur.

Steps to Reproduce 🕹

Edit Autocomplete freeSolo test
Steps:

  1. Add an <Autocomplete freeSolo /> component like one of the demos in the documentation
  2. Add a onChange and value connected to state to it
  3. Try typing in the field, the controlled value will not update but the field text will

Context 🔦

I wanted to use the Autocomplete to add a single tag filter to a videos list page. The tag can be arbitrary because I only plan to load the most common tags from the API.

Your Environment 🌎

Tech Version
Material-UI v4.5.2
Material-UI Lab v4.0.0-alpha.30
React 16.8.6
Browser Chrome
@oliviertassinari
Copy link
Member

oliviertassinari commented Oct 31, 2019

In freeSolo mode onChange should fire on every input key press like a normal TextField does.

The onChange and value props for the freeSolo are meant to account for user confirmation (click / enter). The google place demo access the input value like this:
https://github.com/mui-org/material-ui/blob/c28697d0ba4c891f826133a77e58f298dc50c4bb/docs/src/pages/components/autocomplete/GoogleMaps.js#L89-L106

We could add an onInputValueChange prop to make it easier to merge the change event with custom input implementations.

Additionally, outside of freeSolo Autocomplete should probably fire onChange when you type out one of the options and blur.

It's what the autoSelect prop is for.

@oliviertassinari oliviertassinari added component: autocomplete This is the name of the generic UI component, not the React module! new feature New feature or request labels Oct 31, 2019
@dantman
Copy link
Contributor Author

dantman commented Nov 1, 2019

The onChange and value props for the freeSolo are meant to account for user confirmation (click / enter). The google place demo access the input value like this:

This demo is broken. Using onChange on the TextField is not sufficient to keep up with the current input of the field.

I've modified that example with the debug output from my demo. You'll see that the clear button does not work correctly, it clears the field but the value the host gets from onChange is not cleared.

Edit Material demo

It's hard to do in a demo because CodeSandbox doesn't have the right origin to make the demo work with the Google API. However using the DevTools on the demo page I can also confirm that clicking one of the autocomplete options also does not update inputValue.

@oliviertassinari
Copy link
Member

@dantman Interesting, it seems that the input onChange can be used to know when to query the API for a new value and that the Autocomplete onChange to know when the user selects a new value.

What would you like to change and what problem would it solve? Thanks.

@dantman
Copy link
Contributor Author

dantman commented Nov 1, 2019

Interesting, it seems that the input onChange can be used to know when to query the API for a new value and that the Autocomplete onChange to know when the user selects a new value.

Except that's not actually true, because pressing the clear button should have the same effect as "⌘+A, Delete", it's user input and should result in a new API value just like typing.

You do make note of a valid use case. Wanting to re-fetch data/change value only for user input in the field (typing and clearing) but not when the field is updated via autoComplete or selection of auto-complete. Like with a location search where the full label and the text you'd search are completely disconnected and it would work strangely if you tried to search with the value you get back from autocomplete.

However I believe this type of situation is the minority of freeSolo use cases. For every other type of filtering, searching, etc... I can think of that you might do with freeSolo you'd want your value to keep up with whatever the current input of the field is no matter how the user did it (typing or autocompleting).

How about this, we change onChange's behaviour in freeSolo to work for the common and most React-like case (onChange is fired such that value will always match whatever is displayed in the text field, like a normal controlled input) and we add other events to cover the scenarios where you don't always want your value to be in-sync with the text field.

  • onChange: Fires on typing, clearing, autoComplete "preview", and item selection
  • onInputChange: Fires on typing and clearing, does not include the autoComplete "preview" text
  • onSelect (or onItemChange?): Only fires on item selection (via manual selection or autoSelect)

Outside of freeSolo where values outside of the options are not valid, onChange of course will work more like a <select>'s onChange and would not fire without a valid selection:

  • onChange: Only fires on item selection (via manual selection or autoSelect)
  • onInputChange: Fires on typing and clearing, does not include the autoComplete "preview" text
  • onSelect (or onItemChange?): Only fires on item selection (via manual selection or autoSelect)

This will actually be an improvement for the use-case you describe because onInputChange is properly fired when clearing the field, but onChange on the text input is not fired.

Alternatively since onChange and onSelect overlap, perhaps onChange could fire with autoComplete's "previews" as they are valid selections, but onSelect would not fire for a preview until the selection is confirmed (item selection, blur, etc...).

Then onChange can be used for live updating previews (like how onChange normally behaves in react). And onSelect can be used for when the changed needs to be "committed".

@oliviertassinari
Copy link
Member

Except that's not actually true, because pressing the clear button should have the same effect as "⌘+A, Delete", it's user input and should result in a new API value just like typing.

@dantman You are right, this even causes a bug where if you clear the input with the button and press the down arrow key, it shows the suggestions for the preview query:

  • type "a" in the input
  • click on the clear button
  • press down arrow

Expected output: no popup, actual output: popup. We need to fix that. Regarding your suggestions:

  • I strongly think that we shouldn't change the onChange behavior. I think that it should trigger when a user has manifested a strong intention to change the "selected" value.
  • I don't think that autocomplete "preview" should trigger an event. It's meant to be a different "view" of the data.
  • I like the addition of a prop like the onInputChange one you propose. Alternative wording would be onInputValueChange or onSearch.

However, to go back to the initial problem, what are you trying to implement that the current implement stops you from? I'm interested in this part:

I wanted to use the Autocomplete to add a single tag filter to a videos list page. The tag can be arbitrary because I only plan to load the most common tags from the API.

Thank you!

@dantman
Copy link
Contributor Author

dantman commented Nov 1, 2019

  • I strongly think that we shouldn't change the onChange behavior. I think that it should trigger when a user has manifested a strong intention to change the "selected" value.

Honestly in another scenario I might agree, but IMHO that doesn't really fit with the scenario we have. For better or worse in React onChange is not for commit like change, it's a live reflection of the value like input. If it weren't for that I'd agree that onChange should be the "commit" event and the live updating event should have a different name.

IMHO while Autocomplete with freeSolo={false} may be akin to a select with a search box. Autocomplete with freeSolo={true} is more equivalent to a TextField with an optional autocompletion box. So it makes sense for onChange to keep up with whatever is in the input for a controlled component.

However, to go back to the initial problem, what are you trying to implement that the current implement stops you from? I'm interested in this part:

I wanted to use the Autocomplete to add a single tag filter to a videos list page. The tag can be arbitrary because I only plan to load the most common tags from the API.

Personally I have a video list page and I'm adding a tag filter input with autocomplete of popular tags to it. There are no other filters yet (or ever, not sure) and there is no "submit". Input is debounced and live-updates the video list. I don't want to wait for the input to be "committed" because typing a tag name without selecting from the list is a valid behaviour and there are no other fields that would encourage the user to blur the input triggering a "commit" of what they typed. If I waited for "commit" not only would it add an extra delay waiting for that user interaction, there's a fair chance a user may type out a tag and then sit there confused not realizing they need to click/tab out of the input before the video list will update.

Screen Shot 2019-11-01 at 3 03 18 PM

I'm currently working around the limitations by using the following, which IMHO I consider a really ugly hack. Personally I want to fix the issues in MUI Autocomplete so I can remove the hack, but I'm also thinking about the other use cases for Autocomplete that need fixing.

<Autocomplete
  freeSolo
  options={tagsList}
  loading={tagsLoading}
  onChange={onTagInputChange}
  value={tagInputValue}
  renderInput={props => (
    <TextField
      {...props}
      label="Tag (filter)"
      variant="outlined"
      fullWidth
      inputProps={{
        ...props.inputProps,
        onChange: e => {
          onTagInputChange(e, e.target.value || null);
          props.inputProps.onChange(e);
        },
      }}
    />
  )}
/>

@oliviertassinari
Copy link
Member

oliviertassinari commented Nov 4, 2019

@dantman Awesome, thank you for sharing the use case. It sounds like you implement a "search as you type" like experience. The onChange event would probably only be useful to change the URL.
I have implemented a similar UX in the past with react-autosuggest. Eventually, we change it to be more Google search like (https://www.theverge.com/2017/7/26/16034844/google-kills-off-instant-search-for-mobile-consistency).

@kennethmervin01 in #18169 raises a similar concern, he wants to be able to control the textbox value.

In this context, what do you guys think if we add this API to the useAutocomplete.js (and Autocomplete.js). Would it solve your use cases?

/**
 * The input value.
 */
inputValue?: string;
/**
 * Callback fired when the input value changes.
 */
onInputValueChange?: (event: React.ChangeEvent<{}>, value: any) => void;

We might want to add a third reason argument in the future, but I think that we should wait to hear valid use cases for it.

@dallashuggins
Copy link

@oliviertassinari I am running into the same issue and that would solve it - thank you! Do you have any idea when this will be added? I am trying to avoid switching to a new library right now but we have tight deadlines coming up and this is required functionality, so I thought it was worth checking. :)

@oliviertassinari
Copy link
Member

@dallashuggins This issue is labeled "good first issue" as there is a clear resolution path. We tend not to work on such issues, for a matter of efficiency. What alternative did you consider? :)

@dallashuggins
Copy link

dallashuggins commented Nov 7, 2019

@oliviertassinari Super appreciate the quick response! Gotcha, understood. Originally I was going to use react-autosuggest, but I switched to Material UI once I discovered the Autosuggest component, especially since we're already using Material UI for certain components. Other than an issue with the Mui-focused class name not being added to the TextField component used in renderInput, it's worked great. I have already implemented the Autosuggest component, and found a fix for the onFocus issue, and really want to find a solution for this if at all possible. Our use-case is users need to be able to select a lender from a long list of lenders (hence the autofill) and also add a new lender name if a lender is not found. Both names and IDs are valid to send in the API request we're sending on form submission.

@oliviertassinari oliviertassinari removed the good first issue Great for first contributions. Enable to learn the contribution process. label Nov 9, 2019
@shchalel
Copy link

shchalel commented May 6, 2022

I solved this by using
onInputChange={(e, data, id) => handleInputChange(e, data, interruption.id)} instead of OnChange.

On Input seems to fire both after key press and after selecting an option from the List.

const handleInputChange = (e: any, data: any) => {
setDescription(data);
return data;
};

<Autocomplete
id="description"
disabled={loading}
freeSolo
autoSelect
value={description}
onInputChange={(e, data) => handleInputChange(e, data)}
options={options.map((option: interruptionDefinitionModel | undefined) => option?.description)}
// onChange={handleDescriptionChange}
renderInput={(params) => (
<TextField {...params} fullWidth margin="normal" error={!!errors.description}/>
)}
/>

@fadil9872
Copy link

I have an api but how can I use autocomplete with my api and the data changes every time I type something

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: autocomplete This is the name of the generic UI component, not the React module! new feature New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants