Merge pull request #4661 from marmelab/fix-emptytext-inconsistencies

Fix EmptyText Inconsistencies
marmelab · Apr 8, 2020 · 87249ad · 87249ad
2 parents c10a5d5 + d0413c6
commit 87249ad
Show file tree

Hide file tree

Showing 13 changed files with 163 additions and 147 deletions.
diff --git a/packages/ra-ui-materialui/src/field/BooleanField.spec.js b/packages/ra-ui-materialui/src/field/BooleanField.spec.js
@@ -59,18 +59,21 @@ describe('<BooleanField />', () => {
         expect(queryByTitle('ra.boolean.false')).toBeNull();
     });
 
-    it('should display the emptyText when is present and the value is null', () => {
-        const { queryByTitle, queryByText } = render(
-            <BooleanField
-                {...defaultProps}
-                record={{ published: null }}
-                emptyText="NA"
-            />
-        );
-        expect(queryByTitle('ra.boolean.true')).toBeNull();
-        expect(queryByTitle('ra.boolean.false')).toBeNull();
-        expect(queryByText('NA')).not.toBeNull();
-    });
+    it.each([null, undefined])(
+        'should display the emptyText when is present and the value is %s',
+        published => {
+            const { queryByTitle, queryByText } = render(
+                <BooleanField
+                    {...defaultProps}
+                    record={{ published }}
+                    emptyText="NA"
+                />
+            );
+            expect(queryByTitle('ra.boolean.true')).toBeNull();
+            expect(queryByTitle('ra.boolean.false')).toBeNull();
+            expect(queryByText('NA')).not.toBeNull();
+        }
+    );
 
     it('should use custom className', () => {
         const { container } = render(

diff --git a/packages/ra-ui-materialui/src/field/BooleanField.tsx b/packages/ra-ui-materialui/src/field/BooleanField.tsx
@@ -37,7 +37,7 @@ export const BooleanField: FunctionComponent<
         ariaLabel = value === false ? 'ra.boolean.false' : 'ra.boolean.true';
     }
 
-    if (value === false) {
+    if (value === false || value === true) {
         return (
             <Typography
                 component="span"
@@ -46,22 +46,11 @@ export const BooleanField: FunctionComponent<
                 {...sanitizeRestProps(rest)}
             >
                 <Tooltip title={translate(ariaLabel, { _: ariaLabel })}>
-                    <FalseIcon data-testid="false" />
-                </Tooltip>
-            </Typography>
-        );
-    }
-
-    if (value === true) {
-        return (
-            <Typography
-                component="span"
-                variant="body2"
-                className={className}
-                {...sanitizeRestProps(rest)}
-            >
-                <Tooltip title={translate(ariaLabel, { _: ariaLabel })}>
-                    <TrueIcon data-testid="true" />
+                    {value === true ? (
+                        <TrueIcon data-testid="true" />
+                    ) : (
+                        <FalseIcon data-testid="false" />
+                    )}
                 </Tooltip>
             </Typography>
         );

diff --git a/packages/ra-ui-materialui/src/field/ChipField.spec.js b/packages/ra-ui-materialui/src/field/ChipField.spec.js
@@ -31,16 +31,19 @@ describe('<ChipField />', () => {
         expect(getByText('foo')).not.toBeNull();
     });
 
-    it('should render the emptyText when value is null', () => {
-        const { getByText } = render(
-            <ChipField
-                className="className"
-                classes={{}}
-                source="name"
-                record={{ name: null }}
-                emptyText="NA"
-            />
-        );
-        expect(getByText('NA')).not.toBeNull();
-    });
+    it.each([null, undefined])(
+        'should render the emptyText when value is %s',
+        name => {
+            const { getByText } = render(
+                <ChipField
+                    className="className"
+                    classes={{}}
+                    source="name"
+                    record={{ name }}
+                    emptyText="NA"
+                />
+            );
+            expect(getByText('NA')).not.toBeNull();
+        }
+    );
 });
diff --git a/packages/ra-ui-materialui/src/field/DateField.spec.js b/packages/ra-ui-materialui/src/field/DateField.spec.js
@@ -106,15 +106,18 @@ describe('<DateField />', () => {
         assert.notEqual(queryByText(date), null);
     });
 
-    it('should render the emptyText when value is null', () => {
-        const { queryByText } = render(
-            <DateField
-                record={{ foo: null }}
-                source="foo"
-                locales="fr-FR"
-                emptyText="NA"
-            />
-        );
-        assert.notEqual(queryByText('NA'), null);
-    });
+    it.each([null, undefined])(
+        'should render the emptyText when value is %s',
+        foo => {
+            const { queryByText } = render(
+                <DateField
+                    record={{ foo }}
+                    source="foo"
+                    locales="fr-FR"
+                    emptyText="NA"
+                />
+            );
+            assert.notEqual(queryByText('NA'), null);
+        }
+    );
 });
