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

[RFR] Display a skeleton instead of empty datagrid when loading #2706

Merged
merged 4 commits into from
Jan 1, 2019
Merged
Show file tree
Hide file tree
Changes from 2 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
5 changes: 5 additions & 0 deletions packages/ra-core/src/controller/ListController.js
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,7 @@ export class ListController extends Component {
hasCreate,
data,
ids,
loadedOnce,
total,
isLoading,
translate,
Expand Down Expand Up @@ -278,6 +279,7 @@ export class ListController extends Component {
hideFilter: this.hideFilter,
ids,
isLoading,
loadedOnce,
onSelect: this.handleSelect,
onToggleItem: this.handleToggleItem,
onUnselectItems: this.handleUnselectItems,
Expand Down Expand Up @@ -323,6 +325,7 @@ ListController.propTypes = {
hasList: PropTypes.bool,
hasShow: PropTypes.bool,
ids: PropTypes.array,
loadedOnce: PropTypes.bool,
selectedIds: PropTypes.array,
isLoading: PropTypes.bool.isRequired,
location: PropTypes.object.isRequired,
Expand Down Expand Up @@ -360,6 +363,7 @@ const injectedProps = [
'hideFilter',
'ids',
'isLoading',
'loadedOnce',
'onSelect',
'onToggleItem',
'onUnselectItems',
Expand Down Expand Up @@ -425,6 +429,7 @@ function mapStateToProps(state, props) {
query: selectQuery(props),
params: resourceState.list.params,
ids: resourceState.list.ids,
loadedOnce: resourceState.list.loadedOnce,
selectedIds: resourceState.list.selectedIds,
total: resourceState.list.total,
data: resourceState.data,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ export class ReferenceManyFieldController extends Component {
setSort = field => {
const order =
this.state.sort.field === field &&
this.state.sort.order === SORT_ASC
this.state.sort.order === SORT_ASC
? SORT_DESC
: SORT_ASC;
this.setState({ sort: { field, order } }, this.fetchReferences);
Expand Down Expand Up @@ -135,7 +135,7 @@ export class ReferenceManyFieldController extends Component {
currentSort: this.state.sort,
data,
ids,
isLoading: typeof ids === 'undefined',
loadedOnce: typeof ids !== 'undefined',
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this can be considered a BC break, but it was never documented, so I'm confident it should not break much.

page,
perPage,
referenceBasePath,
Expand Down
2 changes: 2 additions & 0 deletions packages/ra-core/src/reducer/admin/resource/list/index.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
import { combineReducers } from 'redux';
import ids from './ids';
import loadedOnce from './loadedOnce';
import params from './params';
import selectedIds from './selectedIds';
import total from './total';

export default combineReducers({
ids,
loadedOnce,
params,
selectedIds,
total,
Expand Down
20 changes: 20 additions & 0 deletions packages/ra-core/src/reducer/admin/resource/list/loadedOnce.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import { Reducer } from 'redux';
import { CRUD_GET_LIST_SUCCESS } from '../../../../actions/dataActions';

type State = boolean;

/**
* This resource reducer is false until the list loads successfully
*/
const loadedOnce: Reducer<State> = (previousState = false, { type }) => {
// early return
if (previousState === true) {
return previousState;
}
if (type === CRUD_GET_LIST_SUCCESS) {
return true;
}
return previousState;
};

export default loadedOnce;
51 changes: 20 additions & 31 deletions packages/ra-ui-materialui/src/field/ReferenceManyField.js
Original file line number Diff line number Diff line change
@@ -1,21 +1,14 @@
import React, { Fragment, cloneElement } from 'react';
import PropTypes from 'prop-types';
import LinearProgress from '@material-ui/core/LinearProgress';
import { withStyles } from '@material-ui/core/styles';
import { ReferenceManyFieldController } from 'ra-core';

const styles = {
progress: { marginTop: '1em' },
};

export const ReferenceManyFieldView = ({
children,
classes = {},
className,
currentSort,
data,
ids,
isLoading,
loadedOnce,
page,
pagination,
perPage,
Expand All @@ -25,38 +18,40 @@ export const ReferenceManyFieldView = ({
setPerPage,
setSort,
total,
}) => isLoading ? <LinearProgress className={classes.progress} /> :
<Fragment>
{cloneElement(children, {
className,
resource: reference,
ids,
data,
basePath: referenceBasePath,
currentSort,
setSort,
total,
})}
{pagination && cloneElement(pagination, {
}) => (
<Fragment>
{cloneElement(children, {
className,
resource: reference,
ids,
loadedOnce,
data,
basePath: referenceBasePath,
currentSort,
setSort,
total,
})}
{pagination &&
cloneElement(pagination, {
page,
perPage,
setPage,
setPerPage,
total,
})}
</Fragment>;
</Fragment>
);

ReferenceManyFieldView.propTypes = {
children: PropTypes.element,
classes: PropTypes.object,
className: PropTypes.string,
currentSort: PropTypes.shape({
field: PropTypes.string,
order: PropTypes.string,
}),
data: PropTypes.object,
ids: PropTypes.array,
isLoading: PropTypes.bool,
loadedOnce: PropTypes.bool,
pagination: PropTypes.element,
reference: PropTypes.string,
referenceBasePath: PropTypes.string,
Expand Down Expand Up @@ -154,13 +149,7 @@ ReferenceManyField.defaultProps = {
perPage: 25,
sort: { field: 'id', order: 'DESC' },
source: 'id',
};

const EnhancedReferenceManyField = withStyles(styles)(ReferenceManyField);

EnhancedReferenceManyField.defaultProps = {
addLabel: true,
source: 'id',
};

export default EnhancedReferenceManyField;
export default ReferenceManyField;
29 changes: 29 additions & 0 deletions packages/ra-ui-materialui/src/list/Datagrid.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import classnames from 'classnames';

import DatagridHeaderCell from './DatagridHeaderCell';
import DatagridBody from './DatagridBody';
import DatagridLoading from './DatagridLoading';

const styles = theme => ({
table: {
Expand Down Expand Up @@ -125,6 +126,7 @@ class Datagrid extends Component {
hover,
ids,
isLoading,
loadedOnce,
onSelect,
onToggleItem,
resource,
Expand All @@ -137,10 +139,37 @@ class Datagrid extends Component {
...rest
} = this.props;

/**
* if loadedOnce is false, the list displays for the first time, and the dataProvider hasn't answered yet
* if loadedOnce is true, the data for the list has at least been returned once by the dataProvider
* if loadedOnce is undefined, the Datagrid parent doesn't track loading state (e.g. ReferenceArrayField)
*/
if (loadedOnce === false) {
return (
<DatagridLoading
classes={classes}
className={className}
expand={expand}
hasBulkActions={hasBulkActions}
nbChildren={React.Children.count(children)}
/>
);
}

/**
* 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 (!isLoading && (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 (
<Table
className={classnames(classes.table, className)}
Expand Down
108 changes: 108 additions & 0 deletions packages/ra-ui-materialui/src/list/DatagridLoading.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
import React from 'react';
import Table from '@material-ui/core/Table';
import TableCell from '@material-ui/core/TableCell';
import TableHead from '@material-ui/core/TableHead';
import TableRow from '@material-ui/core/TableRow';
import TableBody from '@material-ui/core/TableBody';
import ExpandMoreIcon from '@material-ui/icons/ExpandMore';
import IconButton from '@material-ui/core/IconButton';
import Checkbox from '@material-ui/core/Checkbox';
import classnames from 'classnames';
import { withStyles } from '@material-ui/core/styles';

const RawPlaceholder = ({ classes }) => (
<div className={classes.root}>&nbsp;</div>
);

const styles = theme => ({
root: {
backgroundColor: theme.palette.grey[300],
display: 'flex',
},
});

const Placeholder = withStyles(styles)(RawPlaceholder);

const times = (nbChildren, fn) =>
Array.from({ length: nbChildren }, (_, key) => fn(key));

export default ({
classes,
className,
expand,
hasBulkActions,
nbChildren,
nbFakeLines = 5,
}) => (
<Table className={classnames(classes.table, className)}>
<TableHead>
<TableRow className={classes.row}>
{expand && <TableCell className={classes.expandHeader} />}
{hasBulkActions && (
<TableCell
padding="none"
className={classes.expandIconCell}
>
<Checkbox
className="select-all"
color="primary"
checked={false}
/>
</TableCell>
)}
{times(nbChildren, key => (
<TableCell
padding="none"
variant="head"
className={classes.headerCell}
key={key}
>
<Placeholder />
</TableCell>
))}
</TableRow>
</TableHead>
<TableBody>
{times(nbFakeLines, key1 => (
<TableRow key={key1} style={{ opacity: 1 / (key1 + 1) }}>
{expand && (
<TableCell
padding="none"
className={classes.expandIconCell}
>
<IconButton
className={classes.expandIcon}
component="div"
aria-hidden="true"
role="expand"
>
<ExpandMoreIcon />
</IconButton>
</TableCell>
)}
{hasBulkActions && (
<TableCell
padding="none"
className={classes.expandIconCell}
>
<Checkbox
className="select-all"
color="primary"
checked={false}
/>
</TableCell>
)}
{times(nbChildren, key2 => (
<TableCell
padding="none"
className={classes.rowCell}
key={key2}
>
<Placeholder />
</TableCell>
))}
</TableRow>
))}
</TableBody>
</Table>
);
1 change: 1 addition & 0 deletions packages/ra-ui-materialui/src/list/List.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ const sanitizeRestProps = ({
history,
ids,
isLoading,
loadedOnce,
locale,
location,
match,
Expand Down
8 changes: 7 additions & 1 deletion packages/ra-ui-materialui/src/list/SingleFieldList.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import React, { cloneElement, Component } from 'react';
import PropTypes from 'prop-types';
import classnames from 'classnames';
import LinearProgress from '@material-ui/core/LinearProgress';
import { withStyles } from '@material-ui/core/styles';
import { linkToRecord } from 'ra-core';

Expand All @@ -13,7 +14,7 @@ const styles = {
// useful to prevent click bubbling in a datagrid with rowClick
const stopPropagation = e => e.stopPropagation();

const sanitizeRestProps = ({ currentSort, setSort, isLoading, ...props }) =>
const sanitizeRestProps = ({ currentSort, setSort, loadedOnce, ...props }) =>
props;

/**
Expand Down Expand Up @@ -59,13 +60,18 @@ export class SingleFieldList extends Component {
className,
ids,
data,
loadedOnce,
resource,
basePath,
children,
linkType,
...rest
} = this.props;

if (loadedOnce === false) {
return <LinearProgress />;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not faking multiple lines here too ?

Copy link
Member Author

@fzaninotto fzaninotto Jan 1, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SingleFieldList can contain anything (usually in-line <ChipField>) so I don’t think faking several lines does make sense here.

Besides, I just added the <LinearProgress> here because I removed it from <ReferenceManyField>: it’s now the responsibility of the child to display a loader, not the parent. That way, the loader can look like the upcoming content (e.g. data grid skeleton).

}

return (
<div
className={classnames(classes.root, className)}
Expand Down
Loading