From f1f15c9f5b77de208175829ee33d555e58dec725 Mon Sep 17 00:00:00 2001 From: fzaninotto Date: Fri, 19 May 2023 16:58:13 +0200 Subject: [PATCH 1/3] Fix DatagridConfigurable crashes the app when using a Field with a label element Closes #8843 --- .../datagrid/DatagridConfigurable.stories.tsx | 27 +++++++++++++++++++ .../list/datagrid/DatagridConfigurable.tsx | 9 +++++-- 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/packages/ra-ui-materialui/src/list/datagrid/DatagridConfigurable.stories.tsx b/packages/ra-ui-materialui/src/list/datagrid/DatagridConfigurable.stories.tsx index d3c689acbea..612509e1999 100644 --- a/packages/ra-ui-materialui/src/list/datagrid/DatagridConfigurable.stories.tsx +++ b/packages/ra-ui-materialui/src/list/datagrid/DatagridConfigurable.stories.tsx @@ -131,3 +131,30 @@ export const PreferenceKey = () => ( ); + +export const LabelElement = () => ( + + + + + + + + + + + Original title} /> + + + + + + + + +); diff --git a/packages/ra-ui-materialui/src/list/datagrid/DatagridConfigurable.tsx b/packages/ra-ui-materialui/src/list/datagrid/DatagridConfigurable.tsx index 68c0f6f2468..5b9bffb3c1c 100644 --- a/packages/ra-ui-materialui/src/list/datagrid/DatagridConfigurable.tsx +++ b/packages/ra-ui-materialui/src/list/datagrid/DatagridConfigurable.tsx @@ -64,9 +64,14 @@ export const DatagridConfigurable = ({ index: String(index), source: child.props.source, label: - child.props.source || child.props.label + child.props.label && + typeof child.props.label === 'string' // this list is serializable, so we can't store ReactElement in it ? child.props.label - : translate( + : child.props.source + ? // force the label to be the source + undefined + : // no source or label, generate a label + translate( 'ra.configurable.Datagrid.unlabeled', { column: index, From b424cfae9d7815c585c66f3fa0c3841129ca4b6c Mon Sep 17 00:00:00 2001 From: fzaninotto Date: Fri, 19 May 2023 17:07:30 +0200 Subject: [PATCH 2/3] Add test --- .../datagrid/DatagridConfigurable.spec.tsx | 22 +- .../datagrid/DatagridConfigurable.stories.tsx | 191 ++++++++---------- 2 files changed, 107 insertions(+), 106 deletions(-) diff --git a/packages/ra-ui-materialui/src/list/datagrid/DatagridConfigurable.spec.tsx b/packages/ra-ui-materialui/src/list/datagrid/DatagridConfigurable.spec.tsx index 2f2e534a00a..e328b1e9c10 100644 --- a/packages/ra-ui-materialui/src/list/datagrid/DatagridConfigurable.spec.tsx +++ b/packages/ra-ui-materialui/src/list/datagrid/DatagridConfigurable.spec.tsx @@ -2,7 +2,12 @@ import * as React from 'react'; import { render, screen, fireEvent } from '@testing-library/react'; import expect from 'expect'; -import { Basic, Omit, PreferenceKey } from './DatagridConfigurable.stories'; +import { + Basic, + Omit, + PreferenceKey, + LabelElement, +} from './DatagridConfigurable.stories'; describe('', () => { it('should render a datagrid with configurable columns', async () => { @@ -18,7 +23,7 @@ describe('', () => { screen.getByLabelText('Year').click(); expect(screen.queryByText('1869')).not.toBeNull(); }); - it('should accept fields with a custom title', async () => { + it('should accept fields with a custom label', async () => { render(); screen.getByLabelText('Configure mode').click(); await screen.findByText('Inspector'); @@ -31,6 +36,19 @@ describe('', () => { screen.getByLabelText('Original title').click(); expect(screen.queryByText('War and Peace')).not.toBeNull(); }); + it('should accept fields with a label element', async () => { + render(); + screen.getByLabelText('Configure mode').click(); + await screen.findByText('Inspector'); + fireEvent.mouseOver(screen.getByText('Leo Tolstoy')); + await screen.getByTitle('ra.configurable.customize').click(); + await screen.findByText('Datagrid'); + expect(screen.queryByText('War and Peace')).not.toBeNull(); + screen.getByLabelText('Title').click(); + expect(screen.queryByText('War and Peace')).toBeNull(); + screen.getByLabelText('Title').click(); + expect(screen.queryByText('War and Peace')).not.toBeNull(); + }); it('should accept fields with no source', async () => { render(); screen.getByLabelText('Configure mode').click(); diff --git a/packages/ra-ui-materialui/src/list/datagrid/DatagridConfigurable.stories.tsx b/packages/ra-ui-materialui/src/list/datagrid/DatagridConfigurable.stories.tsx index 612509e1999..154a5b09866 100644 --- a/packages/ra-ui-materialui/src/list/datagrid/DatagridConfigurable.stories.tsx +++ b/packages/ra-ui-materialui/src/list/datagrid/DatagridConfigurable.stories.tsx @@ -2,6 +2,7 @@ import * as React from 'react'; import { MemoryRouter } from 'react-router-dom'; import { PreferencesEditorContextProvider } from 'ra-core'; import { Box } from '@mui/material'; +import { memoryStore, StoreContextProvider } from 'ra-core'; import { DatagridConfigurable } from './DatagridConfigurable'; import { Inspector, InspectorButton } from '../../preferences'; @@ -43,118 +44,100 @@ AuthorField.defaultProps = { label: 'Author' }; const theme = createTheme(); +const Wrapper = ({ children }) => ( + + + + + + + + + {children} + + + + +); + export const Basic = () => ( - - - - - - - - - - - - - - - - - - - + + + + + + + + + ); export const Omit = () => ( - - - - - - - - - - - - - - - - + + + + + + + + ); export const PreferenceKey = () => ( - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + ); export const LabelElement = () => ( - - - - - - - - - - - Original title} /> - - - - - - - - + + + + Original title} /> + + + + + ); From 76dc267006f970162fa48d8140b42384631bec42 Mon Sep 17 00:00:00 2001 From: fzaninotto Date: Fri, 19 May 2023 17:15:42 +0200 Subject: [PATCH 3/3] Document limitation --- docs/Datagrid.md | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/docs/Datagrid.md b/docs/Datagrid.md index a5ed353e618..b3b9bd9b353 100644 --- a/docs/Datagrid.md +++ b/docs/Datagrid.md @@ -920,6 +920,25 @@ const PostList = () => ( ); ``` +The inspector uses the field `source` (or `label` when it's a string) to display the column name. If you use non-field children (e.g. action buttons), then it's your responsibility to wrap them in a component with a `label` prop, that will be used by the inspector: + +```jsx +const FieldWrapper = ({ children, label }) => children; +const PostList = () => ( + + + + + + + + + + + +); +``` + `` accepts the same props as ``. ## Editable Spreadsheet