Open dantman opened 4 years ago
@dantman There is a 4th argument to the onChange event that allows handling this problem. It looks like there is an opportunity to better document it. What do you think about this change?
diff --git a/packages/material-ui/src/useAutocomplete/useAutocomplete.d.ts b/packages/material-ui/src/useAutocomplete/useAutocomplete.d.ts
index 0dfea8c799..47be1bd484 100644
--- a/packages/material-ui/src/useAutocomplete/useAutocomplete.d.ts
+++ b/packages/material-ui/src/useAutocomplete/useAutocomplete.d.ts
@@ -250,6 +250,7 @@ export interface UseAutocompleteProps<
* @param {object} event The event source of the callback.
* @param {T|T[]} value The new value of the component.
* @param {string} reason One of "create-option", "select-option", "remove-option", "blur" or "clear".
+ * @param {string} [details]
*/
onChange?: (
event: React.SyntheticEvent,
it will then automatically update the API:
diff --git a/docs/pages/api-docs/autocomplete.md b/docs/pages/api-docs/autocomplete.md
index 34caa37fa2..41f18c5120 100644
--- a/docs/pages/api-docs/autocomplete.md
+++ b/docs/pages/api-docs/autocomplete.md
@@ -67,7 +67,7 @@ The `MuiAutocomplete` name can be used for providing [default props](/customizat
| <span class="prop-name">loadingText</span> | <span class="prop-type">node</span> | <span class="prop-default">'Loading…'</span> | Text to display when in a loading state.<br>For localization purposes, you can use the provided [translations](/guides/localization/). |
| <span class="prop-name">multiple</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | If `true`, `value` must be an array and the menu will support multiple selections. |
| <span class="prop-name">noOptionsText</span> | <span class="prop-type">node</span> | <span class="prop-default">'No options'</span> | Text to display when there are no options.<br>For localization purposes, you can use the provided [translations](/guides/localization/). |
-| <span class="prop-name">onChange</span> | <span class="prop-type">func</span> | | Callback fired when the value changes.<br><br>**Signature:**<br>`function(event: object, value: T \| T[], reason: string) => void`<br>*event:* The event source of the callback.<br>*value:* The new value of the component.<br>*reason:* One of "create-option", "select-option", "remove-option", "blur" or "clear". |
+| <span class="prop-name">onChange</span> | <span class="prop-type">func</span> | | Callback fired when the value changes.<br><br>**Signature:**<br>`function(event: object, value: T \| T[], reason: string, details?: string) => void`<br>*event:* The event source of the callback.<br>*value:* The new value of the component.<br>*reason:* One of "create-option", "select-option", "remove-option", "blur" or "clear". |
| <span class="prop-name">onClose</span> | <span class="prop-type">func</span> | | Callback fired when the popup requests to be closed. Use in controlled mode (see open).<br><br>**Signature:**<br>`function(event: object, reason: string) => void`<br>*event:* The event source of the callback.<br>*reason:* Can be: `"toggleInput"`, `"escape"`, `"select-option"`, `"remove-option"`, `"blur"`. |
| <span class="prop-name">onHighlightChange</span> | <span class="prop-type">func</span> | | Callback fired when the highlight option changes.<br><br>**Signature:**<br>`function(event: object, option: T, reason: string) => void`<br>*event:* The event source of the callback.<br>*option:* The highlighted option.<br>*reason:* Can be: `"keyboard"`, `"auto"`, `"mouse"`. |
| <span class="prop-name">onInputChange</span> | <span class="prop-type">func</span> | | Callback fired when the input value changes.<br><br>**Signature:**<br>`function(event: object, value: string, reason: string) => void`<br>*event:* The event source of the callback.<br>*value:* The new value of the text input.<br>*reason:* Can be: `"input"` (user input), `"reset"` (programmatic change), `"clear"`. |
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?
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.
We have a similar pain in this direction on #21489. Ok, why not. we could:
getOptionValue
to feed the default behavior of getOptionSelected
.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.
Could I try and take a wack at this?
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!
@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.
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.
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.
@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 https://github.com/mui-org/material-ui/issues/23708#issuecomment-733640638? Keeping value and options with the same type?
@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
.
@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.
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))}
/>
// 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.
Ok, so this isssue is actually a WONTFIX
and the getOptionValue
you're describing is a fix for a completely different issue.
From what I understand, it's also not possible with Downshift too.
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.
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 🙂
@austinlangdon could you expland on the problem you are facing, what's the pain?
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.
@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
?
@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
@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.
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:
options
propgetOptionLabel
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.
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
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
@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 https://github.com/mui/material-ui/issues/23708#issuecomment-832819637
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}
...
/>
Hi any update on this issue?
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
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.
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)}
/>
</>
);
}}
/>
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?..
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.
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.
:'(
@oliviertassinari Waiting on this fix (getOptionValue) to be implemented
Upvote for me here, having the exact same issues described.
Upvote , still writing tons of codes to convert backend id's into objects .
Summary 💡
Right now (at least if you use
multiple
) Autocomplete/useAutocomplete thevalue
that comes back inonChange
is always the raw option data. i.e. If you useoptions=[{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 agetOptionSelected
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 amultiple
array, all useAutocomplete does isnewValue.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 🌈
Motivation 🔦
MUI's
<Select>
easily supports this. It uses children instead, so all it takes isoptions.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.