diff --git a/packages/ra-ui-materialui/src/field/EmailField.spec.js b/packages/ra-ui-materialui/src/field/EmailField.spec.js
@@ -1,9 +1,11 @@
 import React from 'react';
 import assert from 'assert';
-import { render } from '@testing-library/react';
+import { render, cleanup } from '@testing-library/react';
 import EmailField from './EmailField';
 
 describe('<EmailField />', () => {
+    afterEach(cleanup);
+
     it('should render as an email link', () => {
         const record = { foo: 'foo@bar.com' };
         const { container } = render(
@@ -48,14 +50,17 @@ describe('<EmailField />', () => {
         assert.ok(container.firstChild.classList.contains('foo'));
     });
 
-    it('should render the emptyText when value is null', () => {
-        const { queryByText } = render(
-            <EmailField record={{ foo: null }} source="foo" emptyText="NA" />
-        );
-        assert.notEqual(queryByText('NA'), null);
-    });
+    it.each([null, undefined])(
+        'should render the emptyText when value is %s',
+        foo => {
+            const { queryByText } = render(
+                <EmailField record={{ foo }} source="foo" emptyText="NA" />
+            );
+            assert.notEqual(queryByText('NA'), null);
+        }
+    );
 
-    it('should return null when the record has no value for the source', () => {
+    it('should return null when the record has no value for the source and no emptyText', () => {
         const { container } = render(<EmailField record={{}} source="foo" />);
         assert.equal(container.firstChild, null);
     });

diff --git a/packages/ra-ui-materialui/src/field/FileField.spec.js b/packages/ra-ui-materialui/src/field/FileField.spec.js
@@ -16,16 +16,15 @@ describe('<FileField />', () => {
         expect(container.firstChild.textContent).toEqual('');
     });
 
-    it('should render the emptyText when record has no value', () => {
-        const { queryByText } = render(
-            <FileField
-                record={{ url: null }}
-                emptyText="NA"
-                {...defaultProps}
-            />
-        );
-        expect(queryByText('NA')).not.toBeNull();
-    });
+    it.each([null, undefined])(
+        'should render the emptyText when value is %s',
+        url => {
+            const { queryByText } = render(
+                <FileField record={{ url }} emptyText="NA" {...defaultProps} />
+            );
+            expect(queryByText('NA')).not.toBeNull();
+        }
+    );
 
     it('should render a link with correct attributes based on `source` and `title`', () => {
         const { getByTitle } = render(

diff --git a/packages/ra-ui-materialui/src/field/ImageField.spec.js b/packages/ra-ui-materialui/src/field/ImageField.spec.js
@@ -16,16 +16,15 @@ describe('<ImageField />', () => {
         expect(container.firstChild.textContent).toEqual('');
     });
 
-    it('should render the emptyText when record has no value', () => {
-        const { queryByText } = render(
-            <ImageField
-                record={{ url: null }}
-                emptyText="NA"
-                {...defaultProps}
-            />
-        );
-        expect(queryByText('NA')).not.toBeNull();
-    });
+    it.each([null, undefined])(
+        'should render the emptyText when value is %s',
+        url => {
+            const { queryByText } = render(
+                <ImageField record={{ url }} emptyText="NA" {...defaultProps} />
+            );
+            expect(queryByText('NA')).not.toBeNull();
+        }
+    );
 
     it('should render an image with correct attributes based on `source` and `title`', () => {
         const { getByRole } = render(

diff --git a/packages/ra-ui-materialui/src/field/NumberField.spec.js b/packages/ra-ui-materialui/src/field/NumberField.spec.js
@@ -1,9 +1,11 @@
 import React from 'react';
 import assert from 'assert';
-import { render } from '@testing-library/react';
+import { render, cleanup } from '@testing-library/react';
 import { NumberField } from './NumberField';
 
 describe('<NumberField />', () => {
+    afterEach(cleanup);
+
     it('should return null when the record is not set', () => {
         const { container } = render(<NumberField source="foo" />);
         assert.equal(container.firstChild, null);
@@ -14,12 +16,15 @@ describe('<NumberField />', () => {
         assert.equal(container.firstChild, null);
     });
 
-    it('should render the emptyText when value is null', () => {
-        const { queryByText } = render(
-            <NumberField record={{ foo: null }} emptyText="NA" source="foo" />
-        );
-        assert.notEqual(queryByText('NA'), null);
-    });
+    it.each([null, undefined])(
+        'should render the emptyText when value is %s',
+        foo => {
+            const { getByText } = render(
+                <NumberField record={{ foo }} emptyText="NA" source="foo" />
+            );
+            assert.notEqual(getByText('NA'), null);
+        }
+    );
 
     it('should render a number', () => {
         const { queryByText } = render(
@@ -64,6 +69,6 @@ describe('<NumberField />', () => {
             <NumberField record={{ foo: { bar: 2 } }} source="foo.bar" />
         );
 
-        assert.notEqual(queryByText('1'), null);
+        assert.notEqual(queryByText('2'), null);
     });
 });
diff --git a/packages/ra-ui-materialui/src/field/RichTextField.spec.js b/packages/ra-ui-materialui/src/field/RichTextField.spec.js
@@ -103,26 +103,28 @@ describe('<RichTextField />', () => {
         assert.equal(container.firstChild.classList.contains('foo'), true);
     });
 
-    it('should render the emptyText when value is null and stripTags is set to false', () => {
-        const { queryByText } = render(
-            <RichTextField
-                record={{ body: null }}
-                emptyText="NA"
-                source="body"
-            />
-        );
-        assert.notEqual(queryByText('NA'), null);
-    });
+    it.each([null, undefined])(
+        'should render the emptyText when value is %s and stripTags is set to false',
+        body => {
+            const { queryByText } = render(
+                <RichTextField record={{ body }} emptyText="NA" source="body" />
+            );
+            assert.notEqual(queryByText('NA'), null);
+        }
+    );
 
-    it('should render the emptyText when value is null and stripTags is set to true', () => {
-        const { queryByText } = render(
-            <RichTextField
-                record={{ body: null }}
-                emptyText="NA"
-                source="body"
-                stripTags
-            />
-        );
-        assert.notEqual(queryByText('NA'), null);
-    });
+    it.each([null, undefined])(
+        'should render the emptyText when value is %s and stripTags is set to true',
+        body => {
+            const { queryByText } = render(
+                <RichTextField
+                    record={{ body }}
+                    emptyText="NA"
+                    source="body"
+                    stripTags
+                />
+            );
+            assert.notEqual(queryByText('NA'), null);
+        }
+    );
 });
diff --git a/packages/ra-ui-materialui/src/field/RichTextField.tsx b/packages/ra-ui-materialui/src/field/RichTextField.tsx
@@ -17,18 +17,6 @@ const RichTextField: FunctionComponent<
     Props & InjectedFieldProps & TypographyProps
 > = ({ className, emptyText, source, record = {}, stripTags, ...rest }) => {
     const value = get(record, source);
-    if (stripTags) {
-        return (
-            <Typography
-                className={className}
-                variant="body2"
-                component="span"
-                {...sanitizeRestProps(rest)}
-            >
-                {value == null && emptyText ? emptyText : removeTags(value)}
-            </Typography>
-        );
-    }
 
     return (
         <Typography
@@ -39,6 +27,8 @@ const RichTextField: FunctionComponent<
         >
             {value == null && emptyText ? (
                 emptyText
+            ) : stripTags ? (
+                removeTags(value)
             ) : (
                 <span dangerouslySetInnerHTML={{ __html: value }} />
             )}

diff --git a/packages/ra-ui-materialui/src/field/TextField.spec.js b/packages/ra-ui-materialui/src/field/TextField.spec.js
@@ -1,6 +1,6 @@
 import React from 'react';
 import assert from 'assert';
-import { render, cleanup } from '@testing-library/react';
+import { render, cleanup, getNodeText } from '@testing-library/react';
 import TextField from './TextField';
 
 describe('<TextField />', () => {
@@ -19,6 +19,32 @@ describe('<TextField />', () => {
         );
     });
 
+    it.each([null, undefined])(
+        'should display emptyText prop if provided for %s value',
+        value => {
+            const record = { title: value };
+            const { queryByText } = render(
+                <TextField
+                    emptyText="Sorry, there's nothing here"
+                    record={record}
+                    source="title"
+                />
+            );
+            assert.notEqual(queryByText("Sorry, there's nothing here"), null);
+        }
+    );
+
+    it.each([null, undefined])(
+        'should display nothing for %s value without emptyText prop',
+        value => {
+            const record = { title: value };
+            const { container } = render(
+                <TextField record={record} source="title" />
+            );
+            assert.strictEqual(getNodeText(container), '');
+        }
+    );
+
     it('should handle deep fields', () => {
         const record = {
             foo: { title: "I'm sorry, Dave. I'm afraid I can't do that." },