Merge pull request #4442 from marmelab/keep-displayed-filters

Fix to keep displayed filters through navigation, even if they are empty

cypress/integration/list.js

@@ -96,6 +96,27 @@ describe('List Page', () => {
             ListPagePosts.setFilterValue('q', '');
         });
 
+        it('should keep added filters when emptying it after navigating away and back', () => {
+            ListPagePosts.logout();
+            LoginPage.login('admin', 'password');
+            ListPagePosts.showFilter('title');
+            ListPagePosts.setFilterValue('title', 'quis culpa impedit');
+            cy.contains('1-1 of 1');
+
+            cy.get('[href="#/users"]').click();
+
+            cy.get('[href="#/posts"]').click();
+
+            cy.get(ListPagePosts.elements.filter('title')).should(el =>
+                expect(el).to.have.value('quis culpa impedit')
+            );
+            ListPagePosts.setFilterValue('title', '');
+            ListPagePosts.waitUntilDataLoaded();
+            cy.get(ListPagePosts.elements.filter('title')).should(
+                el => expect(el).to.exist
+            );
+        });
+
         it('should allow to disable alwaysOn filters with default value', () => {
             ListPagePosts.logout();
             LoginPage.login('admin', 'password');

packages/ra-core/src/actions/listActions.ts

@@ -8,6 +8,7 @@ export interface ListParams {
     page: number;
     perPage: number;
     filter: any;
+    displayedFilters: any;
 }
 
 export interface ChangeListParamsAction {

packages/ra-core/src/controller/useListController.ts

@@ -63,7 +63,7 @@ export interface ListControllerProps {
     perPage: number;
     resource: string;
     selectedIds: Identifier[];
-    setFilters: (filters: any) => void;
+    setFilters: (filters: any, displayedFilters: any) => void;
     setPage: (page: number) => void;
     setPerPage: (page: number) => void;
     setSort: (sort: string) => void;

packages/ra-core/src/controller/useListParams.ts

@@ -41,7 +41,7 @@ interface Modifiers {
     setPage: (page: number) => void;
     setPerPage: (pageSize: number) => void;
     setSort: (sort: string) => void;
-    setFilters: (filters: any) => void;
+    setFilters: (filters: any, displayedFilters: any) => void;
     hideFilter: (filterName: string) => void;
     showFilter: (filterName: string, defaultValue: any) => void;
 }
@@ -112,10 +112,8 @@ const useListParams = ({
     perPage = 10,
     debounce = 500,
 }: ListParamsOptions): [Parameters, Modifiers] => {
-    const [displayedFilters, setDisplayedFilters] = useState({});
     const dispatch = useDispatch();
     const history = useHistory();
-
     const params = useSelector(
         (reduxState: ReduxState) =>
             reduxState.admin.resources[resource]
@@ -151,6 +149,7 @@ const useListParams = ({
             search: `?${stringify({
                 ...newParams,
                 filter: JSON.stringify(newParams.filter),
+                displayedFilters: JSON.stringify(newParams.displayedFilters),
             })}`,
         });
         dispatch(changeListParams(resource, newParams));
@@ -174,43 +173,56 @@ const useListParams = ({
     );
 
     const filterValues = query.filter || emptyObject;
+    const displayedFilterValues = query.displayedFilters || emptyObject;
 
     const debouncedSetFilters = lodashDebounce(
-        newFilters =>
+        (newFilters, newDisplayedFilters) => {
+            let payload = {
+                filter: removeEmpty(newFilters),
+                displayedFilters: undefined,
+            };
+            if (newDisplayedFilters) {
+                payload.displayedFilters = Object.keys(
+                    newDisplayedFilters
+                ).reduce((filters, filter) => {
+                    return newDisplayedFilters[filter]
+                        ? { ...filters, [filter]: true }
+                        : filters;
+                }, {});
+            }
             changeParams({
                 type: SET_FILTER,
-                payload: removeEmpty(newFilters),
-            }),
+                payload,
+            });
+        },
         debounce
     );
 
     const setFilters = useCallback(
-        filters => debouncedSetFilters(filters),
+        (filters, displayedFilters) =>
+            debouncedSetFilters(filters, displayedFilters),
         requestSignature // eslint-disable-line react-hooks/exhaustive-deps
     );
 
     const hideFilter = useCallback((filterName: string) => {
-        setDisplayedFilters(previousFilters => ({
-            ...previousFilters,
-            [filterName]: false,
-        }));
         const newFilters = removeKey(filterValues, filterName);
-        setFilters(newFilters);
+        const newDisplayedFilters = removeKey(
+            displayedFilterValues,
+            filterName
+        );
+        setFilters(newFilters, newDisplayedFilters);
     }, requestSignature); // eslint-disable-line react-hooks/exhaustive-deps
 
     const showFilter = useCallback((filterName: string, defaultValue: any) => {
-        setDisplayedFilters(previousFilters => ({
-            ...previousFilters,
-            [filterName]: true,
-        }));
-        if (typeof defaultValue !== 'undefined') {
-            setFilters(set(filterValues, filterName, defaultValue));
-        }
+        setFilters(
+            set(filterValues, filterName, defaultValue),
+            set(displayedFilterValues, filterName, true)
+        );
     }, requestSignature); // eslint-disable-line react-hooks/exhaustive-deps
 
     return [
         {
-            displayedFilters,
+            displayedFilters: displayedFilterValues,
             filterValues,
             requestSignature,
             ...query,
@@ -227,20 +239,32 @@ const useListParams = ({
     ];
 };
 
-export const validQueryParams = ['page', 'perPage', 'sort', 'order', 'filter'];
+export const validQueryParams = [
+    'page',
+    'perPage',
+    'sort',
+    'order',
+    'filter',
+    'displayedFilters',
+];
+
+const parseObject = (query, field) => {
+    if (query[field] && typeof query[field] === 'string') {
+        try {
+            query[field] = JSON.parse(query[field]);
+        } catch (err) {
+            delete query[field];
+        }
+    }
+};
 
 export const parseQueryFromLocation = ({ search }) => {
     const query = pickBy(
         parse(search),
         (v, k) => validQueryParams.indexOf(k) !== -1
     );
-    if (query.filter && typeof query.filter === 'string') {
-        try {
-            query.filter = JSON.parse(query.filter);
-        } catch (err) {
-            delete query.filter;
-        }
-    }
+    parseObject(query, 'filter');
+    parseObject(query, 'displayedFilters');
     return query;
 };
 

packages/ra-core/src/reducer/admin/resource/list/queryReducer.spec.ts

@@ -36,7 +36,7 @@ describe('Query Reducer', () => {
                 {},
                 {
                     type: 'SET_FILTER',
-                    payload: { title: 'foo' },
+                    payload: { filter: { title: 'foo' } },
                 }
             );
             assert.deepEqual(updatedState.filter, { title: 'foo' });
@@ -51,13 +51,28 @@ describe('Query Reducer', () => {
                 },
                 {
                     type: 'SET_FILTER',
-                    payload: { title: 'bar' },
+                    payload: { filter: { title: 'bar' } },
                 }
             );
 
             assert.deepEqual(updatedState.filter, { title: 'bar' });
         });
 
+        it('should add new filter and displayedFilter with given value when set', () => {
+            const updatedState = queryReducer(
+                {},
+                {
+                    type: 'SET_FILTER',
+                    payload: {
+                        filter: { title: 'foo' },
+                        displayedFilters: { title: true },
+                    },
+                }
+            );
+            assert.deepEqual(updatedState.filter, { title: 'foo' });
+            assert.deepEqual(updatedState.displayedFilters, { title: true });
+        });
+
         it('should reset page to 1', () => {
             const updatedState = queryReducer(
                 { page: 3 },

packages/ra-core/src/reducer/admin/resource/list/queryReducer.ts

@@ -43,7 +43,14 @@ const queryReducer: Reducer<ListParams> = (
             return { ...previousState, page: 1, perPage: payload };
 
         case SET_FILTER: {
-            return { ...previousState, page: 1, filter: payload };
+            return {
+                ...previousState,
+                page: 1,
+                filter: payload.filter,
+                displayedFilters: payload.displayedFilters
+                    ? payload.displayedFilters
+                    : previousState.displayedFilters,
+            };
         }
 
         default:

packages/ra-ui-materialui/src/list/FilterButton.js

@@ -18,7 +18,7 @@ const useStyles = makeStyles(
 
 const FilterButton = ({
     filters,
-    displayedFilters,
+    displayedFilters = {},
     filterValues,
     showFilter,
     classes: classesOverride,
@@ -91,7 +91,7 @@ const FilterButton = ({
 FilterButton.propTypes = {
     resource: PropTypes.string.isRequired,
     filters: PropTypes.arrayOf(PropTypes.node).isRequired,
-    displayedFilters: PropTypes.object.isRequired,
+    displayedFilters: PropTypes.object,
     filterValues: PropTypes.object.isRequired,
     showFilter: PropTypes.func.isRequired,
     classes: PropTypes.object,

packages/ra-ui-materialui/src/list/FilterForm.js

@@ -89,7 +89,7 @@ export const FilterForm = ({
     margin,
     variant,
     filters,
-    displayedFilters,
+    displayedFilters = {},
     hideFilter,
     initialValues,
     ...rest
@@ -147,7 +147,7 @@ const handleSubmit = event => {
 FilterForm.propTypes = {
     resource: PropTypes.string.isRequired,
     filters: PropTypes.arrayOf(PropTypes.node).isRequired,
-    displayedFilters: PropTypes.object.isRequired,
+    displayedFilters: PropTypes.object,
     hideFilter: PropTypes.func.isRequired,
     initialValues: PropTypes.object,
     classes: PropTypes.object,