From a7444dee7e67e74fa620492a900475bf424509ce Mon Sep 17 00:00:00 2001 From: Luwangel Date: Tue, 28 Jul 2020 10:51:59 +0200 Subject: [PATCH 01/10] Fix the props used to memoize the DatagridHeaderCell --- .../ra-ui-materialui/src/list/DatagridHeaderCell.js | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/ra-ui-materialui/src/list/DatagridHeaderCell.js b/packages/ra-ui-materialui/src/list/DatagridHeaderCell.js index 15a9204d2a6..db214de76c7 100644 --- a/packages/ra-ui-materialui/src/list/DatagridHeaderCell.js +++ b/packages/ra-ui-materialui/src/list/DatagridHeaderCell.js @@ -36,6 +36,7 @@ export const DatagridHeaderCell = props => { } = props; const classes = useStyles(props); const translate = useTranslate(); + return ( +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) -); + ); +}); From 996edd33bfd260ac421097e2965ac81629c5eb15 Mon Sep 17 00:00:00 2001 From: Luwangel Date: Tue, 28 Jul 2020 12:23:39 +0200 Subject: [PATCH 02/10] Rename the data-sort to data-field in the DatagridHeaderCell to avoid confusion (sort should be an object including field) --- packages/ra-ui-materialui/src/list/Datagrid.tsx | 2 +- packages/ra-ui-materialui/src/list/DatagridHeaderCell.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/ra-ui-materialui/src/list/Datagrid.tsx b/packages/ra-ui-materialui/src/list/Datagrid.tsx index d3343bbed44..ca4f988204e 100644 --- a/packages/ra-ui-materialui/src/list/Datagrid.tsx +++ b/packages/ra-ui-materialui/src/list/Datagrid.tsx @@ -100,7 +100,7 @@ const Datagrid: FC = props => { event => { event.stopPropagation(); setSort( - event.currentTarget.dataset.sort, + event.currentTarget.dataset.field, event.currentTarget.dataset.order ); }, diff --git a/packages/ra-ui-materialui/src/list/DatagridHeaderCell.js b/packages/ra-ui-materialui/src/list/DatagridHeaderCell.js index db214de76c7..37c9335fa8e 100644 --- a/packages/ra-ui-materialui/src/list/DatagridHeaderCell.js +++ b/packages/ra-ui-materialui/src/list/DatagridHeaderCell.js @@ -61,7 +61,7 @@ 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-field={field.props.sortBy || field.props.source} data-order={field.props.sortByOrder || 'ASC'} onClick={updateSort} classes={classes} From 9da8c5f240000dfbfd6659723406467b25920405 Mon Sep 17 00:00:00 2001 From: Luwangel Date: Tue, 28 Jul 2020 12:26:04 +0200 Subject: [PATCH 03/10] Fix the updateSort method in the Datagrid to query the right order --- .../ra-ui-materialui/src/list/Datagrid.tsx | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/packages/ra-ui-materialui/src/list/Datagrid.tsx b/packages/ra-ui-materialui/src/list/Datagrid.tsx index ca4f988204e..2c95820106d 100644 --- a/packages/ra-ui-materialui/src/list/Datagrid.tsx +++ b/packages/ra-ui-materialui/src/list/Datagrid.tsx @@ -99,12 +99,14 @@ const Datagrid: FC = props => { const updateSort = useCallback( event => { event.stopPropagation(); - setSort( - event.currentTarget.dataset.field, - event.currentTarget.dataset.order - ); + const newField = event.currentTarget.dataset.field; + const newOrder = + currentSort.field === newField && currentSort.order === 'ASC' + ? 'DESC' + : 'ASC'; + setSort(newField, newOrder); }, - [setSort] + [currentSort.field, currentSort.order, setSort] ); const handleSelectAll = useCallback( @@ -197,8 +199,8 @@ const Datagrid: FC = props => { /> )} - {Children.map(children, (field, index) => - isValidElement(field) ? ( + {Children.map(children, (field, index) => { + return isValidElement(field) ? ( = props => { resource={resource} updateSort={updateSort} /> - ) : null - )} + ) : null; + })} {cloneElement( From 33f32c4d039b384c2c1cea327f9e7759267a063f Mon Sep 17 00:00:00 2001 From: Luwangel Date: Tue, 28 Jul 2020 12:29:00 +0200 Subject: [PATCH 04/10] Fix tests --- packages/ra-ui-materialui/src/list/DatagridHeaderCell.spec.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/ra-ui-materialui/src/list/DatagridHeaderCell.spec.js b/packages/ra-ui-materialui/src/list/DatagridHeaderCell.spec.js index 2d046b4e7c6..ab236d6bf82 100644 --- a/packages/ra-ui-materialui/src/list/DatagridHeaderCell.spec.js +++ b/packages/ra-ui-materialui/src/list/DatagridHeaderCell.spec.js @@ -47,7 +47,7 @@ describe('', () => { ); - 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', () => { @@ -64,7 +64,7 @@ describe('', () => { ); - 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', () => { From c94c789ddca1286a6fb56cd29ec717e18eeafbb7 Mon Sep 17 00:00:00 2001 From: Luwangel Date: Tue, 28 Jul 2020 14:48:01 +0200 Subject: [PATCH 05/10] Fix E2E selector for the ListPage sort by --- cypress/support/ListPage.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cypress/support/ListPage.js b/cypress/support/ListPage.js index 02f873b8024..ca39ffa9521 100644 --- a/cypress/support/ListPage.js +++ b/cypress/support/ListPage.js @@ -15,7 +15,7 @@ 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}`, logout: '.logout', From 0f51e1904f6b970f1c9e5ac6a9454ef9868fbd0d Mon Sep 17 00:00:00 2001 From: Luwangel Date: Tue, 28 Jul 2020 14:59:47 +0200 Subject: [PATCH 06/10] Fix E2E selector for the ListPage svg --- cypress/support/ListPage.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cypress/support/ListPage.js b/cypress/support/ListPage.js index ca39ffa9521..e9f1eac04c0 100644 --- a/cypress/support/ListPage.js +++ b/cypress/support/ListPage.js @@ -17,7 +17,7 @@ export default url => ({ datagridHeaders: 'th', 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: From 937dbb77af61394296b149f55305493a60e94b3e Mon Sep 17 00:00:00 2001 From: Luwangel Date: Wed, 29 Jul 2020 10:48:42 +0200 Subject: [PATCH 07/10] Keep both data-sort and data-field in DatagridHeaderCell to avoid breaking changes --- packages/ra-ui-materialui/src/list/DatagridHeaderCell.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/ra-ui-materialui/src/list/DatagridHeaderCell.js b/packages/ra-ui-materialui/src/list/DatagridHeaderCell.js index 37c9335fa8e..4c05587c6c1 100644 --- a/packages/ra-ui-materialui/src/list/DatagridHeaderCell.js +++ b/packages/ra-ui-materialui/src/list/DatagridHeaderCell.js @@ -61,6 +61,7 @@ export const DatagridHeaderCell = props => { (field.props.sortBy || field.props.source) } direction={currentSort.order === 'ASC' ? 'asc' : 'desc'} + 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} From 80cd4b0ac541152d3f2634c1ca6b26aa89b9431b Mon Sep 17 00:00:00 2001 From: Luwangel Date: Mon, 3 Aug 2020 17:22:09 +0200 Subject: [PATCH 08/10] Revert prettier format --- packages/ra-ui-materialui/src/list/Datagrid.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/ra-ui-materialui/src/list/Datagrid.tsx b/packages/ra-ui-materialui/src/list/Datagrid.tsx index 2c95820106d..137cf6521fd 100644 --- a/packages/ra-ui-materialui/src/list/Datagrid.tsx +++ b/packages/ra-ui-materialui/src/list/Datagrid.tsx @@ -199,8 +199,8 @@ const Datagrid: FC = props => { /> )} - {Children.map(children, (field, index) => { - return isValidElement(field) ? ( + {Children.map(children, (field, index) => + isValidElement(field) ? ( = props => { resource={resource} updateSort={updateSort} /> - ) : null; - })} + ) : null + )} {cloneElement( From d747b15cf85400ee224bc8c59f6f54c1a8efdd7a Mon Sep 17 00:00:00 2001 From: Luwangel Date: Mon, 3 Aug 2020 17:34:57 +0200 Subject: [PATCH 09/10] Use the order from the DataHeaderCell to update the Datagrid sort --- packages/ra-ui-materialui/src/list/Datagrid.tsx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/ra-ui-materialui/src/list/Datagrid.tsx b/packages/ra-ui-materialui/src/list/Datagrid.tsx index 137cf6521fd..553e9b61f91 100644 --- a/packages/ra-ui-materialui/src/list/Datagrid.tsx +++ b/packages/ra-ui-materialui/src/list/Datagrid.tsx @@ -101,12 +101,13 @@ const Datagrid: FC = props => { event.stopPropagation(); const newField = event.currentTarget.dataset.field; const newOrder = - currentSort.field === newField && currentSort.order === 'ASC' + currentSort.field === newField && + event.currentTarget.dataset.order === 'ASC' ? 'DESC' : 'ASC'; setSort(newField, newOrder); }, - [currentSort.field, currentSort.order, setSort] + [currentSort.field, setSort] ); const handleSelectAll = useCallback( From d635a3fde3d9a6d46a6b307bbca597cb739f1a98 Mon Sep 17 00:00:00 2001 From: Luwangel Date: Mon, 10 Aug 2020 10:48:23 +0200 Subject: [PATCH 10/10] Don't break the default sorting passed to the DataGridHeaderCell --- packages/ra-ui-materialui/src/list/Datagrid.tsx | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/ra-ui-materialui/src/list/Datagrid.tsx b/packages/ra-ui-materialui/src/list/Datagrid.tsx index 553e9b61f91..37fc93babc2 100644 --- a/packages/ra-ui-materialui/src/list/Datagrid.tsx +++ b/packages/ra-ui-materialui/src/list/Datagrid.tsx @@ -101,13 +101,15 @@ const Datagrid: FC = props => { event.stopPropagation(); const newField = event.currentTarget.dataset.field; const newOrder = - currentSort.field === newField && - event.currentTarget.dataset.order === 'ASC' - ? 'DESC' - : 'ASC'; + currentSort.field === newField + ? currentSort.order === 'ASC' + ? 'DESC' + : 'ASC' + : event.currentTarget.dataset.order; + setSort(newField, newOrder); }, - [currentSort.field, setSort] + [currentSort.field, currentSort.order, setSort] ); const handleSelectAll = useCallback(