Skip to content

Fix <AutocompleteInput> can flicker when used in a <FormDataConsumer>#10496

Closed
slax57 wants to merge 5 commits intomasterfrom
fix-FormDataConsumer-flicker-2
Closed

Fix <AutocompleteInput> can flicker when used in a <FormDataConsumer>#10496
slax57 wants to merge 5 commits intomasterfrom
fix-FormDataConsumer-flicker-2

Conversation

@slax57
Copy link
Contributor

@slax57 slax57 commented Feb 6, 2025

Problem

Fixes #10415

Supersedes #10417 (which was reverted in 3f6a312 as it caused other issues notably with StackedFilters).

Solution

The flicker actually comes from a missing memoization of the optionText prop in the <AutocompleteInput> component.

How To Test

For both: Enter a title then select a user. Notice the flicker each time you change the user value. You might have to set a CPU throttle in your devtool to notice it.


FTR: Also added a story allowing to reproduce the issue the first fix attempt created in the StackedFilters component.
To reproduce the bug:

  1. Open story: http://localhost:9010/?path=/story/ra-core-form-formdataconsumer--stacked-filters
  2. Add a new filter, leave every input blank
  3. Remove the newly added filter
  4. Add a new filter again
  5. This should trigger an error: Cannot read properties of null (reading 'focus')

Additional Checks

  • The PR targets master for a bugfix, or next for a feature
  • The PR includes unit tests (if not possible, describe why): not possible
  • The PR includes one or several stories (if not possible, describe why)
  • The documentation is up to date

Also, please make sure to read the contributing guidelines.

Comment on lines +131 to +142
export const StackedFilters = () => (
<AdminContext dataProvider={dataProvider}>
<ResourceContextProvider value="users">
<ListBase>
<Paper sx={{ p: 2 }}>
<StackedFiltersForm />
<FiltersDebugger />
</Paper>
</ListBase>
</ResourceContextProvider>
</AdminContext>
);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the end I did not use this story, as we did not change the FormDataConsumer implementation. But I figured we could still keep it to ease tests later. Wdyt, should we keep it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, let's keep it

@slax57 slax57 added the RFR Ready For Review label Feb 6, 2025
@slax57
Copy link
Contributor Author

slax57 commented Feb 6, 2025

Some tests are failing 🙁 . Back to WIP.

@slax57 slax57 added WIP Work In Progress and removed RFR Ready For Review labels Feb 6, 2025
@slax57
Copy link
Contributor Author

slax57 commented Feb 6, 2025

It turns out memoizing the optionText callback is not that easy, because it is used in the render phase (so useEvent will throw an error).
Memoizing it would require a partial refactor of AutocompleteInput to make sure this prop is not used during the render phase.
We may do that eventually, but this requires quite a lot of work.

Additionally, we wanted to document the fact that, for now, it's up to the user to make sure to pass a stable reference to the optionText prop. But it turns out it's already documented!

So, for these reasons, I'm closing this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

WIP Work In Progress

Development

Successfully merging this pull request may close these issues.

FormDataConsumer component cause rerendering or glitching the input upon selection/reset the value

2 participants