mui / material-ui

Material UI: Comprehensive React component library that implements Google's Material Design. Free forever.
https://mui.com/material-ui/
MIT License
93.46k stars 32.16k forks source link

[Autocomplete] Warn when value and option doesn't match #18514

Closed guimex22 closed 4 years ago

guimex22 commented 4 years ago

Hi, I am trying to change list options and default value from state. List option is ok but default value is never update.

I have add an exemple on this code box : https://codesandbox.io/s/test-material-ui-autocomplete-700nh?fontsize=14&hidenavigation=1&theme=dark

Thank you for yours ideas...

support[bot] commented 4 years ago

👋 Thanks for using Material-UI!

We use GitHub issues exclusively as a bug and feature requests tracker, however, this issue appears to be a support request.

For support, please check out https://material-ui.com/getting-started/support/. Thanks!

If you have a question on StackOverflow, you are welcome to link to it here, it might help others. If your issue is subsequently confirmed as a bug, and the report follows the issue template, it can be reopened.

oliviertassinari commented 4 years ago

Control the component, we don't aim to support defaultValue prop updates (even if a native input do).

guimex22 commented 4 years ago

Thank you for your reply. I am newbie andI think dificulty come from here... but when I add state variable on autocomplete value, items are show on textarea but not selected on dropdown list (and I can selected them second time). Do you have a sample of controlled used of autocomplete ?

oliviertassinari commented 4 years ago

You can find one controlled demo in https://material-ui.com/components/autocomplete/#playground.

guimex22 commented 4 years ago

I have see this but when I try to manage values directely, dropdownlist is not updated with selected values passed by state as you can see on my exemple : https://codesandbox.io/s/test-material-ui-autocomplete-o3uic?fontsize=14&hidenavigation=1&theme=dark

Push button change list and add item preselected but in dropdown list this item is not selected and we can add it another time.

oliviertassinari commented 4 years ago

@guimex22 The value needs to be referentially equal to the option to consider it selected. We have #18443 to customize this.

However, I think that we have an opportunity to warn about this case. What do you think of this diff?

