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] - resetInput when value reference changes #35533

Closed
omerp-explorium opened this issue Dec 19, 2022 · 7 comments · Fixed by #38734
Closed

[Autocomplete] - resetInput when value reference changes #35533

omerp-explorium opened this issue Dec 19, 2022 · 7 comments · Fixed by #38734
Assignees
Labels
component: autocomplete This is the name of the generic UI component, not the React module! docs Improvements or additions to the documentation

Comments

@omerp-explorium
Copy link

omerp-explorium commented Dec 19, 2022

const valueChange = value !== prevValue.current;

Sometimes the value is an array (When autocomplete is multiple), So if after render the value does not referentially the same, the autocomplete will fire reset event.
For example

<Autocomplete value={values.filter(x => isSelected(x.value))}

The Bug/question is, shouldn't we check if the reference is the same for each item in case of multiple?

Thanks

@zannager zannager added component: autocomplete This is the name of the generic UI component, not the React module! status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Dec 19, 2022
@michaldudak
Copy link
Member

Could you please provide a codesandbox so we can reproduce the problem?

@michaldudak michaldudak added status: waiting for author Issue with insufficient information and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Dec 20, 2022
@omerp-explorium
Copy link
Author

Sure. on it

@github-actions github-actions bot removed the status: waiting for author Issue with insufficient information label Dec 20, 2022
@omerp-explorium
Copy link
Author

omerp-explorium commented Dec 20, 2022

@michaldudak
Copy link
Member

So if I understand you correctly, your Autocomplete component resets its value, yes? Could you provide the steps I should take to reproduce the issue on the sandbox you provided?

@michaldudak michaldudak added the status: waiting for author Issue with insufficient information label Dec 20, 2022
@omerp-explorium
Copy link
Author

omerp-explorium commented Dec 21, 2022

the inputValue resets.
Just type in the search anything and it will reset immediately.

It happens because the value which is an array, shallowly equal the prev value but isn't referentially equals.

@github-actions github-actions bot removed the status: waiting for author Issue with insufficient information label Dec 21, 2022
@Zikoat
Copy link

Zikoat commented Jun 30, 2023

It seems like the input breaks when the value doesn't have referential equality between rerenders. There are 2 fixes for this:

  1. Restructure your code so the controlled variable value from onChange does not change referential equality. In @omerp-explorium's case:
-export function Tags({initialIds}) {
-  const [films, setFilms] = React.useState(top100Films.filter(film=>initialIds.includes(film.year)));
- ...
- value={map(ids, (id) => find(top100Films, { year: id }))}
- onChange={(e, next) => {
-   setIds(map(next, (x) => x.year));
- }}
+ export default function Tags() {
+  const [ids, setIds] = React.useState([]);
+ ...
+ value={films}
+ onChange={(e, next) => {
+   setFilms(next);
+ }}
  1. Use useMemo to keep referential equality when the variable changes
const [ids, setIds] = React.useState([]);
const cachedIds = React.useMemo(
  ()=>map(ids, (id) => find(top100Films, { year: id })),
  [ids]
);
...
value={cachedIds}
onChange={(e, next) => {
  setIds(map(next, (x) => x.year));
}}

@michaldudak To fix this, the line referenced in the question should be changed to use shallowEquals or deepEquals.
Otherwise you would have to do some larger restructurings in useAutocomplete to not send a "reset" onInputChange, but i don't know how a solution for that would look.
You could also try to detect this by checking if you send an input event at the same time as a reset event, but i think the reset event gets triggered as an effect, so they don't happen at the same exact time.

The easiest would probably be to add this requirement to the docs, optionally with a fix using useMemo like i have done above.

@michaldudak
Copy link
Member

Thanks for investigating the issue, @Zikoat. We will have this fixed when we refactor the useAutocomplete implementation to use Base UI building blocks, but we don't have an ETA for this.
For now, I'll add a note in the docs.

@michaldudak michaldudak added the docs Improvements or additions to the documentation label Aug 31, 2023
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
Development

Successfully merging a pull request may close this issue.

4 participants