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] getOptionLabel callback is getting the value instead of option object #31192

Open
2 tasks done
hasan-aa opened this issue Feb 24, 2022 · 17 comments
Open
2 tasks done
Labels
component: autocomplete This is the name of the generic UI component, not the React module! docs Improvements or additions to the documentation

Comments

@hasan-aa
Copy link

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the latest version

Current behavior 馃槸

When using different types for options and value properties, the getOptionLabel method is passed value instead of option object.

let's say I'm using following types:

type Option = { label: string; value: any; group?: string }
type Value = string;

I'm expecting getOptionLabel method always passed an Option type but that's not the case. That's why the picked option can not be displayed properly.

getOptionLabel={(option) => option.label || "!!!"} //here sometimes Value type is passes instead of Option type.
isOptionEqualToValue={(option, value) => return option.value === value}

Expected behavior 馃

Always pass and option to getOptionLabel as a param to be consistent.

Steps to reproduce 馃暪

Here's a demo:
https://codesandbox.io/s/controllablestates-material-demo-forked-0lekyg?file=/demo.js

Steps:

  1. Render a controlled Autocomplete component using different types for options and value props. (object and string)
  2. Utilize isOptionEqualToValue and getOptionLabel callback props

Context 馃敠

I'm trying to render options that are objects like {label:"Label", value:"some value"}, but I want to use the value of the selected option instead of the whole option object.

Your environment 馃寧

No response

@hasan-aa hasan-aa added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Feb 24, 2022
@danilo-leal danilo-leal changed the title [Autocomplete] getOptionLabel callback is getting the value instead of option object [Autocomplete] getOptionLabel callback is getting the value instead of option object Feb 28, 2022
@danilo-leal danilo-leal added component: autocomplete This is the name of the generic UI component, not the React module! and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Feb 28, 2022
@ZeeshanTamboli
Copy link
Member

ZeeshanTamboli commented Apr 28, 2022

Hi @hasan-aa ,

Why do you want to have value as a string? You can keep the value as one of the option object in the state if your options are objects.

Context 馃敠

I'm trying to render options that are objects like {label:"Label", value:"some value"}, but I want to use the value of the selected option instead of the whole option object.

You can use the value of the selected option. See the following CodeSandbox: https://codesandbox.io/s/controllablestates-material-demo-forked-c4fw5y?file=/demo.js

@lovervoorde
Copy link

For my use-case, I wanted to have the option as a string because my onChange uses the value to update an object in redux state, and I don't want the label in there as well.

@Zirafnik
Copy link

Zirafnik commented Nov 9, 2022

Why was this closed and the suggested PR just rejected?

I spent 2 days debugging what was breaking, only to find that getOptionLabel uses the value property to determine the text (label) for displaying selected option.

Assuming I have an array of objects as options, for instance [ { name: 'Charles Dickens', value: 'charles-dickens' } ]. I want to display the name property as label getOptionLabel={(option) => option.name} and save only the value to state, to be later sent to the backend onChange={(e, value) => setValue(value.value)}. In this case isOptionEqualToValue={(option, value) => option.value === value}.

Why am I therefore forced to save the whole object as the value? (just so getOptionLabel can then pull it to display selected value)

The fix linked above should be reconsidered.

@ZeeshanTamboli
Copy link
Member

Okay I am re-opening the PR.

@Bearfinn
Copy link

To support this request, I am using MUI with react-hook-form in which value from the server is in form of uuid (string) type and send the data back to the server with uuid (string) only.

If this issue is not tackled, I will need to either:

  1. Convert uuid to full object as value to be compatible with the types, and then extract uuid from the full object when sending update request to the server.
  2. Forcing a lot of types to work with having different types of value and option as in getOptionLabel isOptionEqualToValue etc.
    Both took a lot of work and are prone to errors

I think the concept that value and option is separated is beneficial, while you can choose whether it is the same thing or not. This behavior is similar to Select component in which the options and value are separated.

@michaldudak
Copy link
Member

A value must have the same type as an element of options. To solve your problem, I'd go with something like this:

// outside of the component:
type Option = { value: string, name: string };
const options: Option[] = [ { name: 'Charles Dickens', value: 'charles-dickens' } ]

// in component:
const values = React.useMemo(() => options.map(o => o.value));
function getLabel(value) {
  return options.find(o => o.value === value);
}