diff --git a/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js b/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js
index f833a0c0c..5cd34fa78 100644
--- a/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js
+++ b/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js
@@ -247,6 +247,35 @@ export default function useAutocomplete(props) {

   popupOpen = freeSolo && filteredOptions.length === 0 ? false : popupOpen;

+  if (process.env.NODE_ENV !== 'production') {
+    if (value !== null && !freeSolo && options.length > 0) {
+      const missingValue = (multiple ? value : [value]).filter(
+        value2 => !options.some(option => getOptionSelected(option, value2)),
+      );
+
+      if (missingValue.length > 0) {
+        // eslint-disable-next-line react-hooks/rules-of-hooks
+        console.warn(
+          [
+            `Material-UI: you have provided an out-of-range value${
+              missingValue.length > 1 ? 's' : ''
+            } \`${
+              missingValue.length > 1
+                ? JSON.stringify(missingValue)
+                : JSON.stringify(missingValue[0])
+            }\` for the autocomplete ${id ? `(id="${id}") ` : ''}component.`,
+            '',
+            'Consider providing a value that matches one of the available options.',
+            'You can use the `getOptionSelected` prop to customize the equality test.',
+          ].join('\n'),
+        );
+      }
+    }
+  }
+
   const focusTag = useEventCallback(tagToFocus => {
     if (tagToFocus === -1) {
       inputRef.current.focus();

Do you want to work on a pull request? :)

thechinedu commented 4 years ago

@oliviertassinari , I'd like to work on this if the op doesn't have the time. Can I go ahead?

oliviertassinari commented 4 years ago

@blueyedgeek Awesome, feel free to go ahead 👌

hefedev commented 4 years ago

@oliviertassinari I'd like to jump on this if no one is currently active.

oliviertassinari commented 4 years ago

@hefedev Great :) we will also need a test case.

thechinedu commented 4 years ago

@hefedev, @oliviertassinari I had meant to work on this but some personal issues came up and I couldn't find the time to do it. Glad someone else was able to put in the required effort 👏🏾

hefedev commented 4 years ago

I won't be able to work on this anymore. but for the new first timers please check below and make sure you need a test case for it.

https://github.com/mui-org/material-ui/blob/575776f3004c6ac655b128fbdb30bd4b35115ab7/packages/material-ui-lab/src/Autocomplete/Autocomplete.test.js#L553

igorbrasileiro commented 4 years ago

@oliviertassinari Can I take this issue?

oliviertassinari commented 4 years ago

@igorbrasileiro Sure :)

CHPrado commented 4 years ago

I think I'm having a problem now because of this.

I'm using an object as value and options like this:

value: {
    value: agendamento?.origem?.cd_cidade,
    description: agendamento?.origem?.nm_cidade,
 },
options: [{
    value: agendamento?.origem?.cd_cidade,
    description: agendamento?.origem?.nm_cidade,
}],

it is still working like before but now I'm having a spam of warnings.

Any suggestion?

oliviertassinari commented 4 years ago

@CHPrado Check the getOptionSelected prop.

CHPrado commented 4 years ago

Thanks, this worked:

getOptionSelected: (
    option,
    value,
 ) => value.value === option.value,
cdpautsch commented 4 years ago

This is actually causing us problems as implemented.

We are using useAutocomplete with multi-select and are fetching our possible options from a back-end service. However, some filtering has already been applied to this list, meaning that it's entirely possible that a selected value will not match the currently available options, even when completely valid (just not visible in the current set).

Adding the freeSolo prop does not solve this, as we do want to restrict the user to only selecting values from the options list.

Example: Type in "exa" and you see a list of options, one of which is "Example". You select it. "Example" is now selected and shows as a little pill in the input. Options are cleared.

Now type in "fo" and you see a different list of options. "Example" is no longer in the options list (having been filtered out by the back end), and now you get an error saying that nothing in the options match what you've selected.

Some of our code:

// Searching the back-end (triggered on debounce in our custom input element)
  const searchCodes= async (match) => {
    try {
      const response = await // call API endpoint w/ match and get codes
      setCodes(response.data);
    } catch (error) {
      setCodes([]);
    }
  };

// UseAutocomplete
  const {
    getRootProps,
    getInputLabelProps,
    getInputProps,
    getTagProps,
    getListboxProps,
    getOptionProps,
    groupedOptions,
    focused,
    value,
    setAnchorEl,
  } = useAutocomplete({
    id: baseId,
    multiple: true,
    options: codes,
    getOptionLabel: (option) => option,
    getOptionSelected: (optionToEval, valueToEval) =>
      optionToEval === valueToEval,
    value: field.value,
  });

Is there a way to disable the warning while still restricting the user to select only from possible options?

oliviertassinari commented 4 years ago

@cdpautsch It looks like this warning has surfaced a bug you had in your current version: the existing selected options were not accounted for. Check the getOptionSelected option to both solve the warning and this bug.

cdpautsch commented 4 years ago

@oliviertassinari I'm not sure what you mean or how to modify getOptionSelected to resolve this error.

getOptionSelected is used to indicate if a given option is selected. This is working exactly as intended. I only want it returning true if an option matches something in value.

Could you please give more insight? I still feel this is a valid concern.

oliviertassinari commented 4 years ago

"Example" is no longer in the options list (having been filtered out by the back end)

@cdpautsch Interesting, I didn't consider this point in my previous comment. So Material-UI has to make a tradeoff between 1. enforcing to keep the previous values int the options + filterSelectedOptions vs 2. ignoring the multi-select case, which won't give any safeguard. I'm leaning for 1.

cdpautsch commented 4 years ago

No worries. I miss things all the time!

To check my understanding on 1, are you saying it's important for Material-UI to enforce that currently selected values are always in the available options?

I would be curious about how useAutocomplete is intended to be used with a back-end service providing the options. If it was never meant to be, I think that's something that should be considered in the future, because it was a clear choice for us. The autocomplete we're using is searching potentially tens of thousands of records, and holding everything in front-end state wasn't practical for us. I feel like this issue would come up again in any similar implementation where the back-end is doing the filtering for you.

For now, my workaround is to manually add in all existing codes to the available options. That way, the options for the autocompletes will always have the selected values, regardless of whether or not your text would actually match them.

oliviertassinari commented 4 years ago

is intended to be used with a back-end service providing the options

@cdpautsch Definitely, I would expect backend filtering as common as frontend filtering.

manually add in all existing codes to the available options

The solution you are mentioning is exactly what I had in mind, plus the filter selected option. This sounds great.

, are you saying it's important for Material-UI to enforce that currently selected values are always in the available options?

Yes, we can't implement this warning without this constraint. My main fear is that people won't easily understand what's going wrong without it. For instance, in the issue's description.


The issue in your case comes from the combination of: 1. filterSelectedOptions, 2. multi-select, 3. API options loading. I think that it's less common and less painful that 1. didn't implement getOptionSelected correctly. Hence why I'm leaning toward maintaining the current logic of the warning.

NivethikaM commented 4 years ago

GroupByError

As mentioned by @cdpautsch I have pushed the already selected option into the options array. Also i have sorted the options using GroupBy method. Now i am getting the below error message:

Material-UI: the options provided combined with the groupBy method of Autocomplete returns duplicated headers. You can solve the issue by sorting the options with the output of groupBy

Inorder to solve the getOptionSelected error, I am pushing the selected values because every time i am getting the differnet option from backend as mentioned above in the post. After pushing the value, getOptionSelected error is not thrown but now i am getting 'GroupBy' error since i am pusing the selected value at the end of the array.

oliviertassinari commented 4 years ago

@NivethikaM This is a different warning, please head to: #20376.

Amritpd commented 4 years ago

Hi is it possible to simply mute this warning in production?

oliviertassinari commented 4 years ago

@Amritpd No warnings are or shoul be printed in production. Check that you build for production (NODE_ENV === 'production)

rmar72 commented 4 years ago

@oliviertassinari I'm in the same situation as @cdpautsch where we are fetching our possible options from a back-end service. When we filter and remove the previous list of options the previous selected option persists behind the scenes, I say behind scenes because you won't see it as an option from the new list in the dropdown. So here you are looking at the new options and don't select any of them but rather click outside(away from dropdown) that's when the previous selected option just pops up again in the TextField and in the console the warning that suggests getOptionsSelected; which we already have in place.

So just to re-iterate on your guys' convo there's no way around this correct ↓

, are you saying it's important for Material-UI to enforce that currently selected values are always in the available options?

Yes, we can't implement this warning without this constraint. My main fear is that people won't easily understand what's going wrong without it. For instance, in the issue's description.

I understand that this was closed after @igorbrasileiro worked on it, if there's a new way to work with this I totally missed it, was this fixed?

@oliviertassinari highly appreciate your time, thanks!

oliviertassinari commented 4 years ago

@rmar72 Apply the same strategy as @cdpautsch to solve the issue, let the component hides the already selected options for you.

rmar72 commented 4 years ago

@oliviertassinari appreciate the reply. How is this not an issue with the google maps api example? You are loading new options, getting rid of the previous selected option and these are updating the options given prop and selected value.

oliviertassinari commented 4 years ago

@rmar72 Thanks for raising about the warning that http://material-ui.com/components/autocomplete#google-maps-place has. Could you open a new issue?

finguer commented 4 years ago

@oliviertassinari , I have a problem when I want to init the value in the AutoComplete image I have created the form from json config all it is ok only I have that problem image image

image

in that case I am init the value with a json, but the component working good image the value and label it is ok when I want to submit only that msg in the console I have put {...(configInput.initialValue && { value })} because when I use the value={value} in my anothes autocomplete I have other warning when I remove the selected image

image

and in my component changed for this way image the icon for remove the label keep there

piotros commented 4 years ago

Is there any way to suppress this warning?

oliviertassinari commented 4 years ago

@piotros By providing the missing option? :)

piotros commented 4 years ago

@oliviertassinari this is not always possible ;) In my use case options changes on user actions but I want to have all selected values preserved.

