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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Autocomplete] Support values other than raw options #23708

Open
1 task done
dantman opened this issue Nov 25, 2020 · 33 comments
Open
1 task done

[Autocomplete] Support values other than raw options #23708

dantman opened this issue Nov 25, 2020 · 33 comments
Labels
component: autocomplete This is the name of the generic UI component, not the React module! new feature New feature or request waiting for 馃憤 Waiting for upvotes

Comments

@dantman
Copy link
Contributor

dantman commented Nov 25, 2020

  • I have searched the issues of this repository and believe that this is not a duplicate.

Summary 馃挕

Right now (at least if you use multiple) Autocomplete/useAutocomplete the value that comes back in onChange is always the raw option data. i.e. If you use options=[{value: 'a', label: 'A'}, {value: 'b', label: 'B'}] and select the "A" option then value will be [{value: 'a', label: 'A'}].

Ideally it would be possible to provide multiple=true options=[{value: 'a', label: 'A'}, {value: 'b', label: 'B'}] and get back values like ['a']. Autocomplete does feel like this is intended to be supported, because you can provide a getOptionSelected and it's used to identify selected options instead of doing any comparisons on the option itself. However when it comes to adding an item to a multiple array, all useAutocomplete does is newValue.push(option);. There is no way to control what part of option is actually used as the value.

I think a getOptionValue(option) function would fit the current options implementation the most.

Examples 馃寛

