Skip to content

Commit

Permalink
Merge pull request #5668 from marmelab/optimize-references-loaders-di…
Browse files Browse the repository at this point in the history
…splay

Optimize References Loaders Display
  • Loading branch information
fzaninotto authored Dec 18, 2020
2 parents 6fea3ff + df48a4d commit 352ef61
Show file tree
Hide file tree
Showing 8 changed files with 102 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import SingleFieldList from '../list/SingleFieldList';

describe('<ReferenceArrayField />', () => {
afterEach(cleanup);
it('should render a loading indicator when related records are not yet fetched', () => {
it('should render a loading indicator when related records are not yet fetched and a second has passed', async () => {
const { queryAllByRole } = render(
<ListContextProvider
value={{
Expand All @@ -33,6 +33,8 @@ describe('<ReferenceArrayField />', () => {
</ReferenceArrayFieldView>
</ListContextProvider>
);

await new Promise(resolve => setTimeout(resolve, 1001));
expect(queryAllByRole('progressbar')).toHaveLength(1);
});

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import * as React from 'react';
import { Children, cloneElement, FC, memo, ReactElement } from 'react';
import PropTypes from 'prop-types';
import { LinearProgress } from '@material-ui/core';
import { makeStyles } from '@material-ui/core/styles';
import {
ListContextProvider,
Expand All @@ -16,6 +15,7 @@ import {
import { fieldPropTypes, PublicFieldProps, InjectedFieldProps } from './types';
import { ClassesOverride } from '../types';
import sanitizeFieldRestProps from './sanitizeFieldRestProps';
import { LinearProgress } from '../layout';

/**
* A container component that fetches records from another resource specified
Expand Down
49 changes: 36 additions & 13 deletions packages/ra-ui-materialui/src/field/ReferenceField.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,20 +9,43 @@ import TextField from './TextField';

describe('<ReferenceField />', () => {
afterEach(cleanup);
const record = { id: 123, postId: 123 };

describe('Progress bar', () => {
it('should display a loader on mount if the reference is not in the store', () => {
it("should not display a loader on mount if the reference is not in the store and a second hasn't passed yet", async () => {
const { queryByRole, container } = renderWithRedux(
<ReferenceField
record={{ id: 123, postId: 123 }}
<ReferenceFieldView
record={record}
resource="comments"
source="postId"
reference="posts"
basePath="/comments"
loaded={false}
loading={true}
>
<TextField source="title" />
</ReferenceField>
</ReferenceFieldView>
);
await new Promise(resolve => setTimeout(resolve, 500));
expect(queryByRole('progressbar')).toBeNull();
const links = container.getElementsByTagName('a');
expect(links).toHaveLength(0);
});
it('should display a loader on mount if the reference is not in the store and a second has passed', async () => {
const { queryByRole, container } = renderWithRedux(
<ReferenceFieldView
record={record}
resource="comments"
source="postId"
reference="posts"
basePath="/comments"
loaded={false}
loading={true}
>
<TextField source="title" />
</ReferenceFieldView>
);
await new Promise(resolve => setTimeout(resolve, 1001));
expect(queryByRole('progressbar')).not.toBeNull();
const links = container.getElementsByTagName('a');
expect(links).toHaveLength(0);
Expand All @@ -32,7 +55,7 @@ describe('<ReferenceField />', () => {
const { queryByRole, container } = renderWithRedux(
<MemoryRouter>
<ReferenceField
record={{ id: 123, postId: 123 }}
record={record}
resource="comments"
source="postId"
reference="posts"
Expand Down Expand Up @@ -67,7 +90,7 @@ describe('<ReferenceField />', () => {
<DataProviderContext.Provider value={dataProvider}>
<MemoryRouter>
<ReferenceField
record={{ id: 123, postId: 123 }}
record={record}
resource="comments"
source="postId"
reference="posts"
Expand Down Expand Up @@ -99,7 +122,7 @@ describe('<ReferenceField />', () => {
// @ts-ignore-line
<DataProviderContext.Provider value={dataProvider}>
<ReferenceField
record={{ id: 123, postId: 123 }}
record={record}
resource="comments"
source="postId"
reference="posts"
Expand All @@ -124,7 +147,7 @@ describe('<ReferenceField />', () => {
// @ts-ignore-line
<DataProviderContext.Provider value={dataProvider}>
<ReferenceField
record={{ id: 123, postId: 123 }}
record={record}
resource="comments"
source="postId"
reference="posts"
Expand Down Expand Up @@ -161,7 +184,7 @@ describe('<ReferenceField />', () => {
const { container, getByText } = renderWithRedux(
<MemoryRouter>
<ReferenceField
record={{ id: 123, postId: 123 }}
record={record}
resource="comments"
source="postId"
reference="posts"
Expand Down Expand Up @@ -197,7 +220,7 @@ describe('<ReferenceField />', () => {
<DataProviderContext.Provider value={dataProvider}>
<MemoryRouter>
<ReferenceField
record={{ id: 123, postId: 123 }}
record={record}
resource="comments"
source="postId"
reference="posts"
Expand All @@ -223,7 +246,7 @@ describe('<ReferenceField />', () => {
// @ts-ignore-line
<DataProviderContext.Provider value={dataProvider}>
<ReferenceField
record={{ id: 123, postId: 123 }}
record={record}
resource="comments"
source="postId"
reference="posts"
Expand All @@ -244,7 +267,7 @@ describe('<ReferenceField />', () => {
const { container } = render(
<MemoryRouter>
<ReferenceFieldView
record={{ id: 123, postId: 123 }}
record={record}
source="postId"
referenceRecord={{ id: 123, title: 'foo' }}
reference="posts"
Expand All @@ -266,7 +289,7 @@ describe('<ReferenceField />', () => {
it('should render no link when resourceLinkPath is not specified', () => {
const { container } = render(
<ReferenceFieldView
record={{ id: 123, fooId: 123 }}
record={record}
source="fooId"
referenceRecord={{ id: 123, title: 'foo' }}
reference="bar"
Expand Down
8 changes: 5 additions & 3 deletions packages/ra-ui-materialui/src/field/ReferenceField.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -139,20 +139,21 @@ export const NonEmptyReferenceField: FC<Omit<
if (React.Children.count(children) !== 1) {
throw new Error('<ReferenceField> only accepts a single child');
}
const { basePath, resource } = props;
const { basePath, resource, reference } = props;
const resourceLinkPath = getResourceLinkPath({
...props,
resource,
record,
source,
basePath,
});

return (
<ResourceContextProvider value={props.reference}>
<ResourceContextProvider value={reference}>
<PureReferenceFieldView
{...props}
{...useReference({
reference: props.reference,
reference,
id: get(record, source),
})}
resourceLinkPath={resourceLinkPath}
Expand Down Expand Up @@ -194,6 +195,7 @@ export const ReferenceFieldView: FC<ReferenceFieldViewProps> = props => {
...rest
} = props;
const classes = useStyles(props);

if (!loaded) {
return <LinearProgress />;
}
Expand Down
21 changes: 20 additions & 1 deletion packages/ra-ui-materialui/src/input/ReferenceArrayInput.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ describe('<ReferenceArrayInput />', () => {
translate: x => `*${x}*`,
};

it('should render a progress bar if loading is true', () => {
it("should not render a progress bar if loading is true and a second hasn't passed", async () => {
const MyComponent = () => <div>MyComponent</div>;
const { queryByRole, queryByText } = render(
<ReferenceArrayInputView
Expand All @@ -28,6 +28,25 @@ describe('<ReferenceArrayInput />', () => {
<MyComponent />
</ReferenceArrayInputView>
);
await new Promise(resolve => setTimeout(resolve, 250));
expect(queryByRole('progressbar')).toBeNull();
expect(queryByText('MyComponent')).toBeNull();
});

it('should render a progress bar if loading is true and a second has passed', async () => {
const MyComponent = () => <div>MyComponent</div>;
const { queryByRole, queryByText } = render(
<ReferenceArrayInputView
{...{
...defaultProps,
loading: true,
input: {},
}}
>
<MyComponent />
</ReferenceArrayInputView>
);
await new Promise(resolve => setTimeout(resolve, 1001));
expect(queryByRole('progressbar')).not.toBeNull();
expect(queryByText('MyComponent')).toBeNull();
});
Expand Down
20 changes: 19 additions & 1 deletion packages/ra-ui-materialui/src/input/ReferenceInput.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ describe('<ReferenceInput />', () => {

afterEach(cleanup);

it('should render a LinearProgress if loading is true', () => {
it('should render a LinearProgress if loading is true and a second has passed', async () => {
const { queryByRole } = render(
<ReferenceInputView
{...{
Expand All @@ -72,9 +72,27 @@ describe('<ReferenceInput />', () => {
</ReferenceInputView>
);

await new Promise(resolve => setTimeout(resolve, 1001));
expect(queryByRole('progressbar')).not.toBeNull();
});

it("should not render a LinearProgress if loading is true and a second hasn't passed", async () => {
const { queryByRole } = render(
<ReferenceInputView
{...{
...defaultProps,
input: { value: 1 },
loading: true,
}}
>
<MyComponent />
</ReferenceInputView>
);

await new Promise(resolve => setTimeout(resolve, 250));
expect(queryByRole('progressbar')).toBeNull();
});

it('should not render a LinearProgress if loading is false', () => {
const { queryByRole } = render(
<ReferenceInputView
Expand Down
20 changes: 16 additions & 4 deletions packages/ra-ui-materialui/src/layout/LinearProgress.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
import * as React from 'react';
import Progress from '@material-ui/core/LinearProgress';
import Progress, {
LinearProgressProps as ProgressProps,
} from '@material-ui/core/LinearProgress';
import PropTypes from 'prop-types';
import { makeStyles } from '@material-ui/core/styles';
import classnames from 'classnames';
import { useTimeout } from 'ra-core';

const useStyles = makeStyles(
theme => ({
Expand All @@ -24,18 +27,27 @@ const useStyles = makeStyles(
*
* @param {Object} classes CSS class names
*/
const LinearProgress = props => {
const LinearProgress = ({ timeout = 1000, ...props }: LinearProgressProps) => {
const { classes: classesOverride, className, ...rest } = props;
const classes = useStyles(props);
return (
const oneSecondHasPassed = useTimeout(timeout);

return oneSecondHasPassed ? (
<Progress className={classnames(classes.root, className)} {...rest} />
);
) : null;
};

LinearProgress.propTypes = {
classes: PropTypes.object,
className: PropTypes.string,
timeout: PropTypes.number,
};

// wat? TypeScript looses the displayName if we don't set it explicitly
LinearProgress.displayName = 'LinearProgress';

export interface LinearProgressProps extends ProgressProps {
timeout?: number;
}

export default LinearProgress;
3 changes: 2 additions & 1 deletion packages/ra-ui-materialui/src/layout/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import HideOnScroll, { HideOnScrollProps } from './HideOnScroll';
import Layout, { LayoutProps } from './Layout';
import Loading from './Loading';
import LoadingPage from './LoadingPage';
import LinearProgress from './LinearProgress';
import LinearProgress, { LinearProgressProps } from './LinearProgress';
import LoadingIndicator from './LoadingIndicator';
import Menu, { MenuProps } from './Menu';
import MenuItemLink, { MenuItemLinkProps } from './MenuItemLink';
Expand Down Expand Up @@ -57,6 +57,7 @@ export type {
ErrorProps,
HideOnScrollProps,
LayoutProps,
LinearProgressProps,
MenuItemLinkProps,
MenuProps,
ResponsiveProps,
Expand Down

0 comments on commit 352ef61

Please sign in to comment.