Take look at simple simulation of my case: https://stackblitz.com/edit/material-ui-autocomplete

Autocomplete components acts as a multistep select. This works as I expect but this worning... :)

oliviertassinari commented 4 years ago

@piotros Nice "hack" for building a "tree select" component with the Autocomplete 😁. Try this diff:

  return (
    <Autocomplete
      renderInput={params => <TextField {...params} />}
      multiple
      open={open}
      getOptionLabel={({ label }) => label}
-     options={options}
+     options={[...value, ...options]}
+     filterSelectedOptions
      onChange={(e, val) => setValue(val)}
    />
  );
piotros commented 4 years ago

Thank you @oliviertassinari. Works perfect now! :)

jcable commented 4 years ago

Hi, how do I stop this warning happening when the initial value is nothing selected and on clear?

flavitsky commented 4 years ago

Hey everyone! Can anybody help me? How can I change focus from the selected option back to the input field without using a mouse? To be clear - I'm trying to use autocompletion without the mouse and once I turn to option selection with pressing arrow down I can`t go back to the input TextField. Thx

vince1995 commented 4 years ago

If anybody is struggling with this around, try combining this:

Thanks, this worked:

getOptionSelected: (
    option,
    value,
 ) => value.value === option.value,

And this:

@piotros Nice "hack" for building a "tree select" component with the Autocomplete 😁. Try this diff:

  return (
    <Autocomplete
      renderInput={params => <TextField {...params} />}
      multiple
      open={open}
      getOptionLabel={({ label }) => label}
-     options={options}
+     options={[...value, ...options]}
+     filterSelectedOptions
      onChange={(e, val) => setValue(val)}
    />
  );