<Autocomplete
  multiple
  getOptionSelected={(option, value) => option.value === value}
  getOptionValue={(option) => option.value}
  options={[{value: 'US', label: 'United States', {value: 'CA', label: 'Canada'}]}
  />

Motivation 馃敠

MUI's <Select> easily supports this. It uses children instead, so all it takes is options.map(item => <MenuItem key={item.value} value={item.value}>{item.value}</MenuItem>) to do this in Select and pretty much every project I've worked on uses Select this way. It would be strongly preferred if it were easy to use useAutocomplete the same way.

@dantman dantman added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Nov 25, 2020
@oliviertassinari oliviertassinari added component: autocomplete This is the name of the generic UI component, not the React module! docs Improvements or additions to the documentation good first issue Great for first contributions. Enable to learn the contribution process. and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Nov 25, 2020
@oliviertassinari

This comment has been minimized.

@oliviertassinari oliviertassinari changed the title [Autocomplete:multiple] Support values other than raw options [Autocomplete] Support values other than raw options Nov 25, 2020
@dantman
Copy link
Contributor Author

dantman commented Nov 25, 2020

Forcing the developer to manually manage the values list doesn't feel like a great alternative. Using Select, TextField, etc... isn't this complex even when using the {value, label} pattern.

However, in your case, the simplest is probably to store the value as-is, and derive it with a map when you need to transform it. Why not use this approach?

You mean store country: [{value: 'US', label: 'United States'}, {value: 'CA', label: 'Canada'}] in the form values when US+CA are selected and countries.map(country => country.value) it when doing an api call?

Honestly it feels wrong to have localized labels in form data just because of limitations in useAutocomplete's implementation. And MUI doesn't work this way with any other field. Select, Radio, and Checkbox can easily be connected to form handling code and all support using a separate value and label for options. It doesn't feel right that if you have a Select implemented that way (with the same simple submit handler code as the rest of your fields) and it becomes too long to be a normal select, you have to rewrite the submit handler code to work differently than every other field, just because the field type used changed from Select to Autocomplete.

@oliviertassinari
Copy link
Member

We have a similar pain in this direction on #21489. Ok, why not. we could:

  • use getOptionValue to feed the default behavior of getOptionSelected.
  • use getOptionValue to later implement native integration with a <form>.

Feel free to work on it. At least, we have spotted a small issue with the generated documentation.

@oliviertassinari oliviertassinari added ready to take Help wanted. Guidance available. There is a high chance the change will be accepted and removed docs Improvements or additions to the documentation good first issue Great for first contributions. Enable to learn the contribution process. labels Dec 10, 2020
@ZovcIfzm
Copy link
Contributor

Could I try and take a wack at this?

@ZovcIfzm
Copy link
Contributor

Hi! I'm completely new to open source contributions so I'm a bit confused, wouldn't this just be a one line change? Where we just add,

getOptionValue = (option) => option.value ?? option,

to Autocomplete.js
And then maybe a test?

I'm thinking since getOptionValue is a completely new function, nothing depends on it so all I would need to add is the function itself and a test.

Please correct me if I'm wrong, thank you so much!

@oliviertassinari
Copy link
Member

@ZovcIfzm If you are completely new, you might want to do one "good first issue" before moving to this "goo to take" issue.

Regarding your question. Yes, we would need a new test case, to make sure it has the correct impacts on the component, e.g. change the onChange returned value, change how value <-> option comparison is done, etc.

@ZovcIfzm
Copy link
Contributor

Thank you! Looking at the issue more in-depth I realize it might be out of my scope. I don't quite understand how to change the onChange returned value for example or what you mean by change how value <-> option comparison is done. I'll go and try to tackle a different issue.

@dantman
Copy link
Contributor Author

dantman commented Feb 25, 2021

Investigating this a bit more, this actually requires a breaking change to the UseAutocompleteProps/AutocompleteProps types.

Right now we have getOptionSelected?: (option: T, value: T) => boolean;. However after the change option and value will need different types so we can have getOptionSelected?: (option: O, value: V) => boolean; and getOptionValue?: (option: O) => V;.

So we'll need to change T to O/Option and add a V/Value generic parameter to the type. And any TypeScript users using the type will need to update.

@oliviertassinari oliviertassinari removed the ready to take Help wanted. Guidance available. There is a high chance the change will be accepted label Feb 25, 2021
@oliviertassinari
Copy link
Member

oliviertassinari commented Feb 25, 2021

@dantman option and value should keep the same type. I don't think that it's OK to allow them to diverge. How about we only do #23708 (comment)? Keeping value and options with the same type?

@dantman
Copy link
Contributor Author

dantman commented Feb 25, 2021

@dantman option and value should keep the same type. I don't think that it's OK to allow them to diverge.

That's the whole point of this issue.

Given options=[{value: 'a', label: 'A'}, {value: 'b', label: 'B'}] and thus option[0] = {value: 'a', label: 'A'} (T = { value: string, label: string }) then you get {value: 'a', label: 'A'} as your value and the only type it can be is { value: string, label: string }.

But in real-world applications what we actually want is the ability to have value be "a" and thus its type would be string.

If we instead implemented it as getOptionValue?: (option: T) => T; then the majority of uses for getOptionValue would be impossible type wise.

If the breaking change is so bad, would it would be better to add a permanent workaround?

We could keep UseAutocompleteProps<T, Multiple, DisableClearable, FreeSolo> as an alias to UseComplexAutocompleteProps<O, V, Multiple, DisableClearable, FreeSolo>. That would keep things from breaking and the type migration would only be required for code trying to support getOptionValue.

@oliviertassinari
Copy link
Member

oliviertassinari commented Feb 26, 2021

@dantman I'm not too worried about the branking change but about the mental model it implies. I think that getOptionValue?: (option: T) => any; would work just fine. In my mind, the value returned has nothing to do with the value prop. It's about having a simpler comparison to find the selected options.

@dantman
Copy link
Contributor Author

dantman commented Feb 26, 2021

That's not the limitation in Autocomplete that triggered this issue. The issue is if you use <Autocomplete> instead of <Select> to make a combobox, you cannot easily use simple values the <Autocomplete> will instead shove an object into your form.

const [age, setAge] = React.useState('');

const ages = [
  {value: '10', label: 'Ten'},
  {value: '20', label: 'Twenty'},
  {value: '30', label: 'Thirty'},
];

<FormControl className={classes.formControl}>
  <InputLabel id="demo-simple-select-label">Age</InputLabel>
  <Select
    labelId="demo-simple-select-label"
    id="demo-simple-select"
    value={age}
    onChange={(e) => setAge(e.target.value))}
  >
    {ages.map(age => <MenuItem key={age.value} value={age.value}>{age.label}</MenuItem>)}
  </Select>
</FormControl>

<Autocomplete
  id="demo-combo-box"
  options={ages}
  getOptionLabel={(option) => option.label}
  getOptionValue={(option) => option.value} // Unless we implement this `age` will be `{value: '10', label: 'Ten'}` instead of "10"
  renderInput={(params) => <TextField {...params} label="Age" />}
  value={age}
  onChange={(e, value) => setAge(value))}
/>

@oliviertassinari
Copy link
Member

oliviertassinari commented Feb 26, 2021

