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.

Sign up for GitHub

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

Jump to bottom

Fix sorting a list using a <Datagrid> inside a <ReferenceManyField> #5094

Merged
merged 10 commits into from
Aug 10, 2020
Merged

Fix sorting a list using a <Datagrid> inside a <ReferenceManyField> #5094

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
a7444de
Fix the props used to memoize the DatagridHeaderCell
Luwangel Jul 28, 2020
996edd3
Rename the data-sort to data-field in the DatagridHeaderCell to avoid…
Luwangel Jul 28, 2020
9da8c5f
Fix the updateSort method in the Datagrid to query the right order
Luwangel Jul 28, 2020
33f32c4
Fix tests
Luwangel Jul 28, 2020
c94c789
Fix E2E selector for the ListPage sort by
Luwangel Jul 28, 2020
0f51e19
Fix E2E selector for the ListPage svg
Luwangel Jul 28, 2020
937dbb7
Keep both data-sort and data-field in DatagridHeaderCell to avoid bre…
Luwangel Jul 29, 2020
80cd4b0
Revert prettier format
Luwangel Aug 3, 2020
d747b15
Use the order from the DataHeaderCell to update the Datagrid sort
Luwangel Aug 3, 2020
d635a3f
Don't break the default sorting passed to the DataGridHeaderCell
Luwangel Aug 10, 2020
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
4 changes: 2 additions & 2 deletions cypress/support/ListPage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ export default url => ({
recordRows: '.datagrid-body tr',
viewsColumn: '.datagrid-body tr td:nth-child(7)',
datagridHeaders: 'th',
sortBy: name => `th span[data-sort="${name}"]`,
sortBy: name => `th span[data-field="${name}"]`,
svg: (name, criteria = '') =>
`th span[data-sort="${name}"] svg${criteria}`,
`th span[data-field="${name}"] svg${criteria}`,
logout: '.logout',
bulkActionsToolbar: '[data-test=bulk-actions-toolbar]',
customBulkActionsButton:
Expand Down
15 changes: 10 additions & 5 deletions packages/ra-ui-materialui/src/list/Datagrid.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,12 +99,17 @@ const Datagrid: FC<DatagridProps> = props => {
const updateSort = useCallback(
event => {
event.stopPropagation();
setSort(
event.currentTarget.dataset.sort,
event.currentTarget.dataset.order
);
const newField = event.currentTarget.dataset.field;
const newOrder =
currentSort.field === newField
? currentSort.order === 'ASC'
? 'DESC'
: 'ASC'
: event.currentTarget.dataset.order;

setSort(newField, newOrder);
},
[setSort]
[currentSort.field, currentSort.order, setSort]
);

const handleSelectAll = useCallback(
Expand Down
14 changes: 8 additions & 6 deletions packages/ra-ui-materialui/src/list/DatagridHeaderCell.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export const DatagridHeaderCell = props => {
} = props;
const classes = useStyles(props);
const translate = useTranslate();

return (
<TableCell
className={classnames(className, field.props.headerClassName)}
Expand All @@ -60,7 +61,8 @@ export const DatagridHeaderCell = props => {
(field.props.sortBy || field.props.source)
}
direction={currentSort.order === 'ASC' ? 'asc' : 'desc'}
data-sort={field.props.sortBy || field.props.source}
data-sort={field.props.sortBy || field.props.source} // @deprecated. Use data-field instead.
data-field={field.props.sortBy || field.props.source}
data-order={field.props.sortByOrder || 'ASC'}
onClick={updateSort}
classes={classes}
Expand Down Expand Up @@ -97,11 +99,11 @@ DatagridHeaderCell.propTypes = {
updateSort: PropTypes.func.isRequired,
};

export default memo(
DatagridHeaderCell,
(props, nextProps) =>
export default memo(DatagridHeaderCell, (props, nextProps) => {
return (
props.updateSort === nextProps.updateSort &&
props.currentSort.sort === nextProps.currentSort.sort &&
props.currentSort.field === nextProps.currentSort.field &&
props.currentSort.order === nextProps.currentSort.order &&
!(nextProps.isSorting && props.sortable !== nextProps.sortable)
);
);
});
4 changes: 2 additions & 2 deletions packages/ra-ui-materialui/src/list/DatagridHeaderCell.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ describe('<DatagridHeaderCell />', () => {
</tbody>
</table>
);
expect(getByTitle('ra.action.sort').dataset.sort).toBe('title');
expect(getByTitle('ra.action.sort').dataset.field).toBe('title');
});

it('should be enabled when field has a sortBy props', () => {
Expand All @@ -64,7 +64,7 @@ describe('<DatagridHeaderCell />', () => {
</tbody>
</table>
);
expect(getByTitle('ra.action.sort').dataset.sort).toBe('title');
expect(getByTitle('ra.action.sort').dataset.field).toBe('title');
});

it('should be change order when field has a sortByOrder props', () => {
Expand Down