alainib commented 4 years ago

i have the same warning but i already added getOptionSelected, i make infinity console warning

Material-UI: The value provided to useAutocomplete is invalid.
None of the options match with `{"id":"geonames:6429478","name":"Périgueux"}`.
You can use the `getOptionSelected` prop to customize the equality test.

the code:


  let {
    getRootProps,
    getInputLabelProps,
    getInputProps,
    getListboxProps,
    getOptionProps,
    groupedOptions,
    getOptionSelected,
    focused,
    setAnchorEl,
    anchorEl,
    inputValue
  } = useAutocomplete({
    value: selectedOption || {},
    options: options,
    getOptionLabel: (option) => {
      return option.name || "";
    },
    getOptionSelected: (option, value) =>{
       console.log({value,option});
       // I added id comparaison because in option.name accent char are replaced ( éàè....) 
        return  option?.name.toLowerCase() === value?.name.toLowerCase() || option?.id === value?.id
    },
    selectOnFocus: true
  });

the log from getOptionSelected :

option: {​​
  id: "geonames:6616167",name: "Mareuil en Perigord"
​​}
value: { 
  id: "geonames:6616167", 
  name: "Mareuil en Périgord"
}

Any idea please ?

OmriAroesti commented 4 years ago

Hi, how do I stop this warning happening when the initial value is nothing selected and on clear?

I personally added the freeSolo flag and then validated the value on onChange(a simple null check)

nicholasbca commented 4 years ago

I want to init first selection as empty string, but I got warning, any solution please?

const [value, setValue] = React.useState(""); //got warned
const [yearValue, setYearValue] = React.useState(top100Films[0]); not warned
Material-UI: The value provided to Autocomplete is invalid.
None of the options match with `""`.
You can use the `getOptionSelected` prop to customize the equality test. 
    in Autocomplete (created by WithStyles(ForwardRef(Autocomplete)))
    in WithStyles(ForwardRef(Autocomplete)) (at demo.js:12)
    in ComboBox (at index.js:6)

here is my code: https://codesandbox.io/s/autocomplete-set-another-autocomplete-value-uko9b?file=/demo.js

hinsxd commented 4 years ago

