Introduce `<FilterLiveForm>` #10344

slax57 · 2024-11-12T16:53:53Z

Problem

The FilterList is designed for buttons - not inputs. We provide the FilterLiveSearch component, which is a TextInput inside a form that submits on change. But some users want to be able to add custom form inputs (e.g. to select an author from a resource using an autocomplete), and re-writing the form wrapper is cumbersome.

Solution

Introduce a new component to wrap form inputs in that case:

export const EventFilterAside = () => (
    <Card>
        <CardContent>
            <FilterList label="Has ordered" icon={<MonetizationOnIcon />}>
                <FilterListItem
                    label="True"
                    value={{
                        nb_commands_gte: 1,
                        nb_commands_lte: undefined,
                    }}
                />
                <FilterListItem
                    label="False"
                    value={{
                        nb_commands_gte: undefined,
                        nb_commands_lte: 0,
                    }}
                />
            </FilterList>
            <FilterLiveForm>
                <TextInput source="q" />
            </FilterLiveForm>
            <FilterLiveForm>
                <ReferenceInput source="authorId" reference="authors" />
            </FilterLiveForm>
        </CardContent>
    </Card>
);

This PR also fixes #5591 🎉

Todo

How To Test

<FilterLiveForm> logic: http://localhost:9010/?path=/story/ra-core-controller-list-filter-filterliveform--basic
<FilterLiveForm> UI: http://localhost:9010/?path=/story/ra-ui-materialui-list-filter-filterliveform--full-app
<FilterListSection>: http://localhost:9010/?path=/story/ra-ui-materialui-list-filter-filterlistsection--basic
Filter debounce bug: http://localhost:9010/?path=/story/ra-ui-materialui-input-searchinput--race-condition
<FilterLiveSearch> old value bug: http://localhost:9010/?path=/story/ra-ui-materialui-list-filter-filterlivesearch--full-app
1. Enter a filter value (e.g. 'foo')
2. Navigate to Countries
3. Navigate back to Books
4. Clear the <FilterLiveSearch>
5. Confirm the old value ('foo') does not come back

Additional Checks

The PR targets master for a bugfix, or next for a feature
The PR includes unit tests (if not possible, describe why)
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.

…tFilterForm to only submit on real changes

slax57 · 2024-11-14T17:45:00Z

packages/ra-ui-materialui/src/list/filter/FilterForm.tsx

