Skip to content

Commit 8b164de

Browse files
djhidjhi
authored
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
2 parents 542b951 + ac56903 commit 8b164de

5 files changed

Lines changed: 296 additions & 20 deletions

File tree

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

Lines changed: 94 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,14 @@
1-
import { fireEvent, render, screen } from '@testing-library/react';
1+
import { fireEvent, render, screen, waitFor } from '@testing-library/react';
22
import { getFilterFormValues } from './FilterLiveForm';
33
import {
44
Basic,
55
GlobalValidation,
66
MultipleFilterLiveForm,
7+
MultipleFilterLiveFormOverlapping,
78
MultipleInput,
89
ParseFormat,
910
PerInputValidation,
11+
WithExternalChanges,
1012
} from './FilterLiveForm.stories';
1113
import React from 'react';
1214
import { WithFilterListSection } from '../../../ra-ui-materialui/src/list/filter/FilterLiveForm.stories';
@@ -163,6 +165,97 @@ describe('<FilterLiveForm />', () => {
163165
await screen.findByText('"has_newsletter": false', { exact: false });
164166
});
165167

168+
it('should not reapply old externally applied filters after clear', async () => {
169+
render(<WithExternalChanges />);
170+
// Set filter body: foo
171+
fireEvent.change(await screen.findByLabelText('body'), {
172+
target: { value: 'foo' },
173+
});
174+
fireEvent.click(await screen.findByText('Apply filter'));
175+
await waitFor(() => {
176+
expect(
177+
JSON.parse(
178+
screen.queryByTestId('filter-values')?.textContent || ''
179+
)
180+
).toEqual({
181+
body: 'foo',
182+
});
183+
});
184+
// Unmount
185+
fireEvent.click(await screen.findByLabelText('Mount/unmount'));
186+
await waitFor(() => {
187+
expect(screen.queryByText('External list')).toBeNull();
188+
});
189+
// Mount
190+
fireEvent.click(await screen.findByLabelText('Mount/unmount'));
191+
await screen.findByText('External list');
192+
expect(
193+
JSON.parse(screen.queryByTestId('filter-values')?.textContent || '')
194+
).toEqual({
195+
body: 'foo',
196+
});
197+
// Clear filters
198+
fireEvent.click(await screen.findByText('Clear filters'));
199+
await waitFor(() => {
200+
expect(
201+
JSON.parse(
202+
screen.queryByTestId('filter-values')?.textContent || ''
203+
)
204+
).toEqual({});
205+
});
206+
// Wait for a bit
207+
await new Promise(resolve => setTimeout(resolve, 510));
208+
expect(
209+
JSON.parse(screen.queryByTestId('filter-values')?.textContent || '')
210+
).toEqual({});
211+
});
212+
213+
it('should not reapply old filter values when changing another FilterLiveForm', async () => {
214+
render(<MultipleFilterLiveFormOverlapping />);
215+
// Set first body input to foo
216+
fireEvent.change((await screen.findAllByLabelText('body'))[0], {
217+
target: { value: 'foo' },
218+
});
219+
await waitFor(() => {
220+
expect(
221+
JSON.parse(
222+
screen.queryByTestId('filter-values')?.textContent || ''
223+
)
224+
).toEqual({
225+
category: 'deals',
226+
body: 'foo',
227+
});
228+
});
229+
// Clear first body input
230+
fireEvent.change((await screen.findAllByLabelText('body'))[0], {
231+
target: { value: '' },
232+
});
233+
await waitFor(() => {
234+
expect(
235+
JSON.parse(
236+
screen.queryByTestId('filter-values')?.textContent || ''
237+
)
238+
).toEqual({
239+
category: 'deals',
240+
});
241+
});
242+
// Change author input
243+
fireEvent.change(await screen.findByLabelText('author'), {
244+
target: { value: 'bar' },
245+
});
246+
await waitFor(() => {
247+
expect(
248+
JSON.parse(
249+
screen.queryByTestId('filter-values')?.textContent || ''
250+
)
251+
).toEqual({
252+
// body should not reappear
253+
category: 'deals',
254+
author: 'bar',
255+
});
256+
});
257+
});
258+
166259
describe('getFilterFormValues', () => {
167260
it('should correctly get the filter form values from the new filterValues', () => {
168261
const currentFormValues = {

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

Lines changed: 139 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,11 @@ import { useListContext } from '../controller/list/useListContext';
1313

1414
export default { title: 'ra-core/form/FilterLiveForm' };
1515

16-
const TextInput = ({ defaultValue = '', ...props }: InputProps) => {
16+
const TextInput = ({
17+
defaultValue = '',
18+
style,
19+
...props
20+
}: InputProps & { style?: React.CSSProperties }) => {
1721
const { field, fieldState } = useInput({ defaultValue, ...props });
1822
const { error } = fieldState;
1923

@@ -24,12 +28,13 @@ const TextInput = ({ defaultValue = '', ...props }: InputProps) => {
2428
display: 'flex',
2529
flexDirection: 'column',
2630
gap: '5px',
31+
...style,
2732
}}
2833
>
29-
<label htmlFor={field.name} id={`id-${field.name}`}>
34+
<label htmlFor={`id-${field.name}`}>
3035
{props.label || field.name}
3136
</label>
32-
<input {...field} aria-labelledby={`id-${field.name}`} />
37+
<input {...field} id={`id-${field.name}`} />
3338
{error && (
3439
<div style={{ color: 'red' }}>
3540
{/* @ts-ignore */}
@@ -143,6 +148,31 @@ export const MultipleFilterLiveForm = () => {
143148
);
144149
};
145150

151+
export const MultipleFilterLiveFormOverlapping = () => {
152+
const listContext = useList({
153+
data: [
154+
{ id: 1, title: 'Hello', has_newsletter: true },
155+
{ id: 2, title: 'World', has_newsletter: false },
156+
],
157+
filter: {
158+
category: 'deals',
159+
},
160+
});
161+
return (
162+
<ListContextProvider value={listContext}>
163+
<FilterLiveForm>
164+
<TextInput source="title" />
165+
<TextInput source="body" />
166+
</FilterLiveForm>
167+
<FilterLiveForm>
168+
<TextInput source="author" />
169+
<TextInput source="body" />
170+
</FilterLiveForm>
171+
<FilterValue />
172+
</ListContextProvider>
173+
);
174+
};
175+
146176
export const PerInputValidation = () => {
147177
const listContext = useList({
148178
data: [
@@ -206,3 +236,109 @@ const FilterValue = () => {
206236
</div>
207237
);
208238
};
239+
240+
const ExternalList = () => {
241+
const [body, setBody] = React.useState<string | undefined>(undefined);
242+
const { filterValues, setFilters, data } = useListContext();
243+
React.useEffect(() => {
244+
setBody(filterValues.body);
245+
}, [filterValues]);
246+
const onChange = (event: React.ChangeEvent<HTMLInputElement>) => {
247+
setBody(event.target.value);
248+
};
249+
const onApplyFilter = () => {
250+
setFilters({ ...filterValues, body });
251+
};
252+
return (
253+
<div
254+
style={{
255+
padding: '1em',
256+
border: '2px solid gray',
257+
}}
258+
>
259+
<p>External list</p>
260+
<div
261+
style={{
262+
display: 'flex',
263+
flexDirection: 'row',
264+
gap: '5px',
265+
}}
266+
>
267+
<label htmlFor="id_body">body</label>
268+
<input
269+
id="id_body"
270+
type="text"
271+
value={body || ''}
272+
onChange={onChange}
273+
/>
274+
<button type="button" onClick={onApplyFilter}>
275+
Apply filter
276+
</button>
277+
</div>
278+
{data.length ? (
279+
<ul>
280+
{data.map(item => (
281+
<li key={item.id}>
282+
{item.id} - {item.title} - {item.body}
283+
</li>
284+
))}
285+
</ul>
286+
) : (
287+
<p>No data</p>
288+
)}
289+
</div>
290+
);
291+
};
292+
293+
const ClearFiltersButton = () => {
294+
const { setFilters } = useListContext();
295+
return (
296+
<div style={{ margin: '1em' }}>
297+
<button
298+
type="button"
299+
onClick={() => {
300+
setFilters({});
301+
}}
302+
>
303+
Clear filters
304+
</button>
305+
</div>
306+
);
307+
};
308+
309+
export const WithExternalChanges = () => {
310+
const [mounted, setMounted] = React.useState(true);
311+
const onToggle = () => {
312+
setMounted(mounted => !mounted);
313+
};
314+
const listContext = useList({
315+
data: [
316+
{ id: 1, title: 'hello', body: 'foo' },
317+
{ id: 2, title: 'world', body: 'bar' },
318+
],
319+
});
320+
return (
321+
<div>
322+
<input
323+
id="id_mounted"
324+
type="checkbox"
325+
onChange={onToggle}
326+
checked={mounted}
327+
/>
328+
<label htmlFor="id_mounted">Mount/unmount</label>
329+
{mounted && (
330+
<ListContextProvider value={listContext}>
331+
<FilterLiveForm>
332+
<TextInput
333+
source="title"
334+
style={{ flexDirection: 'row' }}
335+
/>
336+
<ClearFiltersButton />
337+
</FilterLiveForm>
338+
<ExternalList />
339+
<FilterValue />
340+
</ListContextProvider>
341+
)}
342+
</div>
343+
);
344+
};

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

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -76,27 +76,16 @@ export const FilterLiveForm = (props: FilterLiveFormProps) => {
7676

7777
const formContext = useForm({
7878
mode: 'onChange',
79-
defaultValues: filterValues,
8079
resolver: finalResolver,
8180
...rest,
8281
});
8382
const { handleSubmit, getValues, reset, watch, formState } = formContext;
8483
const { isValid } = formState;
8584

86-
// Ref tracking if there are internal changes pending, i.e. changes that
87-
// should not trigger a reset
88-
const formChangesPending = React.useRef(false);
89-
9085
// Reapply filterValues when they change externally
9186
useEffect(() => {
9287
const newValues = getFilterFormValues(getValues(), filterValues);
9388
const previousValues = getValues();
94-
if (formChangesPending.current) {
95-
// The effect was triggered by a form change (i.e. internal change),
96-
// so we don't need to reset the form
97-
formChangesPending.current = false;
98-
return;
99-
}
10089
if (!isEqual(newValues, previousValues)) {
10190
reset(newValues);
10291
}
@@ -110,7 +99,6 @@ export const FilterLiveForm = (props: FilterLiveFormProps) => {
11099
if (!isValid) {
111100
return;
112101
}
113-
formChangesPending.current = true;
114102
setFilters(mergeObjNotArray(filterValues, values));
115103
};
116104
const debouncedOnSubmit = useDebouncedEvent(onSubmit, debounce || 0);

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

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,63 @@ describe('<FilterButton />', () => {
124124
expect(checkboxes[2].getAttribute('aria-checked')).toBe('false');
125125
});
126126

127+
it('should remove the checked state of the menu item when removing its matching filter even when 2 filters were set', async () => {
128+
render(<Basic />);
129+
130+
fireEvent.click(await screen.findByLabelText('Add filter'));
131+
fireEvent.click(screen.getAllByRole('menuitemcheckbox')[0]);
132+
await screen.findByRole('textbox', {
133+
name: 'Title',
134+
});
135+
136+
await screen.findByText('1-1 of 1');
137+
138+
fireEvent.click(await screen.findByLabelText('Add filter'));
139+
fireEvent.click(screen.getAllByRole('menuitemcheckbox')[2]);
140+
fireEvent.change(
141+
await screen.findByRole('textbox', {
142+
name: 'Body',
143+
}),
144+
{
145+
target: { value: 'foo' },
146+
}
147+
);
148+
await screen.findByText(
149+
'No Posts found using the current filters.'
150+
);
151+
152+
fireEvent.click(screen.getAllByTitle('Remove this filter')[1]);
153+
await screen.findByText('1-1 of 1');
154+
155+
await waitFor(
156+
() => {
157+
expect(
158+
screen.queryByRole('textbox', {
159+
name: 'Body',
160+
})
161+
).toBeNull();
162+
},
163+
{ timeout: 2000 }
164+
);
165+
166+
// Wait for a bit
167+
await new Promise(resolve => setTimeout(resolve, 510));
168+
169+
fireEvent.click(screen.getByTitle('Remove this filter'));
170+
await screen.findByText('1-10 of 13');
171+
172+
await waitFor(
173+
() => {
174+
expect(
175+
screen.queryByRole('textbox', {
176+
name: 'Title',
177+
})
178+
).toBeNull();
179+
},
180+
{ timeout: 2000 }
181+
);
182+
}, 10000);
183+
127184
it('should display the filter button if all filters are shown and there is a filter value', () => {
128185
render(
129186
<AdminContext theme={theme}>

0 commit comments

Comments
 (0)