// Unless we implement this age will be {value: '10', label: 'Ten'}

Which is the behavior react-select implements for the getOptionValue. I don't think that it will be disorienting as long as the description is simple to understand.

@dantman
Copy link
Contributor Author

dantman commented Feb 26, 2021

Ok, so this isssue is actually a WONTFIX and the getOptionValue you're describing is a fix for a completely different issue.

@oliviertassinari
Copy link
Member

From what I understand, it's also not possible with Downshift too.

@oliviertassinari oliviertassinari added the waiting for 馃憤 Waiting for upvotes label Feb 26, 2021
@oliviertassinari
Copy link
Member

I have added the waiting for users upvotes tag. I'm closing the issue as we are not sure enough people are looking for such capability. So please upvote this issue if you are. We will prioritize our effort based on the number of upvotes.

@austinlangdon
Copy link

Would really like to see this be implemented!

I suspect many developers use options that are modeled like in the OP's post. I know we certainly do.
options=[{value: 'a', label: 'A'}, {value: 'b', label: 'B'}]

The AutoComplete should accommodate this common data model to make it easier for the developer 馃檪

@oliviertassinari
Copy link
Member

@austinlangdon could you expland on the problem you are facing, what's the pain?

@cvburgess
Copy link

This is a huge issue for us when integrating autocomplete with a form library like formik or react-hook-form.

If you use a Select the primitive value is returned and all is good, but if you use an AutoComplete it shoves an object into your form and its not obvious how one is supposed to handle it.

Should I make a wrapper for AutoComplete that handles the values in and out until this is taclked? Is there some other obvious thing I am missing?

For us, label is a string and value is a UUID. When using the resulting form data we would expect the result to be a single UUID or an array of UUID (if the multiple prop is used) - not an object we need to parse... that's not how Select works, and autocomplete in my mind is just a fancy select with typeahead support and some other nice features... but at its core, its a select.

@oliviertassinari
Copy link
Member

@cvburgess Agree that your use case is valid. The proposed solution here is a getOptionValue prop, would is solve your issue?

I also wonder. Did you consider providing options as UUID and do the opposite, use getOptionLabel?

@cvburgess
Copy link

cvburgess commented May 5, 2021

@oliviertassinari that would solve it instantly

I tried the UUIDs as options but it got REALLY messy quickly with having to loop through arrays of objects and .find where the IDs match. It seems really convoluted.

If that is the only path forward until getOptionValue is prioritized I think I will create a wrapper for Autocomplete

@oliviertassinari
Copy link
Member

oliviertassinari commented May 5, 2021

@cvburgess So far, we are simply waiting for more developers to upvote the issue. It's the first time a developer reports the pain with react-hook-form or formik, if we have a concrete example on how it would make the integration simpler, I think that it would help.

The second incentive for getOptionValue is to support POST requests when multiple is true, like https://select2.org/getting-started/basic-usage has.

@cvburgess
Copy link

For me, I am loading async values from a graphQL API and I have quite a few AutoCompletes in a form, they are used to allow folks to quickly type the value they are looking for (in a list of potentially thousands of options) or scroll to select like a normal dropdown.

By using react-hook-form, I need to configure each one to:

  1. map through the objects to make an array of UUIDs for the options prop
  2. for getOptionLabel I need to do another set of loops to compare the strings to their (potentially deeply-) nested values inside the array of objects.

It is not a lot of code, but multiply this out times 10 AutoCompletes and it gets to be a lot of duplicate code. I think getOptionValue is a significantly easier concept for people to understand and it makes a lot of sense to implement.

If my options are objects, I'd expect a simple function to pull out the label, and another to pull out the value and everything should "just work" from there.

PS thank you and the team for the hard work over the years. I've used the lib since the v0 days and the support in these github issues has meant so much to the various teams I've worked on.

@yurtaev
Copy link

yurtaev commented May 6, 2021

Just sharing it here so maybe it helps someone in the future. It's example for wrapper for <Autocomplete /> to support value like <Select />: https://codesandbox.io/s/react-hook-form-v7-ex-selectautocomplete-q5xwg?file=/src/MuiSelectAutocomplete.tsx

@uncvrd
Copy link

uncvrd commented Nov 2, 2022

Has anyone come up with a solid implementation of this? Here's my attempt...with a major drawback that any prop that has a generic type must be reconstructed:

interface CustomProps<
    TOption extends Record<keyof TOption, TOption[keyof TOption]>,
    TProp extends keyof TOption,
> {
    valueKey: TProp;
}

function WrappedAutocomplete<
    TOption extends Record<keyof TOption, TOption[keyof TOption]>,
    TInternal extends TOption[TProp],
    TProp extends keyof TOption,
    Multiple extends boolean | undefined = undefined,
    DisableClearable extends boolean | undefined = undefined,
    FreeSolo extends boolean | undefined = undefined,
>({ valueKey, ...props }: Except<AutocompleteProps<TInternal, Multiple, DisableClearable, FreeSolo>, "options" | "getOptionDisabled" | "getOptionLabel" | "groupBy" | "renderOption" | "isOptionEqualToValue">
    & Pick<AutocompleteProps<TOption, Multiple, DisableClearable, FreeSolo>, "options" | "getOptionDisabled" | "getOptionLabel" | "groupBy" | "renderOption"  | "isOptionEqualToValue">
    & CustomProps<TOption, TProp>) {

    const options = props.options.map((option) => option[valueKey]);

    return (
        <>
            <Autocomplete
                {...props}
                getOptionLabel={(option) => {
                    const item = props.options.find((o) => o[valueKey] === option)

                    return (!!item && !!props.getOptionLabel) ? props.getOptionLabel(item) : ""
                }}
                getOptionDisabled={(option) => {
                    const item = props.options.find((o) => o[valueKey] === option)

                    return (!!item && !!props.getOptionDisabled) ? props.getOptionDisabled(item) : false
                }}
                groupBy={(option) => {
                    const item = props.options.find((o) => o[valueKey] === option)

                    return (!!item && !!props.groupBy) ? props.groupBy(item) : ""
                }}
                isOptionEqualToValue={(option, value) => {
                    const opt = props.options.find((o) => o[valueKey] === option);
                    const val = props.options.find((o) => o[valueKey] === value);

                    return (!!opt && !!val && !!props.isOptionEqualToValue) ? props.isOptionEqualToValue(opt, val) : false
                }}
                options={options}
                renderOption={(prop, option, state) => {
                    const item = props.options.find((o) => o[valueKey] === option)

                    return (!!item && !!props.renderOption) ? props.renderOption(prop, item, state) : ""
                }}
            />
        </>
    )
}

But at least now you can do things like this:

<WrappedAutocomplete
    multiple
    onBlur={onBlur}
    options={teamArtistsData ?? []}
    valueKey={"id"}
    value={value}
    disabled={disabled}
    onChange={(event, value) => {
        onChange(value)
    }}
    getOptionLabel={(option) => {
        return option.name
    }}
    renderInput={(params) => (
        <TextField
            {...params}
            label={label}
            inputProps={{
                ...params.inputProps,
                autoComplete: 'new-password', // disable autocomplete and autofill
            }}
            helperText={helperText}
        />
    )}
/>

And as long as you define the valueKey then it sets the value to the type of the key. Please let me know if there are better solutions...I've been trying for a while lol

@jakub-coinmetro
Copy link

@oliviertassinari I can pretty much agree with all cases described here, I have similar issue, but with formik, however issue is still the same: options[0] type has to match value type, when I'd like to value to be type of options[0]['value']

  • options are [{ value: "1", label: "lorem" }, { value: "2", label: "ipsum" }]
  • value to be "1" or "2"

Which is pretty much identical to #23708 (comment)