return (
  <Autocomplete
    options={values}
    getOptionLabel={getLabel}
    ...

Would this solve your problem?

Having separate types for options and value would require more changes than what's proposed in #32439. The Autocomplete types assume that options: ReadonlyArray<T>; (where T is the type of the value)

@hasan-aa
Copy link
Author

hasan-aa commented Nov 25, 2022

I think at least a note should be added to the documentation of getOptionLabel method until this is fixed. The name suggests that it'll pass an option object.
There are ways to work around this but it's time consuming to debug and find the actual reason once you get that point.

@Zirafnik
Copy link

@michaldudak Yes, but what we are saying is that this should not be the case. Value should not have to equal the option type. Value should be decoupled from options.

Obviously there are various workarounds for this at the moment, the easiest one being just saving 2 states: one for saving the actual chosen object (option) & the other for saving only the value that will be sent to the backend (option.value).

According to @Bearfinn this is already the implemented (and expected) behavior in the Select component, so it does not make sense for it to differ in Autocomplete. Until then I agree with the fact that this should be mentioned in docs (to save some people the trouble of debugging this unexpected behavior).

@biggytech
Copy link

biggytech commented Jan 6, 2023

The documentation is misleading:

image

image

Considering these words in the doc we expect that a bunch of string value + isOptionEqualToValue={(option, value) => option.value === value} + getOptionLabel={(option) => option.label} should work. But it doesn't.

Should be: either make getOptionLabel to get an option, either change this lines in the docs.

@ZeeshanTamboli
Copy link
Member

We would like to have the value as the same type as an element of options else there would be lots of changes (eg in TypeScript types etc).

We can go ahead with updating the docs to mention that value prop should be of the same type as an element of option so that getOptionLabel callback accepts an option properly. We can also add a demo showing the implementation.

@ZeeshanTamboli ZeeshanTamboli added the docs Improvements or additions to the documentation label Jan 10, 2023
@kachmul2004
Copy link

The most appropriate, yet longer, name for "getOptionLabel" would have been "getSelectedOptionLabel", because that is what this function actually does, and like many others, it caused me so much pain, especially when used with react-hook-form.

@Zirafnik
Copy link

Seeing as the 'docs' tag was added to this issue, I would like to stress that this issue should not be closed upon only updating the documentation. A nice heads up about the unexpected behavior in the docs would be nice, but it is only a band-aid solution until this problem is properly fixed. (See detailed problem description and previous fix attempts above.)

@markedwards
Copy link

One case where this breaks is when the field is cleared. In this case, the value is "", which is never T in the case that options are some non-string object. So getOptionLabel receives "", but its option argument is not typed as T | string, it's typed as T.

Why is getOptionLabel called in this situation in the first place? There is no option selected, so there is no way to get its label.

@oliviertassinari
Copy link
Member

oliviertassinari commented Feb 9, 2023

This issue looks like a duplicate of #23708 but focus on the docs that is currently confusing.

@cpdwyer
Copy link

cpdwyer commented Feb 10, 2023

I am having a similar issue with the argument in the call back however this is when the value in the option object is an array, it is only giving me the array.

I am running 5.11.2

e.g. options
[{ id: 1, label: 'option 1', value: [ "ABC", 'DEF']}, { id: 2, label: 'option 2', value: ['123', '456']}]

if option 1 is selected I am getting ["ABC", "DEF"] as the argument in getOptionLabel and not { id: 1, label: 'option 1', value: [ "ABC", 'DEF']} as I would expect from the api docs.

I have workaround it for now but is is a little weird that the option object does exist in the correct shape to the spec. The workaround is a little inefficient as it means running a find on the available options again to get the label so would like to remove it.

@alenadvainova
Copy link

I have the same issue.

My case:
I have the form with autocomplete field on it. If i open the form to edit, i already have initial autocomplete value which comes form api and it is id.
autocomplete options: [{id: '1', name: 'option 1'}, {id: '2, name: 'option 2'}]
autocomplete value: '1'

I expect to see correct name in Autocomplete input field , not undefined, because i define getOptionLabel and assume that option parameter is option and not value

why value should have the same type as option if there is isOptionEqualToValue where i can say how value should be mapped to option.

@jarekucinski
Copy link

The same issue.

based on the documentation, I would never have guessed that the 'label' in getOptionLabel is from 'values' and not from 'options'.

I expect that the list of options is there to get information about options from it.
Moreover, as mentioned above, when using a react-hook-form we have to work around it by keeping an eye on everything in many places at the same time, where if it worked in accordance with the above proposal, it would solve many problems.

Like if you use react-query for getting options, and react-hook-form for now I would need to reset form/field on every available options update.

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! docs Improvements or additions to the documentation
Projects
None yet