From 5d74ca8867e9236865dc509bb83280b48962667e Mon Sep 17 00:00:00 2001 From: JulienM Date: Mon, 24 Feb 2020 10:50:13 +0100 Subject: [PATCH 1/5] Keep displayed filters under navigation, even if they are empty --- packages/ra-core/src/actions/listActions.ts | 1 + .../src/controller/useListController.ts | 2 +- .../ra-core/src/controller/useListParams.ts | 77 ++++++++++++------- .../admin/resource/list/queryReducer.ts | 5 ++ 4 files changed, 57 insertions(+), 28 deletions(-) diff --git a/packages/ra-core/src/actions/listActions.ts b/packages/ra-core/src/actions/listActions.ts index 653a22dc597..7586f5e37d3 100644 --- a/packages/ra-core/src/actions/listActions.ts +++ b/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 { diff --git a/packages/ra-core/src/controller/useListController.ts b/packages/ra-core/src/controller/useListController.ts index bac81a86097..750b6bff690 100644 --- a/packages/ra-core/src/controller/useListController.ts +++ b/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; diff --git a/packages/ra-core/src/controller/useListParams.ts b/packages/ra-core/src/controller/useListParams.ts index 218ad4fd88c..b090b750f0c 100644 --- a/packages/ra-core/src/controller/useListParams.ts +++ b/packages/ra-core/src/controller/useListParams.ts @@ -8,6 +8,7 @@ import { Location } from 'history'; import queryReducer, { SET_FILTER, + SET_DISPLAYEDFILTER, SET_PAGE, SET_PER_PAGE, SET_SORT, @@ -41,7 +42,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 +113,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 +150,7 @@ const useListParams = ({ search: `?${stringify({ ...newParams, filter: JSON.stringify(newParams.filter), + displayedFilters: JSON.stringify(newParams.displayedFilters), })}`, }); dispatch(changeListParams(resource, newParams)); @@ -174,43 +174,54 @@ const useListParams = ({ ); const filterValues = query.filter || emptyObject; + const displayedFilterValues = query.displayedFilters || emptyObject; const debouncedSetFilters = lodashDebounce( - newFilters => + (newFilters, newDisplayedFilters) => { changeParams({ type: SET_FILTER, payload: removeEmpty(newFilters), - }), + }); + if (newDisplayedFilters) { + const displayable = Object.keys(newDisplayedFilters).reduce( + (filters, filter) => { + if (newDisplayedFilters[filter]) + return { ...filters, [filter]: true }; + return { ...filters }; + }, + {} + ); + changeParams({ + type: SET_DISPLAYEDFILTER, + payload: displayable, + }); + } + }, 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 newDsplayedFilters = removeKey(displayedFilterValues, filterName); + setFilters(newFilters, newDsplayedFilters); }, 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 +238,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; }; diff --git a/packages/ra-core/src/reducer/admin/resource/list/queryReducer.ts b/packages/ra-core/src/reducer/admin/resource/list/queryReducer.ts index 2158e18ec2d..0ab1cd41824 100644 --- a/packages/ra-core/src/reducer/admin/resource/list/queryReducer.ts +++ b/packages/ra-core/src/reducer/admin/resource/list/queryReducer.ts @@ -8,6 +8,7 @@ export const SET_PAGE = 'SET_PAGE'; export const SET_PER_PAGE = 'SET_PER_PAGE'; export const SET_FILTER = 'SET_FILTER'; +export const SET_DISPLAYEDFILTER = 'SET_DISPLAYEDFILTER'; const oppositeOrder = direction => direction === SORT_DESC ? SORT_ASC : SORT_DESC; @@ -46,6 +47,10 @@ const queryReducer: Reducer = ( return { ...previousState, page: 1, filter: payload }; } + case SET_DISPLAYEDFILTER: { + return { ...previousState, page: 1, displayedFilters: payload }; + } + default: return previousState; } From 893d564980fab60d45cd9665c93f16cd35d4f6ae Mon Sep 17 00:00:00 2001 From: JulienM Date: Mon, 24 Feb 2020 14:46:53 +0100 Subject: [PATCH 2/5] Simplify actions --- .../ra-core/src/controller/useListParams.ts | 37 ++++++++++--------- .../admin/resource/list/queryReducer.ts | 14 ++++--- .../ra-ui-materialui/src/list/FilterButton.js | 4 +- .../ra-ui-materialui/src/list/FilterForm.js | 4 +- 4 files changed, 31 insertions(+), 28 deletions(-) diff --git a/packages/ra-core/src/controller/useListParams.ts b/packages/ra-core/src/controller/useListParams.ts index b090b750f0c..7ac52707dae 100644 --- a/packages/ra-core/src/controller/useListParams.ts +++ b/packages/ra-core/src/controller/useListParams.ts @@ -8,7 +8,6 @@ import { Location } from 'history'; import queryReducer, { SET_FILTER, - SET_DISPLAYEDFILTER, SET_PAGE, SET_PER_PAGE, SET_SORT, @@ -178,24 +177,23 @@ const useListParams = ({ const debouncedSetFilters = lodashDebounce( (newFilters, newDisplayedFilters) => { + let payload = { + filter: removeEmpty(newFilters), + displayedFilters: undefined, + }; + if (newDisplayedFilters) { + payload.displayedFilters = Object.keys( + newDisplayedFilters + ).reduce((filters, filter) => { + if (newDisplayedFilters[filter]) + return { ...filters, [filter]: true }; + return { ...filters }; + }, {}); + } changeParams({ type: SET_FILTER, - payload: removeEmpty(newFilters), + payload, }); - if (newDisplayedFilters) { - const displayable = Object.keys(newDisplayedFilters).reduce( - (filters, filter) => { - if (newDisplayedFilters[filter]) - return { ...filters, [filter]: true }; - return { ...filters }; - }, - {} - ); - changeParams({ - type: SET_DISPLAYEDFILTER, - payload: displayable, - }); - } }, debounce ); @@ -208,8 +206,11 @@ const useListParams = ({ const hideFilter = useCallback((filterName: string) => { const newFilters = removeKey(filterValues, filterName); - const newDsplayedFilters = removeKey(displayedFilterValues, filterName); - setFilters(newFilters, newDsplayedFilters); + const newDisplayedFilters = removeKey( + displayedFilterValues, + filterName + ); + setFilters(newFilters, newDisplayedFilters); }, requestSignature); // eslint-disable-line react-hooks/exhaustive-deps const showFilter = useCallback((filterName: string, defaultValue: any) => { diff --git a/packages/ra-core/src/reducer/admin/resource/list/queryReducer.ts b/packages/ra-core/src/reducer/admin/resource/list/queryReducer.ts index 0ab1cd41824..b629494429f 100644 --- a/packages/ra-core/src/reducer/admin/resource/list/queryReducer.ts +++ b/packages/ra-core/src/reducer/admin/resource/list/queryReducer.ts @@ -8,7 +8,6 @@ export const SET_PAGE = 'SET_PAGE'; export const SET_PER_PAGE = 'SET_PER_PAGE'; export const SET_FILTER = 'SET_FILTER'; -export const SET_DISPLAYEDFILTER = 'SET_DISPLAYEDFILTER'; const oppositeOrder = direction => direction === SORT_DESC ? SORT_ASC : SORT_DESC; @@ -44,11 +43,14 @@ const queryReducer: Reducer = ( return { ...previousState, page: 1, perPage: payload }; case SET_FILTER: { - return { ...previousState, page: 1, filter: payload }; - } - - case SET_DISPLAYEDFILTER: { - return { ...previousState, page: 1, displayedFilters: payload }; + return { + ...previousState, + page: 1, + filter: payload.filter, + displayedFilters: payload.displayedFilters + ? payload.displayedFilters + : previousState.displayedFilters, + }; } default: diff --git a/packages/ra-ui-materialui/src/list/FilterButton.js b/packages/ra-ui-materialui/src/list/FilterButton.js index 2b701e412f4..850bfdced6a 100644 --- a/packages/ra-ui-materialui/src/list/FilterButton.js +++ b/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, diff --git a/packages/ra-ui-materialui/src/list/FilterForm.js b/packages/ra-ui-materialui/src/list/FilterForm.js index 42dbcef65c7..a05eeeb9d23 100644 --- a/packages/ra-ui-materialui/src/list/FilterForm.js +++ b/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, From 5e28b4d3c17323f381b839c79ba45c9fe7108645 Mon Sep 17 00:00:00 2001 From: JulienM Date: Mon, 24 Feb 2020 15:02:05 +0100 Subject: [PATCH 3/5] write e2e test --- cypress/integration/list.js | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/cypress/integration/list.js b/cypress/integration/list.js index 44063fc53b6..62261f4abf6 100644 --- a/cypress/integration/list.js +++ b/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'); From 0bfc38a2fef6ca178b48eaadc16fa8a02daddb72 Mon Sep 17 00:00:00 2001 From: JulienM Date: Mon, 24 Feb 2020 15:10:14 +0100 Subject: [PATCH 4/5] add unit test --- .../admin/resource/list/queryReducer.spec.ts | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/packages/ra-core/src/reducer/admin/resource/list/queryReducer.spec.ts b/packages/ra-core/src/reducer/admin/resource/list/queryReducer.spec.ts index 8bc60b2d95c..1d5bd7475d4 100644 --- a/packages/ra-core/src/reducer/admin/resource/list/queryReducer.spec.ts +++ b/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 }, From f462b47aa6e7da6614f6480160eb27018b68fb10 Mon Sep 17 00:00:00 2001 From: Francois Zaninotto Date: Thu, 5 Mar 2020 17:34:55 +0100 Subject: [PATCH 5/5] avoid clone --- packages/ra-core/src/controller/useListParams.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/ra-core/src/controller/useListParams.ts b/packages/ra-core/src/controller/useListParams.ts index 7ac52707dae..4e9b7b0183c 100644 --- a/packages/ra-core/src/controller/useListParams.ts +++ b/packages/ra-core/src/controller/useListParams.ts @@ -185,9 +185,9 @@ const useListParams = ({ payload.displayedFilters = Object.keys( newDisplayedFilters ).reduce((filters, filter) => { - if (newDisplayedFilters[filter]) - return { ...filters, [filter]: true }; - return { ...filters }; + return newDisplayedFilters[filter] + ? { ...filters, [filter]: true } + : filters; }, {}); } changeParams({