right I have to work around this by:

  const assigneesOptions = users?.map((user) => ({ value: user.id, label: user.email })) || [];
  
  return (
    <Autocomplete
        options={assigneesOptions.map((o) => o.value)}
        getOptionLabel={(option) => assigneesOptions.find(o => o.value === option)?.label ?? ""}
        ...
    />

using getOptionValue would be much easier in this case

  const assigneesOptions = users?.map((user) => ({ value: user.id, label: user.email })) || [];
  
  return (
    <Autocomplete
        options={assigneesOptions}
        getOptionValue={(option) => option.value}
        ...
    />

@eliyahuEdocate
Copy link

Hi any update on this issue?

@trentprynn
Copy link

I'm an MUI Pro user and I'm adding onto why this functionality would be useful to me

I am building an address input form that I want to use google auto complete to help the user complete the form quickly, but I don't want to add a requirement that the address must be returned by Google and successfully parsed by us to be used.

The the first address input field, which should bind to address_line_1, is an autocomplete element which shows google autocomplete address options as the user types. If the user selects one of these options the rest of the is filled out using the place details but they also have the option to just type their full address_line_1 value (and the rest of the form values) manually. This solves the problem of addresses not being found by google / us failing to parse a google address return result into the object expected by the API.

In this case, the options of the MUI Autocomplete element are Google Place results, but the value of the autocomplete is a string.

If anyone has any advice on implementation for this case it would be appreciated, my currently implementation is very hacky and causes console warnings about the autocomplete value / option mismatch

@dwjohnston
Copy link
Contributor

dwjohnston commented Jul 4, 2023

Here's my use case:

I have a list of widgets:

const widgets = [
   {
      id: "1", 
      label: "foo",
   }, 
   {
      id: "2", 
      label: "bar",
   }, 
]

I want to be able to use an Autocomplete in a form, as an uncontrolled component, and I want to be able to have a default value set.

So my options are:

1. Make the label be the value that gets submitted on form submission

https://codesandbox.io/s/elated-sanderson-tzfsgc?file=/demo.tsx

      <Autocomplete
        disablePortal
        id="combo-box-demo"
        options={widgets}
        sx={{ width: 300 }}
        getOptionLabel={(v) => v.xLabel}
        renderInput={(params) => (
          <TextField {...params} label="Widget" name="widget" />
        )}
      />

I could then do some kind of remapping back to the ID. But that's a pain.

2. I can use getOptionLabel as the 'value' getter, and user 'renderOption' as the label getter.

https://codesandbox.io/s/blissful-cray-vcntt6?file=/demo.tsx

This almost works, but the problem is that the value will show up in the text field. Also, the search doesn't work.

      <Autocomplete
        disablePortal
        id="combo-box-demo"
        options={widgets}
        sx={{ width: 300 }}
        getOptionLabel={(v) => v.id}
        renderOption={(props, option) => {
          return <li {...props}>{option.xLabel}</li>;
        }}
        renderInput={(params) => (
          <TextField {...params} label="Widget" name="widget" />
        )}
      />

3. Render tags

The renderTags almost solves my problem here (although the search remains a problem). I can use this to render the unfocused textfield value the way I want.

However, renderTags only works when the multiple is true.

@dwjohnston
Copy link
Contributor

dwjohnston commented Jul 4, 2023

Ok got it!

At least for my use case, I can use a hidden input to contain the value, while I still allow the TextField to contain the label value.

https://codesandbox.io/s/suspicious-jennings-m22jrf?file=/demo.tsx:1346-1376

    
  const labelValueMap = React.useMemo(() => {
    const map = new Map<string, string>();

    widgets.forEach((v) => {
      map.set(v.xLabel, v.id);
    });

    return map;
  }, []);


... 
      <Autocomplete
        disablePortal
        id="combo-box-demo"
        options={widgets}
        sx={{ width: 300 }}
        getOptionLabel={(v) => v.xLabel}
        renderInput={(params) => {
          return (
            <>
              <TextField {...params} label="Widget" />
              <input
                type="hidden"
                name="widget"
                value={labelValueMap.get(params.inputProps.value)}
              />
            </>
          );
        }}
      />

@ievgen-klymenko-uvoteam

Ok got it!

At least for my use case, I can use a hidden input ....

Doesn't work for me. Hidden input value is being ignored and the entire option object is still being sent.

Any updates on this?

This sounds more like a bug than a "new feature". Why stop there? Why doesn't it send the entire DOM tree as an input value, for example?..

@ievgen-klymenko-uvoteam

just as quick update, I am sending select options and default values from backend. And just for this type of inputs (select and multiselect), it requires this ugliest "data transformer" workarounds in the form code, which search for option object (from another data source), replaces backend's defaultValue with found option object, to later send it to Autocomplete useController as defaultValue.

Which all could be removed with this one prop, holy.

@ifeltsweet
Copy link

ifeltsweet commented Apr 19, 2024

This is one of the biggest pain points for me every single time I work with MUI forms. Sigh, here we go again. Not sure how many more upvotes we need to make it obvious that getOptionValue is the right way to go. Sorry for the rant, I appreciate you all.

:'(

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 waiting for 馃憤 Waiting for upvotes
Projects
None yet
Development

No branches or pull requests