@jcable @nicholasbca For single select, I think the empty initial problem can be solved by:

options={
  value === null // depends on what your empty value is
    ? options
    : [value, ...options]
}
filterSelectedOptions

Multiselect has already been solved by @oliviertassinari

iansjk commented 4 years ago

I had the same problem with warnings for None of the options match with "" when using an Autocomplete with a controlled value.

I ended up using this because I didn't want to enable filterSelectedOptions for UX reasons:

options={[value, ...options]}
filterOptions={(options) =>
  options.filter((option) => option !== "")
}
Forfold commented 4 years ago

I have the same issue as @cdpautsch:

We are using useAutocomplete with multi-select and are fetching our possible options from a back-end service. However, some filtering has already been applied to this list, meaning that it's entirely possible that a selected value will not match the currently available options, even when completely valid (just not visible in the current set).

However I was able to get around the strictness of the Autocomplete API with the following code:

export default function MaterialSelect(
  props: CommonSelectProps & (MultiSelectProps | SingleSelectProps),
) {

  . . .

  // https://github.com/mui-org/material-ui/issues/18514
  // be sure to keep the props `filterSelectedOptions` set to hide selected value from dropdown
  // and `getOptionSelected` to ensure what shows as selected for all incoming values
  let options: SelectOption[] = _options
  if (value) {
    // merge options with value
    options = options.concat(Array.isArray(value) ? value : [value])
  }

  return (
    <Autocomplete
      . . .
      filterSelectedOptions
      getOptionSelected={(opt, val) => opt.value === val.value}
  )
}

I originally was using Sets to merge my value and options arrays, and while this worked for selected results that were not in our initial query (we default to showing 5 options when the user focuses the field), if any of the options from the initial dataset are selected the warning still appears. Once I removed using a set allowing duplicates in options, and utilized both filterSelectOptions and getSelectedOption, the issue was resolved.

ffjeanette commented 3 years ago

This is my case. I have a component wrapping some form elements. The data used for the autocomplete value is passed in as a prop to this wrapper component. The draft.valueX can be undefined. As iansjk I didnt want to use the filterSelectedOptions prop for ux reasons, but I need the users to be able to filter the options. I dont like it, but this is my solutions for now.

type Option = {
   value: string
   label: string
}
type Draft = {
   valueX?: string
   . . . 
}

const defaultFilterOptions = createFilterOptions<Option>()

const MyComponent = ({ draft, ...}: { draft: Draft.... }) => {

  const option: Option = useMemo(() => ({
    value: draft.valueX ? draft.valueX  : '',
    label: draft.valueX ? getOptionLabel(draft.valueX ) : ''
  }), [draft])

  return (
      <Autocomplete
      options={!option.value ? [option, ...options] : options}
      filterOptions={(options: Option[], state) => {
          //use default filter and then remove "empty" initial value if needed
          const defaultFiltered = defaultFilterOptions(options, state)
          return defaultFiltered.filter((option) => option.value !== '')
      }}
    . . . 
  />
ifeltsweet commented 3 years ago

We had a similar issue where we were getting filtered options from the backend depending on the current input value. As such, the currently selected value may or may not have been present in the options returned by the backend which in turn triggered the warning everyone is talking about here.

To solve this we did the following:

<Autocomplete
  options={[value, ...serverOptions]} // Currently selected value must be preset to not trigger the missing value warning.
  filterOptions={() => serverOptions} // But we only show what BE returned to us, so there are no duplicates.
  value={value}
  onChange={(_, value) => onChange(value)}
  onInputChange={(_, value) => setSearchQuery(value)} // Triggers BE to return filtered options.
/>

This works exactly how we want it to work, including that the selected option is highlighted.

giladv commented 2 years ago

Just wanted to add these warnings do more harm than good. Requires so much hacks to make it happy so i won't warn when options are dynamic (from server).

kcarmonamurphy commented 2 years ago

I followed the suggestion from @ifeltsweet and I was able to make this warning disappear. For context, I had three <Autocomplete> fields whose values were filtered from the selections of the previous ones.