Skip to content

Commit

Permalink
Merge pull request #5613 from marmelab/list-optims
Browse files Browse the repository at this point in the history
Fix useless AppBar rerenders
  • Loading branch information
djhi committed Dec 2, 2020
2 parents 3fed1f9 + c3ed422 commit 277fd89
Show file tree
Hide file tree
Showing 7 changed files with 36 additions and 12 deletions.
4 changes: 2 additions & 2 deletions examples/simple/src/Layout.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import * as React from 'react';
import { forwardRef } from 'react';
import { forwardRef, memo } from 'react';
import { Layout, AppBar, UserMenu, useLocale, useSetLocale } from 'react-admin';
import { MenuItem, ListItemIcon } from '@material-ui/core';
import { makeStyles } from '@material-ui/core/styles';
Expand Down Expand Up @@ -39,6 +39,6 @@ const MyUserMenu = props => (
</UserMenu>
);

const MyAppBar = props => <AppBar {...props} userMenu={<MyUserMenu />} />;
const MyAppBar = memo(props => <AppBar {...props} userMenu={<MyUserMenu />} />);

export default props => <Layout {...props} appBar={MyAppBar} />;
4 changes: 3 additions & 1 deletion examples/simple/src/posts/PostList.js
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ const PostList = props => {
reference="tags"
source="tags"
sortBy="tags.name"
sort={{ field: 'name', order: 'ASC' }}
sort={tagSort}
cellClassName={classes.hiddenOnSmallScreens}
headerClassName={classes.hiddenOnSmallScreens}
>
Expand All @@ -178,4 +178,6 @@ const PostList = props => {
);
};

const tagSort = { field: 'name', order: 'ASC' };

export default PostList;
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ interface Option {
source: string;
}

const emptyArray = [];
const defaultFilter = {};
const defaultSort = { field: null, order: null };

Expand Down Expand Up @@ -65,7 +66,7 @@ const useReferenceArrayFieldController = (
} = props;
const resource = useResourceContext(props);
const notify = useNotify();
const ids = get(record, source) || [];
const ids = get(record, source) || emptyArray;
const { data, error, loading, loaded } = useGetMany(reference, ids, {
onFailure: error =>
notify(
Expand Down
12 changes: 10 additions & 2 deletions packages/ra-core/src/core/CoreAdminUI.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
import * as React from 'react';
import { createElement, FunctionComponent, ComponentType } from 'react';
import {
createElement,
FunctionComponent,
ComponentType,
useMemo,
} from 'react';
import { Switch, Route } from 'react-router-dom';

import CoreAdminRouter from './CoreAdminRouter';
Expand Down Expand Up @@ -54,6 +59,9 @@ const CoreAdminUI: FunctionComponent<AdminUIProps> = ({
theme,
title = 'React Admin',
}) => {
const logoutElement = useMemo(() => logout && createElement(logout), [
logout,
]);
return (
<Switch>
{loginPage !== false && loginPage !== true ? (
Expand All @@ -78,7 +86,7 @@ const CoreAdminUI: FunctionComponent<AdminUIProps> = ({
dashboard={dashboard}
layout={layout}
loading={loading}
logout={logout && createElement(logout)}
logout={logoutElement}
menu={menu}
ready={ready}
theme={theme}
Expand Down
10 changes: 8 additions & 2 deletions packages/ra-ui-materialui/src/field/BooleanField.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@ describe('<BooleanField />', () => {
it('should display tick and truthy text if value is true', () => {
const { queryByTitle } = render(<BooleanField {...defaultProps} />);
expect(queryByTitle('ra.boolean.true')).not.toBeNull();
expect(queryByTitle('ra.boolean.true').dataset.testid).toBe('true');
expect(
(queryByTitle('ra.boolean.true').firstChild as HTMLElement).dataset
.testid
).toBe('true');
expect(queryByTitle('ra.boolean.false')).toBeNull();
});

Expand All @@ -39,7 +42,10 @@ describe('<BooleanField />', () => {
);
expect(queryByTitle('ra.boolean.true')).toBeNull();
expect(queryByTitle('ra.boolean.false')).not.toBeNull();
expect(queryByTitle('ra.boolean.false').dataset.testid).toBe('false');
expect(
(queryByTitle('ra.boolean.false').firstChild as HTMLElement).dataset
.testid
).toBe('false');
});

it('should use valueLabelFalse for custom falsy text', () => {
Expand Down
11 changes: 9 additions & 2 deletions packages/ra-ui-materialui/src/field/BooleanField.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,16 @@ export const BooleanField: FC<BooleanFieldProps> = memo<BooleanFieldProps>(
>
<Tooltip title={translate(ariaLabel, { _: ariaLabel })}>
{value === true ? (
<TrueIcon data-testid="true" fontSize="small" />
<span>
<TrueIcon data-testid="true" fontSize="small" />
</span>
) : (
<FalseIcon data-testid="false" fontSize="small" />
<span>
<FalseIcon
data-testid="false"
fontSize="small"
/>
</span>
)}
</Tooltip>
</Typography>
Expand Down
4 changes: 2 additions & 2 deletions packages/ra-ui-materialui/src/layout/AppBar.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import * as React from 'react';
import { Children, cloneElement } from 'react';
import { Children, cloneElement, memo } from 'react';
import PropTypes from 'prop-types';
import { useDispatch } from 'react-redux';
import classNames from 'classnames';
Expand Down Expand Up @@ -195,4 +195,4 @@ export interface AppBarProps extends Omit<MuiAppBarProps, 'title' | 'classes'> {
userMenu?: JSX.Element | boolean;
}

export default AppBar;
export default memo(AppBar);

0 comments on commit 277fd89

Please sign in to comment.