Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 5 commits into from
Mar 5, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions cypress/integration/list.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
1 change: 1 addition & 0 deletions packages/ra-core/src/actions/listActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export interface ListParams {
page: number;
perPage: number;
filter: any;
displayedFilters: any;
}

export interface ChangeListParamsAction {
Expand Down
2 changes: 1 addition & 1 deletion packages/ra-core/src/controller/useListController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
80 changes: 52 additions & 28 deletions packages/ra-core/src/controller/useListParams.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -151,6 +149,7 @@ const useListParams = ({
search: `?${stringify({
...newParams,
filter: JSON.stringify(newParams.filter),
displayedFilters: JSON.stringify(newParams.displayedFilters),
})}`,
});
dispatch(changeListParams(resource, newParams));
Expand All @@ -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,
Expand All @@ -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;
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ describe('Query Reducer', () => {
{},
{
type: 'SET_FILTER',
payload: { title: 'foo' },
payload: { filter: { title: 'foo' } },
}
);
assert.deepEqual(updatedState.filter, { title: 'foo' });
Expand All @@ -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 },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
4 changes: 2 additions & 2 deletions packages/ra-ui-materialui/src/list/FilterButton.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ const useStyles = makeStyles(

const FilterButton = ({
filters,
displayedFilters,
displayedFilters = {},
filterValues,
showFilter,
classes: classesOverride,
Expand Down Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions packages/ra-ui-materialui/src/list/FilterForm.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ export const FilterForm = ({
margin,
variant,
filters,
displayedFilters,
displayedFilters = {},
hideFilter,
initialValues,
...rest
Expand Down Expand Up @@ -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,
Expand Down