Merge pull request #10697 from marmelab/fix-FilterLiveForm-RHF-7.55-part-2

Fix `<FilterLiveForm>` compatibility with react-hook-form 7.55.0, part 2

Author: djhi

packages/ra-core/src/form/FilterLiveForm.spec.tsx

@@ -1,12 +1,14 @@
-import { fireEvent, render, screen } from '@testing-library/react';
+import { fireEvent, render, screen, waitFor } from '@testing-library/react';
 import { getFilterFormValues } from './FilterLiveForm';
 import {
     Basic,
     GlobalValidation,
     MultipleFilterLiveForm,
+    MultipleFilterLiveFormOverlapping,
     MultipleInput,
     ParseFormat,
     PerInputValidation,
+    WithExternalChanges,
 } from './FilterLiveForm.stories';
 import React from 'react';
 import { WithFilterListSection } from '../../../ra-ui-materialui/src/list/filter/FilterLiveForm.stories';
@@ -163,6 +165,97 @@ describe('<FilterLiveForm />', () => {
         await screen.findByText('"has_newsletter": false', { exact: false });
     });
 
+    it('should not reapply old externally applied filters after clear', async () => {
+        render(<WithExternalChanges />);
+        // Set filter body: foo
+        fireEvent.change(await screen.findByLabelText('body'), {
+            target: { value: 'foo' },
+        });
+        fireEvent.click(await screen.findByText('Apply filter'));
+        await waitFor(() => {
+            expect(
+                JSON.parse(
+                    screen.queryByTestId('filter-values')?.textContent || ''
+                )
+            ).toEqual({
+                body: 'foo',
+            });
+        });
+        // Unmount
+        fireEvent.click(await screen.findByLabelText('Mount/unmount'));
+        await waitFor(() => {
+            expect(screen.queryByText('External list')).toBeNull();
+        });
+        // Mount
+        fireEvent.click(await screen.findByLabelText('Mount/unmount'));
+        await screen.findByText('External list');
+        expect(
+            JSON.parse(screen.queryByTestId('filter-values')?.textContent || '')
+        ).toEqual({
+            body: 'foo',
+        });
+        // Clear filters
+        fireEvent.click(await screen.findByText('Clear filters'));
+        await waitFor(() => {
+            expect(
+                JSON.parse(
+                    screen.queryByTestId('filter-values')?.textContent || ''
+                )
+            ).toEqual({});
+        });
+        // Wait for a bit
+        await new Promise(resolve => setTimeout(resolve, 510));
+        expect(
+            JSON.parse(screen.queryByTestId('filter-values')?.textContent || '')
+        ).toEqual({});
+    });
+
+    it('should not reapply old filter values when changing another FilterLiveForm', async () => {
+        render(<MultipleFilterLiveFormOverlapping />);
+        // Set first body input to foo
+        fireEvent.change((await screen.findAllByLabelText('body'))[0], {
+            target: { value: 'foo' },
+        });
+        await waitFor(() => {
+            expect(
+                JSON.parse(
+                    screen.queryByTestId('filter-values')?.textContent || ''
+                )
+            ).toEqual({
+                category: 'deals',
+                body: 'foo',
+            });
+        });
+        // Clear first body input
+        fireEvent.change((await screen.findAllByLabelText('body'))[0], {
+            target: { value: '' },
+        });
+        await waitFor(() => {
+            expect(
+                JSON.parse(
+                    screen.queryByTestId('filter-values')?.textContent || ''
+                )
+            ).toEqual({
+                category: 'deals',
+            });
+        });
+        // Change author input
+        fireEvent.change(await screen.findByLabelText('author'), {
+            target: { value: 'bar' },
+        });
+        await waitFor(() => {
+            expect(
+                JSON.parse(
+                    screen.queryByTestId('filter-values')?.textContent || ''
+                )
+            ).toEqual({
+                // body should not reappear
+                category: 'deals',
+                author: 'bar',
+            });
+        });
+    });
+
     describe('getFilterFormValues', () => {
         it('should correctly get the filter form values from the new filterValues', () => {
             const currentFormValues = {

packages/ra-core/src/form/FilterLiveForm.stories.tsx

@@ -13,7 +13,11 @@ import { useListContext } from '../controller/list/useListContext';
 
 export default { title: 'ra-core/form/FilterLiveForm' };
 
-const TextInput = ({ defaultValue = '', ...props }: InputProps) => {
+const TextInput = ({
+    defaultValue = '',
+    style,
+    ...props
+}: InputProps & { style?: React.CSSProperties }) => {
     const { field, fieldState } = useInput({ defaultValue, ...props });
     const { error } = fieldState;
 
@@ -24,12 +28,13 @@ const TextInput = ({ defaultValue = '', ...props }: InputProps) => {
                 display: 'flex',
                 flexDirection: 'column',
                 gap: '5px',
+                ...style,
             }}
         >
-            <label htmlFor={field.name} id={`id-${field.name}`}>
+            <label htmlFor={`id-${field.name}`}>
                 {props.label || field.name}
             </label>
-            <input {...field} aria-labelledby={`id-${field.name}`} />
+            <input {...field} id={`id-${field.name}`} />
             {error && (
                 <div style={{ color: 'red' }}>
                     {/* @ts-ignore */}
@@ -143,6 +148,31 @@ export const MultipleFilterLiveForm = () => {
     );
 };
 
+export const MultipleFilterLiveFormOverlapping = () => {
+    const listContext = useList({
+        data: [
+            { id: 1, title: 'Hello', has_newsletter: true },
+            { id: 2, title: 'World', has_newsletter: false },
+        ],
+        filter: {
+            category: 'deals',
+        },
+    });
+    return (
+        <ListContextProvider value={listContext}>
+            <FilterLiveForm>
+                <TextInput source="title" />
+                <TextInput source="body" />
+            </FilterLiveForm>
+            <FilterLiveForm>
+                <TextInput source="author" />
+                <TextInput source="body" />
+            </FilterLiveForm>
+            <FilterValue />
+        </ListContextProvider>
+    );
+};
+
 export const PerInputValidation = () => {
     const listContext = useList({
         data: [
@@ -206,3 +236,109 @@ const FilterValue = () => {
         </div>
     );
 };
+
+const ExternalList = () => {
+    const [body, setBody] = React.useState<string | undefined>(undefined);
+    const { filterValues, setFilters, data } = useListContext();
+    React.useEffect(() => {
+        setBody(filterValues.body);
+    }, [filterValues]);
+    const onChange = (event: React.ChangeEvent<HTMLInputElement>) => {
+        setBody(event.target.value);
+    };
+    const onApplyFilter = () => {
+        setFilters({ ...filterValues, body });
+    };
+    return (
+        <div
+            style={{
+                padding: '1em',
+                border: '2px solid gray',
+            }}
+        >
+            <p>External list</p>
+            <div
+                style={{
+                    display: 'flex',
+                    flexDirection: 'row',
+                    gap: '5px',
+                }}
+            >
+                <label htmlFor="id_body">body</label>
+                <input
+                    id="id_body"
+                    type="text"
+                    value={body || ''}
+                    onChange={onChange}
+                />
+                <button type="button" onClick={onApplyFilter}>
+                    Apply filter
+                </button>
+            </div>
+            {data.length ? (
+                <ul>
+                    {data.map(item => (
+                        <li key={item.id}>
+                            {item.id} - {item.title} - {item.body}
+                        </li>
+                    ))}
+                </ul>
+            ) : (
+                <p>No data</p>
+            )}
+        </div>
+    );
+};
+
+const ClearFiltersButton = () => {
+    const { setFilters } = useListContext();
+    return (
+        <div style={{ margin: '1em' }}>
+            <button
+                type="button"
+                onClick={() => {
+                    setFilters({});
+                }}
+            >
+                Clear filters
+            </button>
+        </div>
+    );
+};
+
+export const WithExternalChanges = () => {
+    const [mounted, setMounted] = React.useState(true);
+    const onToggle = () => {
+        setMounted(mounted => !mounted);
+    };
+    const listContext = useList({
+        data: [
+            { id: 1, title: 'hello', body: 'foo' },
+            { id: 2, title: 'world', body: 'bar' },
+        ],
+    });
+    return (
+        <div>
+            <input
+                id="id_mounted"
+                type="checkbox"
+                onChange={onToggle}
+                checked={mounted}
+            />
+            <label htmlFor="id_mounted">Mount/unmount</label>
+            {mounted && (
+                <ListContextProvider value={listContext}>
+                    <FilterLiveForm>
+                        <TextInput
+                            source="title"
+                            style={{ flexDirection: 'row' }}
+                        />
+                        <ClearFiltersButton />
+                    </FilterLiveForm>
+                    <ExternalList />
+                    <FilterValue />
+                </ListContextProvider>
+            )}
+        </div>
+    );
+};

packages/ra-core/src/form/FilterLiveForm.tsx

@@ -76,27 +76,16 @@ export const FilterLiveForm = (props: FilterLiveFormProps) => {
 
     const formContext = useForm({
         mode: 'onChange',
-        defaultValues: filterValues,
         resolver: finalResolver,
         ...rest,
     });
     const { handleSubmit, getValues, reset, watch, formState } = formContext;
     const { isValid } = formState;
 
-    // Ref tracking if there are internal changes pending, i.e. changes that
-    // should not trigger a reset
-    const formChangesPending = React.useRef(false);
-
     // Reapply filterValues when they change externally
     useEffect(() => {
         const newValues = getFilterFormValues(getValues(), filterValues);
         const previousValues = getValues();
-        if (formChangesPending.current) {
-            // The effect was triggered by a form change (i.e. internal change),
-            // so we don't need to reset the form
-            formChangesPending.current = false;
-            return;
-        }
         if (!isEqual(newValues, previousValues)) {
             reset(newValues);
         }
@@ -110,7 +99,6 @@ export const FilterLiveForm = (props: FilterLiveFormProps) => {
         if (!isValid) {
             return;
         }
-        formChangesPending.current = true;
         setFilters(mergeObjNotArray(filterValues, values));
     };
     const debouncedOnSubmit = useDebouncedEvent(onSubmit, debounce || 0);

packages/ra-ui-materialui/src/list/filter/FilterButton.spec.tsx

@@ -124,6 +124,63 @@ describe('<FilterButton />', () => {
             expect(checkboxes[2].getAttribute('aria-checked')).toBe('false');
         });
 
+        it('should remove the checked state of the menu item when removing its matching filter even when 2 filters were set', async () => {
+            render(<Basic />);
+
+            fireEvent.click(await screen.findByLabelText('Add filter'));
+            fireEvent.click(screen.getAllByRole('menuitemcheckbox')[0]);
+            await screen.findByRole('textbox', {
+                name: 'Title',
+            });
+
+            await screen.findByText('1-1 of 1');
+
+            fireEvent.click(await screen.findByLabelText('Add filter'));
+            fireEvent.click(screen.getAllByRole('menuitemcheckbox')[2]);
+            fireEvent.change(
+                await screen.findByRole('textbox', {
+                    name: 'Body',
+                }),
+                {
+                    target: { value: 'foo' },
+                }
+            );
+            await screen.findByText(
+                'No Posts found using the current filters.'
+            );
+
+            fireEvent.click(screen.getAllByTitle('Remove this filter')[1]);
+            await screen.findByText('1-1 of 1');
+
+            await waitFor(
+                () => {
+                    expect(
+                        screen.queryByRole('textbox', {
+                            name: 'Body',
+                        })
+                    ).toBeNull();
+                },
+                { timeout: 2000 }
+            );
+
+            // Wait for a bit
+            await new Promise(resolve => setTimeout(resolve, 510));
+
+            fireEvent.click(screen.getByTitle('Remove this filter'));
+            await screen.findByText('1-10 of 13');
+
+            await waitFor(
+                () => {
+                    expect(
+                        screen.queryByRole('textbox', {
+                            name: 'Title',
+                        })
+                    ).toBeNull();
+                },
+                { timeout: 2000 }
+            );
+        }, 10000);
+
         it('should display the filter button if all filters are shown and there is a filter value', () => {
             render(
                 <AdminContext theme={theme}>