@@ -30,35 +24,45 @@ import { FilterFormInput } from './FilterFormInput';
 import { FilterContext } from '../FilterContext';

 export const FilterForm = (props: FilterFormProps) => {
-    const { defaultValues, filters: filtersProps, ...rest } = props;


defaultValues is not used in all RA's code, and I tested it and it does not work, so I removed it to reduce tech debt. Shouldn't be a BC since it does not currently work.

slax57 · 2024-11-14T17:46:24Z

packages/ra-ui-materialui/src/list/filter/FilterForm.tsx

-    const mergedInitialValuesWithDefaultValues =
-        mergeInitialValuesWithDefaultValues(
-            defaultValues || filterValues,
-            filters
-        );


This whole call is actually useless, because:

defaultValues don't work

mergeInitialValuesWithDefaultValues actually does nothing, since there is a runtime check to prevent having both alwaysOn and defaultValue. It's probably a historic tech debt, that's why I removed it.

slax57 · 2024-11-14T17:47:27Z

packages/ra-ui-materialui/src/list/filter/FilterForm.tsx

@@ -270,7 +252,7 @@ const getInputValue = (
    filterValues: Record<string, any>
 ) => {
    if (formValues[key] === undefined || formValues[key] === null) {
-        return '';
+        return get(filterValues, key, '');


same, historic bug (see updated unit test)

djhi · 2024-11-15T11:10:20Z

packages/ra-core/src/controller/list/filter/AutoSubmitFilterForm.tsx

+        resolver,
+        validate,
+        children,
+        component: Component,


Why don't you provide a default value here and avoid the ternary at L124?

I didn't know how to do it, but now I know! Thanks!

packages/ra-ui-materialui/src/list/filter/FilterListWrapper.tsx

Copilot reviewed 13 out of 28 changed files in this pull request and generated no suggestions.

Files not reviewed (15)

docs/navigation.html: Language not supported
docs/Reference.md: Evaluated as low risk
packages/ra-core/src/util/index.ts: Evaluated as low risk
packages/ra-ui-materialui/src/list/filter/FilterList.stories.tsx: Evaluated as low risk
packages/ra-ui-materialui/src/list/filter/FilterForm.spec.tsx: Evaluated as low risk
packages/ra-ui-materialui/src/list/filter/FilterList.tsx: Evaluated as low risk
packages/ra-core/src/controller/list/index.ts: Evaluated as low risk
packages/ra-ui-materialui/src/list/filter/FilterForm.tsx: Evaluated as low risk
packages/ra-core/src/controller/list/filter/index.ts: Evaluated as low risk
packages/ra-core/src/controller/list/filter/AutoSubmitFilterForm.tsx: Evaluated as low risk
packages/ra-ui-materialui/src/input/SearchInput.spec.tsx: Evaluated as low risk
docs/FilteringTutorial.md: Evaluated as low risk
docs/FilterList.md: Evaluated as low risk
docs/FilterLiveSearch.md: Evaluated as low risk
packages/ra-ui-materialui/src/input/SearchInput.stories.tsx: Evaluated as low risk

djhi · 2024-11-18T08:42:41Z

packages/ra-ui-materialui/src/list/filter/FilterListWrapper.tsx

+import { Box, BoxProps, Typography } from '@mui/material';
+import { useTranslate } from 'ra-core';
+
+export const FilterListWrapper = (props: FilterListWrapperProps) => {


I would name this FilterListSection

djhi · 2024-11-18T08:44:22Z

docs/FilterLiveSearch.md

@@ -12,7 +12,7 @@ title: "The FilterLiveSearch Component"
 </video>


-The filter sidebar is not a form. Therefore, if your users need to enter complex filters, you'll have to recreate a filter form using react-hook-form (see the [Building a custom filter](./FilteringTutorial.md#building-a-custom-filter) for an example). However, if you only need one text input with a filter-as-you-type behavior, you'll find the `<FilterLiveSearch>` component convenient.
+The filter sidebar is not a form. Therefore, if your users need to enter complex filters, you'll have to recreate a filter form. This can easily be done thanks to the [`<AutoSubmitFilterForm>`](./AutoSubmitFilterForm.md) component. However, if you only need one text input with a filter-as-you-type behavior, you'll find the `<FilterLiveSearch>` component even more convenient.


Suggested change

The filter sidebar is not a form. Therefore, if your users need to enter complex filters, you'll have to recreate a filter form. This can easily be done thanks to the [`<AutoSubmitFilterForm>`](./AutoSubmitFilterForm.md) component. However, if you only need one text input with a filter-as-you-type behavior, you'll find the `<FilterLiveSearch>` component even more convenient.

The filter sidebar is not a form. Therefore, if your users need to enter complex filters, you'll have to recreate a filter form. This can be done thanks to the [`<AutoSubmitFilterForm>`](./AutoSubmitFilterForm.md) component. However, if you only need one text input with a filter-as-you-type behavior, you'll find the `<FilterLiveSearch>` component even more convenient.

djhi · 2024-11-18T08:50:53Z

packages/ra-core/src/util/useDebouncedEvent.ts

+export const useDebouncedEvent = (callback, delay: number) => {
+    // Create a ref that stores the debounced callback
+    const debouncedCallbackRef = useRef(debounce(callback, delay));
+
+    // Whenever callback or delay changes, we need to update the debounced callback
+    useEffect(() => {
+        debouncedCallbackRef.current = debounce(callback, delay);
+    }, [callback, delay]);
+
+    // The function returned by useCallback will invoke the debounced callback
+    // Its dependencies array is empty, so it never changes across re-renders
+    return useCallback((...args) => debouncedCallbackRef.current(...args), []);
+};


This doesn't have the checks we do in useEvent to prevent calling the callback inside a render. I'm not really sure of the consequences but the react team has made it a strong limitation of useEvent (see its code in our repo)

erwanMarmelab · 2024-11-18T10:26:26Z

packages/ra-core/src/controller/list/filter/AutoSubmitFilterForm.tsx

+    const {
+        debounce = 500,
+        resolver,
+        validate,


When you have a non validated input you can't submit. But if you submit your FilterList in a same time, it will reset your AutoSubmitForm to the last validated state.
Do we need to consider it as an issue or as a normal way of work @fzaninotto @djhi ?

Capture.video.2024-11-18.11.19.16.mp4

IMO this isn't a bug

erwanMarmelab · 2024-11-18T10:51:43Z

Such a really nice job 🦾

erwanMarmelab · 2024-11-18T12:51:27Z

Problem

With a FilterLiveSearch or an AutoSubmitFilterForm. If I write something and click on the clear filters button faster than my debounce, the new search query happens after the clearing.

It can easily happens when your app need to increase a bit the debounce.

Moreover, when it happens, your input seems to be empty… it’s confusing

Solution

Make the clear filters button cancel next debounced search query

fzaninotto

Only reviewed documentation so far

docs/AutoSubmitFilterForm.md

docs/FilterList.md

docs/FilterLiveSearch.md

fzaninotto

Awesome!

packages/ra-core/src/controller/list/filter/AutoSubmitFilterForm.tsx

fzaninotto · 2024-11-18T14:11:52Z

packages/ra-core/src/controller/list/filter/AutoSubmitFilterForm.tsx

+    const {
+        debounce = 500,
+        resolver,
+        validate,


IMO this isn't a bug

packages/ra-core/src/controller/list/filter/AutoSubmitFilterForm.tsx

packages/ra-core/src/controller/list/filter/index.ts

packages/ra-ui-materialui/src/list/filter/FilterForm.tsx

packages/ra-ui-materialui/src/list/filter/FilterListWrapper.tsx

packages/ra-ui-materialui/src/list/filter/FilterLiveSearch.stories.tsx

packages/ra-ui-materialui/src/list/filter/FilterLiveSearch.tsx

slax57 · 2024-11-20T10:34:39Z

Problem

With a FilterLiveSearch or an AutoSubmitFilterForm. If I write something and click on the clear filters button faster than my debounce, the new search query happens after the clearing.

It can easily happens when your app need to increase a bit the debounce.

Moreover, when it happens, your input seems to be empty… it’s confusing

Solution

Make the clear filters button cancel next debounced search query

Reproduced.

However, I don't see how I could fix it. The Clear Filters button is outside the AutoSubmitFilterForm, and has no way of canceling the debounced event.

Fortunately, this really an edge case, the default debounce of 500 ms makes it rather unlikely to happen, and clicking the Clear Filters button a second time (or navigating away and back) fixes all issues.

docs/img/AutoSubmitFilterForm.mp4

djhi · 2024-11-20T15:27:28Z

packages/ra-core/src/util/useDebouncedEvent.ts

+    // Whenever callback or delay changes, we need to update the debounced callback
+    useLayoutEffect(() => {
+        debouncedCallbackRef.current = debounce(stableCallback, delay);
+    }, [stableCallback, delay]);


You don't need the stableCallback dependency anymore: it's a ref so it will never change

Ah it's because ESLint is not aware that stableCallback is a ref. NVM then

slax57 added 8 commits November 12, 2024 12:07

wip: first working version

55555fd

wip: own story and defaultValues

8c3bec7

wip: more stories and fix per input validation

8acaf9e

wip: story with ReferenceInput

612ebc1

wip: add resettable everywhere

c68e047

wip: add AsListActions story

8c8d011

introduce FilterListWrapper and make stories more beautiful

ae9faa0

move AutoSubmitFilterForm to ra-core

af34890

slax57 added the WIP Work In Progress label Nov 12, 2024

slax57 added 8 commits November 14, 2024 15:33

trigger debounce bug in stories

41780ed

fix and debug code

e1342d4

fix fix

159cfd8

remove useless useListParams stories

3b0cb94

apply fix to AutoSubmitFilterForm and remove tech debt from FilterForm

8164dfb

fix unit test

1512a1d

add missing FormGroupsProvider

0c136e3

refactor FilterLiveSearch to use AutoSubmitFilterForm + fix AutoSubmi…

719fc75

…tFilterForm to only submit on real changes

vercel bot deployed to Preview – react-admin-storybook November 14, 2024 17:39 View deployment

slax57 commented Nov 14, 2024

View reviewed changes

erwanMarmelab mentioned this pull request Nov 15, 2024

Fix FilterLiveSearch badly reset #10352

Closed

4 tasks

slax57 added 3 commits November 15, 2024 10:42

add FilterLiveSearch FullApp story

762b687

add filterlivesearch reapply old value bug unit test

5362261

use AutoSubmitFilterForm in FilterForm

47384c8

vercel bot deployed to Preview – react-admin-storybook November 15, 2024 11:07 View deployment

djhi reviewed Nov 15, 2024

View reviewed changes

fzaninotto reviewed Nov 15, 2024

View reviewed changes

packages/ra-ui-materialui/src/list/filter/FilterListWrapper.tsx Outdated Show resolved Hide resolved

slax57 added 4 commits November 15, 2024 14:37

improve AutoSubmitFilterForm component prop default value

5b58e7e

add tests for AutoSubmitFilterForm

b6b8eaf

add test for race condition bug

01c5429

remove debug code

e8005b1

slax57 added the RFR Ready For Review label Nov 15, 2024

vercel bot deployed to Preview – react-admin-storybook November 15, 2024 16:06 View deployment

fzaninotto requested a review from Copilot November 16, 2024 18:08

Copilot AI reviewed Nov 16, 2024

View reviewed changes

This comment was marked as resolved.

Sign in to view

djhi requested changes Nov 18, 2024

View reviewed changes

erwanMarmelab approved these changes Nov 18, 2024

View reviewed changes

fzaninotto requested changes Nov 18, 2024

View reviewed changes

slax57 added 6 commits November 20, 2024 14:01

code review

e34fa24

move AutoSubmitFilterForm in form folder

36d5a45

reintroduce unset

eeee44f

rename component to formComponent

fc30c5a

rename FilterListWrapper to FilterListSection

9f0ef08

rename AutoSubmitFilterForm to FilterLiveForm

05e926f

vercel bot deployed to Preview – react-admin-storybook November 20, 2024 13:34 View deployment

slax57 changed the title ~~Introduce <AutoSubmitFilterForm>~~ Introduce <FilterLiveForm> Nov 20, 2024

fzaninotto reviewed Nov 20, 2024

View reviewed changes

docs/img/AutoSubmitFilterForm.mp4 Outdated Show resolved Hide resolved

rename assets

7fd23c6

vercel bot deployed to Preview – react-admin-storybook November 20, 2024 14:02 View deployment

fzaninotto approved these changes Nov 20, 2024

View reviewed changes

djhi requested changes Nov 20, 2024

View reviewed changes

djhi approved these changes Nov 20, 2024

View reviewed changes

djhi merged commit 2ba1258 into next Nov 20, 2024
15 checks passed

djhi deleted the AutosubmitForm branch November 20, 2024 15:46

djhi added this to the 5.4.0 milestone Nov 20, 2024

lksstml mentioned this pull request Nov 21, 2024

List Filter Value "jumps" if you enter a new value without waiting for a response from the old #5591

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce `<FilterLiveForm>` #10344

Introduce `<FilterLiveForm>` #10344

slax57 commented Nov 12, 2024 •

edited by djhi

Loading

slax57 Nov 14, 2024

slax57 Nov 14, 2024

slax57 Nov 14, 2024

djhi Nov 15, 2024

slax57 Nov 15, 2024

This comment was marked as resolved.

djhi Nov 18, 2024

djhi Nov 18, 2024

djhi Nov 18, 2024

erwanMarmelab Nov 18, 2024

fzaninotto Nov 18, 2024

erwanMarmelab commented Nov 18, 2024

erwanMarmelab commented Nov 18, 2024

fzaninotto left a comment

fzaninotto left a comment

fzaninotto Nov 18, 2024

slax57 commented Nov 20, 2024

Problem

Solution

djhi Nov 20, 2024

djhi Nov 20, 2024

	The filter sidebar is not a form. Therefore, if your users need to enter complex filters, you'll have to recreate a filter form. This can easily be done thanks to the [`<AutoSubmitFilterForm>`](./AutoSubmitFilterForm.md) component. However, if you only need one text input with a filter-as-you-type behavior, you'll find the `<FilterLiveSearch>` component even more convenient.
	The filter sidebar is not a form. Therefore, if your users need to enter complex filters, you'll have to recreate a filter form. This can be done thanks to the [`<AutoSubmitFilterForm>`](./AutoSubmitFilterForm.md) component. However, if you only need one text input with a filter-as-you-type behavior, you'll find the `<FilterLiveSearch>` component even more convenient.

Introduce <FilterLiveForm> #10344

Introduce <FilterLiveForm> #10344

Conversation

slax57 commented Nov 12, 2024 • edited by djhi Loading

Problem

Solution

Todo

How To Test

Additional Checks

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as resolved.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

erwanMarmelab commented Nov 18, 2024

erwanMarmelab commented Nov 18, 2024

Problem

Solution

fzaninotto left a comment

Choose a reason for hiding this comment

fzaninotto left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

slax57 commented Nov 20, 2024

Problem

Solution

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Introduce `<FilterLiveForm>` #10344

Introduce `<FilterLiveForm>` #10344

slax57 commented Nov 12, 2024 •

edited by djhi

Loading