From fb544b126a2460a6479c99d9a9df5bd30b962d67 Mon Sep 17 00:00:00 2001 From: jaytula Date: Thu, 22 Aug 2019 16:22:37 -0700 Subject: [PATCH 1/3] Convert DataGrid component to useStyles --- .../ra-ui-materialui/src/list/Datagrid.js | 363 +++++++++--------- 1 file changed, 177 insertions(+), 186 deletions(-) diff --git a/packages/ra-ui-materialui/src/list/Datagrid.js b/packages/ra-ui-materialui/src/list/Datagrid.js index 2636d73710d..c5920417b6e 100644 --- a/packages/ra-ui-materialui/src/list/Datagrid.js +++ b/packages/ra-ui-materialui/src/list/Datagrid.js @@ -1,12 +1,7 @@ -import React, { - Component, - isValidElement, - Children, - cloneElement, -} from 'react'; +import React, { isValidElement, Children, cloneElement } from 'react'; import PropTypes from 'prop-types'; import { sanitizeListRestProps } from 'ra-core'; -import { withStyles, createStyles } from '@material-ui/core/styles'; +import { makeStyles } from '@material-ui/core/styles'; import Table from '@material-ui/core/Table'; import TableCell from '@material-ui/core/TableCell'; import TableHead from '@material-ui/core/TableHead'; @@ -18,55 +13,54 @@ import DatagridHeaderCell from './DatagridHeaderCell'; import DatagridBody from './DatagridBody'; import DatagridLoading from './DatagridLoading'; -const styles = theme => - createStyles({ - table: { - tableLayout: 'auto', - }, - thead: {}, - tbody: { - height: 'inherit', - }, - headerRow: {}, - headerCell: { - height: 42, - minHeight: 42, +const useStyles = makeStyles(theme => ({ + table: { + tableLayout: 'auto', + }, + thead: {}, + tbody: { + height: 'inherit', + }, + headerRow: {}, + headerCell: { + height: 42, + minHeight: 42, + padding: '0 12px', + '&:last-child': { padding: '0 12px', - '&:last-child': { - padding: '0 12px', - }, - }, - checkbox: {}, - row: {}, - clickableRow: { - cursor: 'pointer', }, - rowEven: {}, - rowOdd: {}, - rowCell: { + }, + checkbox: {}, + row: {}, + clickableRow: { + cursor: 'pointer', + }, + rowEven: {}, + rowOdd: {}, + rowCell: { + padding: '0 12px', + '&:last-child': { padding: '0 12px', - '&:last-child': { - padding: '0 12px', - }, - }, - expandHeader: { - padding: 0, - width: theme.spacing(6), - }, - expandIconCell: { - width: theme.spacing(6), }, - expandIcon: { - padding: theme.spacing(1), - transform: 'rotate(-90deg)', - transition: theme.transitions.create('transform', { - duration: theme.transitions.duration.shortest, - }), - }, - expanded: { - transform: 'rotate(0deg)', - }, - }); + }, + expandHeader: { + padding: 0, + width: theme.spacing(6), + }, + expandIconCell: { + width: theme.spacing(6), + }, + expandIcon: { + padding: theme.spacing(1), + transform: 'rotate(-90deg)', + transition: theme.transitions.create('transform', { + duration: theme.transitions.duration.shortest, + }), + }, + expanded: { + transform: 'rotate(0deg)', + }, +})); /** * The Datagrid component renders a list of records as a table. @@ -100,14 +94,40 @@ const styles = theme => * * */ -class Datagrid extends Component { - updateSort = event => { +function Datagrid({ classes: classesOverride, ...props }) { + const classes = useStyles({ classes: classesOverride }); + const { + basePath, + body, + children, + className, + currentSort, + data, + expand, + hasBulkActions, + hover, + ids, + isLoading, + loaded, + onSelect, + onToggleItem, + resource, + rowClick, + rowStyle, + selectedIds, + setSort, + total, + version, + ...rest + } = props; + + const updateSort = event => { event.stopPropagation(); - this.props.setSort(event.currentTarget.dataset.sort); + props.setSort(event.currentTarget.dataset.sort); }; - handleSelectAll = event => { - const { onSelect, ids, selectedIds } = this.props; + const handleSelectAll = event => { + const { onSelect, ids, selectedIds } = props; if (event.target.checked) { onSelect( ids.reduce( @@ -122,138 +142,109 @@ class Datagrid extends Component { } }; - render() { - const { - basePath, - body, - children, - classes, - className, - currentSort, - data, - expand, - hasBulkActions, - hover, - ids, - isLoading, - loaded, - onSelect, - onToggleItem, - resource, - rowClick, - rowStyle, - selectedIds, - setSort, - total, - version, - ...rest - } = this.props; - - /** - * if loaded is false, the list displays for the first time, and the dataProvider hasn't answered yet - * if loaded is true, the data for the list has at least been returned once by the dataProvider - * if loaded is undefined, the Datagrid parent doesn't track loading state (e.g. ReferenceArrayField) - */ - if (loaded === false) { - return ( - - ); - } + /** + * if loaded is false, the list displays for the first time, and the dataProvider hasn't answered yet + * if loaded is true, the data for the list has at least been returned once by the dataProvider + * if loaded is undefined, the Datagrid parent doesn't track loading state (e.g. ReferenceArrayField) + */ + if (loaded === false) { + return ( + + ); + } - /** - * Once loaded, the data for the list may be empty. Instead of - * displaying the table header with zero data rows, - * the datagrid displays nothing in this case. - */ - if (loaded && (ids.length === 0 || total === 0)) { - return null; - } + /** + * Once loaded, the data for the list may be empty. Instead of + * displaying the table header with zero data rows, + * the datagrid displays nothing in this case. + */ + if (loaded && (ids.length === 0 || total === 0)) { + return null; + } - /** - * After the initial load, if the data for the list isn't empty, - * and even if the data is refreshing (e.g. after a filter change), - * the datagrid displays the current data. - */ - return ( - - - - {expand && ( - + + + {expand && ( + + )} + {hasBulkActions && ( + + 0 && + ids.length > 0 && + !ids.find( + it => selectedIds.indexOf(it) === -1 + ) + } + onChange={handleSelectAll} + /> + + )} + {Children.map(children, (field, index) => + isValidElement(field) ? ( + - )} - {hasBulkActions && ( - - 0 && - ids.length > 0 && - !ids.find( - it => selectedIds.indexOf(it) === -1 - ) - } - onChange={this.handleSelectAll} - /> - - )} - {Children.map(children, (field, index) => - isValidElement(field) ? ( - - ) : null - )} - - - {cloneElement( - body, - { - basePath, - className: classes.tbody, - classes, - expand, - rowClick, - data, - hasBulkActions, - hover, - ids, - onToggleItem, - resource, - rowStyle, - selectedIds, - version, - }, - children - )} -
- ); - } + ) : null + )} + + + {cloneElement( + body, + { + basePath, + className: classes.tbody, + classes, + expand, + rowClick, + data, + hasBulkActions, + hover, + ids, + onToggleItem, + resource, + rowStyle, + selectedIds, + version, + }, + children + )} + + ); } Datagrid.propTypes = { @@ -291,4 +282,4 @@ Datagrid.defaultProps = { body: , }; -export default withStyles(styles)(Datagrid); +export default Datagrid; From 5ce05ecc8691368cfd121ade422a9c3f886ad753 Mon Sep 17 00:00:00 2001 From: jaytula Date: Fri, 23 Aug 2019 08:49:42 -0700 Subject: [PATCH 2/3] Remove unnecessary props extraction --- packages/ra-ui-materialui/src/list/Datagrid.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/ra-ui-materialui/src/list/Datagrid.js b/packages/ra-ui-materialui/src/list/Datagrid.js index c5920417b6e..89bb77af4cc 100644 --- a/packages/ra-ui-materialui/src/list/Datagrid.js +++ b/packages/ra-ui-materialui/src/list/Datagrid.js @@ -123,11 +123,10 @@ function Datagrid({ classes: classesOverride, ...props }) { const updateSort = event => { event.stopPropagation(); - props.setSort(event.currentTarget.dataset.sort); + setSort(event.currentTarget.dataset.sort); }; const handleSelectAll = event => { - const { onSelect, ids, selectedIds } = props; if (event.target.checked) { onSelect( ids.reduce( From 94f606eecf6a39e06c4e362a8a88f3fa89821b77 Mon Sep 17 00:00:00 2001 From: jaytula Date: Fri, 23 Aug 2019 08:52:59 -0700 Subject: [PATCH 3/3] Memoize updateSort and handleSelectAll with useCallback --- .../ra-ui-materialui/src/list/Datagrid.js | 47 ++++++++++++------- 1 file changed, 29 insertions(+), 18 deletions(-) diff --git a/packages/ra-ui-materialui/src/list/Datagrid.js b/packages/ra-ui-materialui/src/list/Datagrid.js index 89bb77af4cc..0d576fbb7a7 100644 --- a/packages/ra-ui-materialui/src/list/Datagrid.js +++ b/packages/ra-ui-materialui/src/list/Datagrid.js @@ -1,4 +1,9 @@ -import React, { isValidElement, Children, cloneElement } from 'react'; +import React, { + isValidElement, + Children, + cloneElement, + useCallback, +} from 'react'; import PropTypes from 'prop-types'; import { sanitizeListRestProps } from 'ra-core'; import { makeStyles } from '@material-ui/core/styles'; @@ -121,25 +126,31 @@ function Datagrid({ classes: classesOverride, ...props }) { ...rest } = props; - const updateSort = event => { - event.stopPropagation(); - setSort(event.currentTarget.dataset.sort); - }; + const updateSort = useCallback( + event => { + event.stopPropagation(); + setSort(event.currentTarget.dataset.sort); + }, + [setSort] + ); - const handleSelectAll = event => { - if (event.target.checked) { - onSelect( - ids.reduce( - (idList, id) => - idList.includes(id) ? idList : idList.concat(id), + const handleSelectAll = useCallback( + event => { + if (event.target.checked) { + onSelect( + ids.reduce( + (idList, id) => + idList.includes(id) ? idList : idList.concat(id), - selectedIds - ) - ); - } else { - onSelect([]); - } - }; + selectedIds + ) + ); + } else { + onSelect([]); + } + }, + [ids, onSelect, selectedIds] + ); /** * if loaded is false, the list displays for the first time, and the dataProvider hasn't answered yet