From 146dd75dd93aa0a2cb25b3240273b2d2760af4cc Mon Sep 17 00:00:00 2001 From: Gildas Garcia Date: Mon, 2 Sep 2019 10:54:00 +0200 Subject: [PATCH 1/7] Fix Datagrid can't handle dynamic children Fixes #3631 and #3629 --- .../ra-ui-materialui/src/list/DatagridBody.js | 77 ++++++++----------- .../ra-ui-materialui/src/list/DatagridRow.js | 9 +-- 2 files changed, 35 insertions(+), 51 deletions(-) diff --git a/packages/ra-ui-materialui/src/list/DatagridBody.js b/packages/ra-ui-materialui/src/list/DatagridBody.js index 125515f0058..6afe7001518 100644 --- a/packages/ra-ui-materialui/src/list/DatagridBody.js +++ b/packages/ra-ui-materialui/src/list/DatagridBody.js @@ -1,4 +1,4 @@ -import React, { cloneElement, useMemo } from 'react'; +import React, { cloneElement, memo } from 'react'; import PropTypes from 'prop-types'; import TableBody from '@material-ui/core/TableBody'; import classnames from 'classnames'; @@ -24,45 +24,36 @@ const DatagridBody = ({ styles, version, ...rest -}) => - useMemo( - () => ( - - {ids.map((id, rowIndex) => - cloneElement( - row, - { - basePath, - classes, - className: classnames(classes.row, { - [classes.rowEven]: rowIndex % 2 === 0, - [classes.rowOdd]: rowIndex % 2 !== 0, - [classes.clickableRow]: rowClick, - }), - expand, - hasBulkActions, - hover, - id, - key: id, - onToggleItem, - record: data[id], - resource, - rowClick, - selected: selectedIds.includes(id), - style: rowStyle - ? rowStyle(data[id], rowIndex) - : null, - }, - children - ) - )} - - ), - [version, data, selectedIds, JSON.stringify(ids), hasBulkActions] // eslint-disable-line - ); +}) => ( + + {ids.map((id, rowIndex) => + cloneElement( + row, + { + basePath, + classes, + className: classnames(classes.row, { + [classes.rowEven]: rowIndex % 2 === 0, + [classes.rowOdd]: rowIndex % 2 !== 0, + [classes.clickableRow]: rowClick, + }), + expand, + hasBulkActions, + hover, + id, + key: id, + onToggleItem, + record: data[id], + resource, + rowClick, + selected: selectedIds.includes(id), + style: rowStyle ? rowStyle(data[id], rowIndex) : null, + }, + children + ) + )} + +); DatagridBody.propTypes = { basePath: PropTypes.string, @@ -91,7 +82,7 @@ DatagridBody.defaultProps = { row: , }; +const MemoDatagridBody = memo(DatagridBody); // trick material-ui Table into thinking this is one of the child type it supports -DatagridBody.muiName = 'TableBody'; - -export default DatagridBody; +MemoDatagridBody.muiName = 'TableBody'; +export default MemoDatagridBody; diff --git a/packages/ra-ui-materialui/src/list/DatagridRow.js b/packages/ra-ui-materialui/src/list/DatagridRow.js index e57abeb81dd..c14b683ea79 100644 --- a/packages/ra-ui-materialui/src/list/DatagridRow.js +++ b/packages/ra-ui-materialui/src/list/DatagridRow.js @@ -8,7 +8,6 @@ import React, { useCallback, memo, } from 'react'; -import isEqual from 'lodash/isEqual'; import PropTypes from 'prop-types'; import classnames from 'classnames'; import { useDispatch } from 'react-redux'; @@ -207,13 +206,7 @@ DatagridRow.defaultProps = { selected: false, }; -const areEqual = (prevProps, nextProps) => { - const { children: _, ...prevPropsWithoutChildren } = prevProps; - const { children: __, ...nextPropsWithoutChildren } = nextProps; - return isEqual(prevPropsWithoutChildren, nextPropsWithoutChildren); -}; - -const PureDatagridRow = memo(DatagridRow, areEqual); +const PureDatagridRow = memo(DatagridRow); PureDatagridRow.displayName = 'PureDatagridRow'; From a88ddc363f37d4d80ee983abc7b262eefaeb6d59 Mon Sep 17 00:00:00 2001 From: Gildas Garcia Date: Mon, 7 Oct 2019 18:40:15 +0200 Subject: [PATCH 2/7] Review --- packages/ra-ui-materialui/src/list/Datagrid.js | 6 +++--- packages/ra-ui-materialui/src/list/DatagridBody.js | 8 ++++++-- packages/ra-ui-materialui/src/list/DatagridRow.js | 11 +++++++++-- 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/packages/ra-ui-materialui/src/list/Datagrid.js b/packages/ra-ui-materialui/src/list/Datagrid.js index 0d576fbb7a7..f25280afe6f 100644 --- a/packages/ra-ui-materialui/src/list/Datagrid.js +++ b/packages/ra-ui-materialui/src/list/Datagrid.js @@ -15,8 +15,8 @@ import Checkbox from '@material-ui/core/Checkbox'; import classnames from 'classnames'; import DatagridHeaderCell from './DatagridHeaderCell'; -import DatagridBody from './DatagridBody'; import DatagridLoading from './DatagridLoading'; +import MemoDatagridBody, { DatagridBody } from './DatagridBody'; const useStyles = makeStyles(theme => ({ table: { @@ -103,7 +103,8 @@ function Datagrid({ classes: classesOverride, ...props }) { const classes = useStyles({ classes: classesOverride }); const { basePath, - body, + optimized = false, + body = optimized ? : , children, className, currentSort, @@ -289,7 +290,6 @@ Datagrid.defaultProps = { hasBulkActions: false, ids: [], selectedIds: [], - body: , }; export default Datagrid; diff --git a/packages/ra-ui-materialui/src/list/DatagridBody.js b/packages/ra-ui-materialui/src/list/DatagridBody.js index 6afe7001518..b8512ae7087 100644 --- a/packages/ra-ui-materialui/src/list/DatagridBody.js +++ b/packages/ra-ui-materialui/src/list/DatagridBody.js @@ -3,9 +3,9 @@ import PropTypes from 'prop-types'; import TableBody from '@material-ui/core/TableBody'; import classnames from 'classnames'; -import DatagridRow from './DatagridRow'; +import PureDatagridRow, { DatagridRow } from './DatagridRow'; -const DatagridBody = ({ +export const DatagridBody = ({ basePath, children, classes, @@ -85,4 +85,8 @@ DatagridBody.defaultProps = { const MemoDatagridBody = memo(DatagridBody); // trick material-ui Table into thinking this is one of the child type it supports MemoDatagridBody.muiName = 'TableBody'; +MemoDatagridBody.defaultProps = { + row: , +}; + export default MemoDatagridBody; diff --git a/packages/ra-ui-materialui/src/list/DatagridRow.js b/packages/ra-ui-materialui/src/list/DatagridRow.js index c14b683ea79..a0d2a169b8a 100644 --- a/packages/ra-ui-materialui/src/list/DatagridRow.js +++ b/packages/ra-ui-materialui/src/list/DatagridRow.js @@ -14,6 +14,7 @@ import { useDispatch } from 'react-redux'; import { push } from 'connected-react-router'; import { TableCell, TableRow, Checkbox } from '@material-ui/core'; import { linkToRecord } from 'ra-core'; +import isEqual from 'lodash/isEqual'; import DatagridCell from './DatagridCell'; import ExpandRowButton from './ExpandRowButton'; @@ -27,7 +28,7 @@ const computeNbColumns = (expand, children, hasBulkActions) => const defaultClasses = {}; -const DatagridRow = ({ +export const DatagridRow = ({ basePath, children, classes = defaultClasses, @@ -206,7 +207,13 @@ DatagridRow.defaultProps = { selected: false, }; -const PureDatagridRow = memo(DatagridRow); +const areEqual = (prevProps, nextProps) => { + const { children: _, ...prevPropsWithoutChildren } = prevProps; + const { children: __, ...nextPropsWithoutChildren } = nextProps; + return isEqual(prevPropsWithoutChildren, nextPropsWithoutChildren); +}; + +const PureDatagridRow = memo(DatagridRow, areEqual); PureDatagridRow.displayName = 'PureDatagridRow'; From 1287ed5fc61e506293076f6c7d0d60dd87345942 Mon Sep 17 00:00:00 2001 From: Gildas Garcia Date: Tue, 8 Oct 2019 10:23:20 +0200 Subject: [PATCH 3/7] Documentation --- docs/List.md | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/docs/List.md b/docs/List.md index 6275346c6e4..17b9c31ecf5 100644 --- a/docs/List.md +++ b/docs/List.md @@ -1026,6 +1026,29 @@ const PostList = ({ classes, ...props }) => ( export default withStyles(styles)(PostList); ``` +### Performances + +when displaying large pages of data, you might experience some performance issues. +This is mostly due to the fact that we iterate over the `` children and clone them. + +In such cases, you can opt-in for an optimized version of the `` by setting its `optimized` prop to `true`. +Be aware that you can't have dynamic children, such as those displayed or hidden by checking permissions, when using this mode. + +```jsx +const PostList = props => ( + + + + + + + +); + +export default withStyles(styles)(PostList); +``` + + ## The `` component For mobile devices, a `` is often unusable - there is simply not enough space to display several columns. The convention in that case is to use a simple list, with only one column per row. The `` component serves that purpose, leveraging [material-ui's `` and `` components](https://material-ui.com/demos/lists/). You can use it as `` or `` child: From f1621b95b2c98052a2f03050ec06f5f0d5a67925 Mon Sep 17 00:00:00 2001 From: Francois Zaninotto Date: Tue, 8 Oct 2019 15:36:01 +0200 Subject: [PATCH 4/7] Fix typo --- docs/List.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/List.md b/docs/List.md index 17b9c31ecf5..9ba682ffe68 100644 --- a/docs/List.md +++ b/docs/List.md @@ -1026,7 +1026,7 @@ const PostList = ({ classes, ...props }) => ( export default withStyles(styles)(PostList); ``` -### Performances +### Performance when displaying large pages of data, you might experience some performance issues. This is mostly due to the fact that we iterate over the `` children and clone them. From 87cc3be9a01a09fe04f7a35ec08f75d87a1d18e5 Mon Sep 17 00:00:00 2001 From: Gildas Garcia Date: Thu, 10 Oct 2019 09:50:08 +0200 Subject: [PATCH 5/7] Review --- packages/ra-ui-materialui/src/list/DatagridBody.js | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/packages/ra-ui-materialui/src/list/DatagridBody.js b/packages/ra-ui-materialui/src/list/DatagridBody.js index b8512ae7087..ab1446af593 100644 --- a/packages/ra-ui-materialui/src/list/DatagridBody.js +++ b/packages/ra-ui-materialui/src/list/DatagridBody.js @@ -2,6 +2,7 @@ import React, { cloneElement, memo } from 'react'; import PropTypes from 'prop-types'; import TableBody from '@material-ui/core/TableBody'; import classnames from 'classnames'; +import isEqual from 'lodash/isEqual'; import PureDatagridRow, { DatagridRow } from './DatagridRow'; @@ -81,8 +82,16 @@ DatagridBody.defaultProps = { ids: [], row: , }; +// trick material-ui Table into thinking this is one of the child type it supports +DatagridBody.muiName = 'TableBody'; + +const areEqual = (prevProps, nextProps) => { + const { children: _, ...prevPropsWithoutChildren } = prevProps; + const { children: __, ...nextPropsWithoutChildren } = nextProps; + return isEqual(prevPropsWithoutChildren, nextPropsWithoutChildren); +}; -const MemoDatagridBody = memo(DatagridBody); +const MemoDatagridBody = memo(DatagridBody, areEqual); // trick material-ui Table into thinking this is one of the child type it supports MemoDatagridBody.muiName = 'TableBody'; MemoDatagridBody.defaultProps = { From 2a961196fa500e8fdf8b2d1b47a1ca8be6629a54 Mon Sep 17 00:00:00 2001 From: Gildas Garcia Date: Mon, 14 Oct 2019 16:22:15 +0200 Subject: [PATCH 6/7] Review --- packages/ra-ui-materialui/src/list/Datagrid.js | 2 +- packages/ra-ui-materialui/src/list/DatagridBody.js | 8 ++++---- packages/ra-ui-materialui/src/list/DatagridRow.js | 6 +++--- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/ra-ui-materialui/src/list/Datagrid.js b/packages/ra-ui-materialui/src/list/Datagrid.js index f25280afe6f..00110721e29 100644 --- a/packages/ra-ui-materialui/src/list/Datagrid.js +++ b/packages/ra-ui-materialui/src/list/Datagrid.js @@ -16,7 +16,7 @@ import classnames from 'classnames'; import DatagridHeaderCell from './DatagridHeaderCell'; import DatagridLoading from './DatagridLoading'; -import MemoDatagridBody, { DatagridBody } from './DatagridBody'; +import DatagridBody, { MemoDatagridBody } from './DatagridBody'; const useStyles = makeStyles(theme => ({ table: { diff --git a/packages/ra-ui-materialui/src/list/DatagridBody.js b/packages/ra-ui-materialui/src/list/DatagridBody.js index ab1446af593..63744b170e8 100644 --- a/packages/ra-ui-materialui/src/list/DatagridBody.js +++ b/packages/ra-ui-materialui/src/list/DatagridBody.js @@ -4,9 +4,9 @@ import TableBody from '@material-ui/core/TableBody'; import classnames from 'classnames'; import isEqual from 'lodash/isEqual'; -import PureDatagridRow, { DatagridRow } from './DatagridRow'; +import DatagridRow, { PureDatagridRow } from './DatagridRow'; -export const DatagridBody = ({ +const DatagridBody = ({ basePath, children, classes, @@ -91,11 +91,11 @@ const areEqual = (prevProps, nextProps) => { return isEqual(prevPropsWithoutChildren, nextPropsWithoutChildren); }; -const MemoDatagridBody = memo(DatagridBody, areEqual); +export const MemoDatagridBody = memo(DatagridBody, areEqual); // trick material-ui Table into thinking this is one of the child type it supports MemoDatagridBody.muiName = 'TableBody'; MemoDatagridBody.defaultProps = { row: , }; -export default MemoDatagridBody; +export default DatagridBody; diff --git a/packages/ra-ui-materialui/src/list/DatagridRow.js b/packages/ra-ui-materialui/src/list/DatagridRow.js index a0d2a169b8a..fce7e09fe44 100644 --- a/packages/ra-ui-materialui/src/list/DatagridRow.js +++ b/packages/ra-ui-materialui/src/list/DatagridRow.js @@ -28,7 +28,7 @@ const computeNbColumns = (expand, children, hasBulkActions) => const defaultClasses = {}; -export const DatagridRow = ({ +const DatagridRow = ({ basePath, children, classes = defaultClasses, @@ -213,8 +213,8 @@ const areEqual = (prevProps, nextProps) => { return isEqual(prevPropsWithoutChildren, nextPropsWithoutChildren); }; -const PureDatagridRow = memo(DatagridRow, areEqual); +export const PureDatagridRow = memo(DatagridRow, areEqual); PureDatagridRow.displayName = 'PureDatagridRow'; -export default PureDatagridRow; +export default DatagridRow; From 69d4a485b6f86d04983e9e1c9efa60591241d2b9 Mon Sep 17 00:00:00 2001 From: Gildas Garcia Date: Mon, 14 Oct 2019 17:44:00 +0200 Subject: [PATCH 7/7] Export optimized components --- packages/ra-ui-materialui/src/list/Datagrid.js | 4 ++-- packages/ra-ui-materialui/src/list/DatagridBody.js | 6 +++--- packages/ra-ui-materialui/src/list/index.js | 6 ++++-- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/packages/ra-ui-materialui/src/list/Datagrid.js b/packages/ra-ui-materialui/src/list/Datagrid.js index 00110721e29..0a8a38db097 100644 --- a/packages/ra-ui-materialui/src/list/Datagrid.js +++ b/packages/ra-ui-materialui/src/list/Datagrid.js @@ -16,7 +16,7 @@ import classnames from 'classnames'; import DatagridHeaderCell from './DatagridHeaderCell'; import DatagridLoading from './DatagridLoading'; -import DatagridBody, { MemoDatagridBody } from './DatagridBody'; +import DatagridBody, { PureDatagridBody } from './DatagridBody'; const useStyles = makeStyles(theme => ({ table: { @@ -104,7 +104,7 @@ function Datagrid({ classes: classesOverride, ...props }) { const { basePath, optimized = false, - body = optimized ? : , + body = optimized ? : , children, className, currentSort, diff --git a/packages/ra-ui-materialui/src/list/DatagridBody.js b/packages/ra-ui-materialui/src/list/DatagridBody.js index 63744b170e8..68a27a4fbe0 100644 --- a/packages/ra-ui-materialui/src/list/DatagridBody.js +++ b/packages/ra-ui-materialui/src/list/DatagridBody.js @@ -91,10 +91,10 @@ const areEqual = (prevProps, nextProps) => { return isEqual(prevPropsWithoutChildren, nextPropsWithoutChildren); }; -export const MemoDatagridBody = memo(DatagridBody, areEqual); +export const PureDatagridBody = memo(DatagridBody, areEqual); // trick material-ui Table into thinking this is one of the child type it supports -MemoDatagridBody.muiName = 'TableBody'; -MemoDatagridBody.defaultProps = { +PureDatagridBody.muiName = 'TableBody'; +PureDatagridBody.defaultProps = { row: , }; diff --git a/packages/ra-ui-materialui/src/list/index.js b/packages/ra-ui-materialui/src/list/index.js index 270a05acef8..b78711af8bf 100644 --- a/packages/ra-ui-materialui/src/list/index.js +++ b/packages/ra-ui-materialui/src/list/index.js @@ -2,8 +2,8 @@ import BulkActionsToolbar from './BulkActionsToolbar'; import BulkDeleteAction from './BulkDeleteAction'; import Datagrid from './Datagrid'; import DatagridLoading from './DatagridLoading'; -import DatagridBody from './DatagridBody'; -import DatagridRow from './DatagridRow'; +import DatagridBody, { PureDatagridBody } from './DatagridBody'; +import DatagridRow, { PureDatagridRow } from './DatagridRow'; import DatagridHeaderCell from './DatagridHeaderCell'; import DatagridCell from './DatagridCell'; import Filter from './Filter'; @@ -37,6 +37,8 @@ export { ListToolbar, Pagination, PaginationLimit, + PureDatagridBody, + PureDatagridRow, SimpleList, SingleFieldList, };