From a73b0d7cd6242fea1dc8837435a74caa8861f724 Mon Sep 17 00:00:00 2001
From: djhi <gildas.garcia@gmail.com>
Date: Thu, 22 Aug 2019 16:51:20 +0200
Subject: [PATCH 1/2] [RFR] Migrate SelectArrayInput to hooks

---
 UPGRADE.md                                    |   7 +-
 .../src/input/SelectArrayInput.js             | 314 +++++++++---------
 .../src/input/SelectArrayInput.spec.js        | 231 ++++++++-----
 3 files changed, 300 insertions(+), 252 deletions(-)

diff --git a/UPGRADE.md b/UPGRADE.md
index 665a03236e2..72dde3e7011 100644
--- a/UPGRADE.md
+++ b/UPGRADE.md
@@ -784,4 +784,9 @@ const OrderEdit = (props) => (
         </SimpleForm>
     </Edit>
 );
-```
\ No newline at end of file
+```
+
+## helperText is handled the same way in all components
+
+Some components (such as `<SelectArrayInput />`) accepted the `helperText` in their `meta` prop. They now receive it directly in their props.
+Besides, all components now display their error or their helperText, but not both at the same time.
diff --git a/packages/ra-ui-materialui/src/input/SelectArrayInput.js b/packages/ra-ui-materialui/src/input/SelectArrayInput.js
index 36937541906..6a56dce5c1d 100644
--- a/packages/ra-ui-materialui/src/input/SelectArrayInput.js
+++ b/packages/ra-ui-materialui/src/input/SelectArrayInput.js
@@ -1,17 +1,18 @@
-import React, { Component } from 'react';
+import React, { useCallback } from 'react';
 import PropTypes from 'prop-types';
 import get from 'lodash/get';
-import Select from '@material-ui/core/Select';
-import MenuItem from '@material-ui/core/MenuItem';
-import InputLabel from '@material-ui/core/InputLabel';
-import Input from '@material-ui/core/Input';
-import FormHelperText from '@material-ui/core/FormHelperText';
-import FormControl from '@material-ui/core/FormControl';
-import Chip from '@material-ui/core/Chip';
-import { withStyles, createStyles } from '@material-ui/core/styles';
-import compose from 'recompose/compose';
+import {
+    makeStyles,
+    Select,
+    MenuItem,
+    InputLabel,
+    Input,
+    FormHelperText,
+    FormControl,
+    Chip,
+} from '@material-ui/core';
 import classnames from 'classnames';
-import { addField, translate, FieldTitle } from 'ra-core';
+import { FieldTitle, useInput, useTranslate } from 'ra-core';
 import InputHelperText from './InputHelperText';
 
 const sanitizeRestProps = ({
@@ -54,21 +55,20 @@ const sanitizeRestProps = ({
     ...rest
 }) => rest;
 
-const styles = theme =>
-    createStyles({
-        root: {},
-        chips: {
-            display: 'flex',
-            flexWrap: 'wrap',
-        },
-        chip: {
-            margin: theme.spacing(1 / 4),
-        },
-        select: {
-            height: 'auto',
-            overflow: 'auto',
-        },
-    });
+const useStyles = makeStyles(theme => ({
+    root: {},
+    chips: {
+        display: 'flex',
+        flexWrap: 'wrap',
+    },
+    chip: {
+        margin: theme.spacing(1 / 4),
+    },
+    select: {
+        height: 'auto',
+        overflow: 'auto',
+    },
+}));
 
 /**
  * An Input component for a select box allowing multiple selections, using an array of objects for the options
@@ -122,149 +122,149 @@ const styles = theme =>
  *    { id: 'photography', name: 'myroot.tags.photography' },
  * ];
  */
-export class SelectArrayInput extends Component {
-    /*
-     * Using state to bypass a redux-form comparison but which prevents re-rendering
-     * @see https://github.com/erikras/redux-form/issues/2456
-     */
-    state = {
-        value: this.props.input.value || [],
-    };
+const SelectArrayInput = ({
+    choices,
+    classes: classesOverride,
+    className,
+    label,
+    helperText,
+    onBlur,
+    onChange,
+    onFocus,
+    options,
+    optionText,
+    optionValue,
+    resource,
+    source,
+    translateChoice,
+    validate,
+    ...rest
+}) => {
+    const classes = useStyles({ classes: classesOverride });
 
-    componentWillReceiveProps(nextProps) {
-        if (nextProps.input.value !== this.props.input.value) {
-            this.setState({ value: nextProps.input.value || [] });
-        }
-    }
+    const translate = useTranslate();
 
-    handleChange = event => {
-        this.props.input.onChange(event.target.value);
-        // HACK: For some reason, redux-form does not consider this input touched without calling onBlur manually
-        this.props.input.onBlur(event.target.value);
-        this.setState({ value: event.target.value });
-    };
+    const {
+        id,
+        input,
+        isRequired,
+        meta: { error, touched },
+    } = useInput({
+        onBlur,
+        onChange,
+        onFocus,
+        resource,
+        source,
+        validate,
+        ...rest,
+    });
 
-    renderMenuItemOption = choice => {
-        const { optionText, translate, translateChoice } = this.props;
-        if (React.isValidElement(optionText)) {
-            return React.cloneElement(optionText, {
-                record: choice,
-            });
-        }
+    const handleChange = useCallback(
+        event => {
+            input.onChange(event.target.value);
+            // HACK: For some reason, redux-form does not consider this input touched without calling onBlur manually
+            input.onBlur(event.target.value);
+        },
+        [input]
+    );
 
-        const choiceName =
-            typeof optionText === 'function'
-                ? optionText(choice)
-                : get(choice, optionText);
+    const renderMenuItemOption = useCallback(
+        choice => {
+            if (React.isValidElement(optionText)) {
+                return React.cloneElement(optionText, {
+                    record: choice,
+                });
+            }
 
-        return translateChoice
-            ? translate(choiceName, { _: choiceName })
-            : choiceName;
-    };
+            const choiceName =
+                typeof optionText === 'function'
+                    ? optionText(choice)
+                    : get(choice, optionText);
 
-    renderMenuItem = choice => {
-        const { optionValue } = this.props;
-        return choice ? (
-            <MenuItem
-                key={get(choice, optionValue)}
-                value={get(choice, optionValue)}
-            >
-                {this.renderMenuItemOption(choice)}
-            </MenuItem>
-        ) : null;
-    };
+            return translateChoice
+                ? translate(choiceName, { _: choiceName })
+                : choiceName;
+        },
+        [optionText, translate, translateChoice]
+    );
 
-    render() {
-        const {
-            choices,
-            classes,
-            className,
-            isRequired,
-            label,
-            meta,
-            options,
-            resource,
-            source,
-            optionText,
-            optionValue,
-            helperText,
-            ...rest
-        } = this.props;
-        if (typeof meta === 'undefined') {
-            throw new Error(
-                "The SelectInput component wasn't called within a redux-form <Field>. Did you decorate it and forget to add the addField prop to your component? See https://marmelab.com/react-admin/Inputs.html#writing-your-own-input-component for details."
-            );
-        }
-        const { touched, error } = meta;
+    const renderMenuItem = useCallback(
+        choice => {
+            return choice ? (
+                <MenuItem
+                    key={get(choice, optionValue)}
+                    value={get(choice, optionValue)}
+                >
+                    {renderMenuItemOption(choice)}
+                </MenuItem>
+            ) : null;
+        },
+        [optionValue, renderMenuItemOption]
+    );
 
-        return (
-            <FormControl
-                margin="normal"
-                className={classnames(classes.root, className)}
-                error={touched && !!error}
-                {...sanitizeRestProps(rest)}
+    return (
+        <FormControl
+            margin="normal"
+            className={classnames(classes.root, className)}
+            error={touched && !!error}
+            {...sanitizeRestProps(rest)}
+        >
+            <InputLabel htmlFor={id}>
+                <FieldTitle
+                    label={label}
+                    source={source}
+                    resource={resource}
+                    isRequired={isRequired}
+                />
+            </InputLabel>
+            <Select
+                autoWidth
+                multiple
+                input={<Input id={id} />}
+                value={input.value || []}
+                error={!!(touched && error)}
+                renderValue={selected => (
+                    <div className={classes.chips}>
+                        {selected
+                            .map(item =>
+                                choices.find(
+                                    choice => get(choice, optionValue) === item
+                                )
+                            )
+                            .map(item => (
+                                <Chip
+                                    key={get(item, optionValue)}
+                                    label={renderMenuItemOption(item)}
+                                    className={classes.chip}
+                                />
+                            ))}
+                    </div>
+                )}
+                data-testid="selectArray"
+                {...options}
+                onChange={handleChange}
             >
-                <InputLabel htmlFor={source}>
-                    <FieldTitle
-                        label={label}
-                        source={source}
-                        resource={resource}
-                        isRequired={isRequired}
+                {choices.map(renderMenuItem)}
+            </Select>
+            {helperText || (touched && error) ? (
+                <FormHelperText>
+                    <InputHelperText
+                        touched={touched}
+                        error={error}
+                        helperText={helperText}
                     />
-                </InputLabel>
-                <Select
-                    autoWidth
-                    multiple
-                    input={<Input id={source} />}
-                    value={this.state.value}
-                    error={!!(touched && error)}
-                    renderValue={selected => (
-                        <div className={classes.chips}>
-                            {selected
-                                .map(item =>
-                                    choices.find(
-                                        choice =>
-                                            get(choice, optionValue) === item
-                                    )
-                                )
-                                .map(item => (
-                                    <Chip
-                                        key={get(item, optionValue)}
-                                        label={this.renderMenuItemOption(item)}
-                                        className={classes.chip}
-                                    />
-                                ))}
-                        </div>
-                    )}
-                    data-testid="selectArray"
-                    {...options}
-                    onChange={this.handleChange}
-                >
-                    {choices.map(this.renderMenuItem)}
-                </Select>
-                {helperText || (touched && error) ? (
-                    <FormHelperText>
-                        <InputHelperText
-                            touched={touched}
-                            error={error}
-                            helperText={helperText}
-                        />
-                    </FormHelperText>
-                ) : null}
-            </FormControl>
-        );
-    }
-}
+                </FormHelperText>
+            ) : null}
+        </FormControl>
+    );
+};
 
 SelectArrayInput.propTypes = {
     choices: PropTypes.arrayOf(PropTypes.object),
     classes: PropTypes.object,
     className: PropTypes.string,
     children: PropTypes.node,
-    input: PropTypes.object,
-    isRequired: PropTypes.bool,
     label: PropTypes.string,
-    meta: PropTypes.object,
     options: PropTypes.object,
     optionText: PropTypes.oneOfType([
         PropTypes.string,
@@ -274,12 +274,10 @@ SelectArrayInput.propTypes = {
     optionValue: PropTypes.string.isRequired,
     resource: PropTypes.string,
     source: PropTypes.string,
-    translate: PropTypes.func.isRequired,
     translateChoice: PropTypes.bool,
 };
 
 SelectArrayInput.defaultProps = {
-    classes: {},
     choices: [],
     options: {},
     optionText: 'name',
@@ -287,10 +285,4 @@ SelectArrayInput.defaultProps = {
     translateChoice: true,
 };
 
-const EnhancedSelectArrayInput = compose(
-    addField,
-    translate,
-    withStyles(styles)
-)(SelectArrayInput);
-
-export default EnhancedSelectArrayInput;
+export default SelectArrayInput;
diff --git a/packages/ra-ui-materialui/src/input/SelectArrayInput.spec.js b/packages/ra-ui-materialui/src/input/SelectArrayInput.spec.js
index 4d20fa1b0bc..01ec6dba8cd 100644
--- a/packages/ra-ui-materialui/src/input/SelectArrayInput.spec.js
+++ b/packages/ra-ui-materialui/src/input/SelectArrayInput.spec.js
@@ -1,54 +1,52 @@
 import React from 'react';
 import expect from 'expect';
 import { render, cleanup } from '@testing-library/react';
+import { Form } from 'react-final-form';
+import { TranslationContext } from 'ra-core';
 
-import { SelectArrayInput } from './SelectArrayInput';
+import SelectArrayInput from './SelectArrayInput';
 
 describe('<SelectArrayInput />', () => {
     const defaultProps = {
-        classes: {},
-        resource: 'bar',
-        source: 'foo',
-        meta: {},
-        input: { onChange: () => null, onBlur: () => null },
-        translate: x => x,
+        resource: 'posts',
+        source: 'categories',
+        choices: [
+            { id: 'programming', name: 'Programming' },
+            { id: 'lifestyle', name: 'Lifestyle' },
+            { id: 'photography', name: 'Photography' },
+        ],
     };
 
     afterEach(cleanup);
 
     it('should use a mui Select', () => {
         const { queryByTestId } = render(
-            <SelectArrayInput {...defaultProps} input={{}} />
+            <Form
+                onSubmit={jest.fn()}
+                render={() => <SelectArrayInput {...defaultProps} />}
+            />
         );
         expect(queryByTestId('selectArray')).toBeDefined();
     });
 
     it('should use the input parameter value as the initial input value', () => {
         const { getByLabelText } = render(
-            <SelectArrayInput
-                {...defaultProps}
-                input={{ value: ['programming', 'lifestyle'] }}
-                choices={[
-                    { id: 'programming', name: 'Programming' },
-                    { id: 'lifestyle', name: 'Lifestyle' },
-                    { id: 'lorem', name: 'Lorem' },
-                ]}
+            <Form
+                initialValues={{ categories: ['programming', 'lifestyle'] }}
+                onSubmit={jest.fn()}
+                render={() => <SelectArrayInput {...defaultProps} />}
             />
         );
-        expect(getByLabelText('resources.bar.fields.foo').value).toBe(
+        expect(getByLabelText('resources.posts.fields.categories').value).toBe(
             'programming,lifestyle'
         );
     });
 
     it('should reveal choices on click', () => {
         const { getByRole, queryByText } = render(
-            <SelectArrayInput
-                {...defaultProps}
-                choices={[
-                    { id: 'programming', name: 'Programming' },
-                    { id: 'lifestyle', name: 'Lifestyle' },
-                    { id: 'photography', name: 'Photography' },
-                ]}
+            <Form
+                onSubmit={jest.fn()}
+                render={() => <SelectArrayInput {...defaultProps} />}
             />
         );
         expect(queryByText('Programming')).toBeNull();
@@ -62,136 +60,189 @@ describe('<SelectArrayInput />', () => {
 
     it('should use optionValue as value identifier', () => {
         const { getByRole, getByText, getByLabelText } = render(
-            <SelectArrayInput
-                {...defaultProps}
-                optionValue="foobar"
-                choices={[{ foobar: 'M', name: 'Male' }]}
+            <Form
+                onSubmit={jest.fn()}
+                render={() => (
+                    <SelectArrayInput
+                        {...defaultProps}
+                        optionValue="foobar"
+                        choices={[
+                            { foobar: 'programming', name: 'Programming' },
+                        ]}
+                    />
+                )}
             />
         );
         getByRole('button').click();
-        getByText('Male').click();
-        expect(getByLabelText('resources.bar.fields.foo').value).toBe('M');
+        getByText('Programming').click();
+        expect(getByLabelText('resources.posts.fields.categories').value).toBe(
+            'programming'
+        );
     });
 
     it('should use optionValue including "." as value identifier', () => {
         const { getByRole, getByText, getByLabelText } = render(
-            <SelectArrayInput
-                {...defaultProps}
-                optionValue="foobar.id"
-                choices={[{ foobar: { id: 'M' }, name: 'Male' }]}
+            <Form
+                onSubmit={jest.fn()}
+                render={() => (
+                    <SelectArrayInput
+                        {...defaultProps}
+                        optionValue="foobar.id"
+                        choices={[
+                            {
+                                foobar: { id: 'programming' },
+                                name: 'Programming',
+                            },
+                        ]}
+                    />
+                )}
             />
         );
         getByRole('button').click();
-        getByText('Male').click();
-        expect(getByLabelText('resources.bar.fields.foo').value).toBe('M');
+        getByText('Programming').click();
+        expect(getByLabelText('resources.posts.fields.categories').value).toBe(
+            'programming'
+        );
     });
 
     it('should use optionText with a string value as text identifier', () => {
         const { getByRole, queryByText } = render(
-            <SelectArrayInput
-                {...defaultProps}
-                optionText="foobar"
-                choices={[{ id: 'M', foobar: 'Male' }]}
+            <Form
+                onSubmit={jest.fn()}
+                render={() => (
+                    <SelectArrayInput
+                        {...defaultProps}
+                        optionText="foobar"
+                        choices={[{ id: 'programming', foobar: 'Programming' }]}
+                    />
+                )}
             />
         );
         getByRole('button').click();
-        expect(queryByText('Male')).not.toBeNull();
+        expect(queryByText('Programming')).not.toBeNull();
     });
 
     it('should use optionText with a string value including "." as text identifier', () => {
         const { getByRole, queryByText } = render(
-            <SelectArrayInput
-                {...defaultProps}
-                optionText="foobar.name"
-                choices={[{ id: 'M', foobar: { name: 'Male' } }]}
+            <Form
+                onSubmit={jest.fn()}
+                render={() => (
+                    <SelectArrayInput
+                        {...defaultProps}
+                        optionText="foobar.name"
+                        choices={[
+                            {
+                                id: 'programming',
+                                foobar: { name: 'Programming' },
+                            },
+                        ]}
+                    />
+                )}
             />
         );
         getByRole('button').click();
-        expect(queryByText('Male')).not.toBeNull();
+        expect(queryByText('Programming')).not.toBeNull();
     });
 
     it('should use optionText with a function value as text identifier', () => {
         const { getByRole, queryByText } = render(
-            <SelectArrayInput
-                {...defaultProps}
-                optionText={choice => choice.foobar}
-                choices={[{ id: 'M', foobar: 'Male' }]}
+            <Form
+                onSubmit={jest.fn()}
+                render={() => (
+                    <SelectArrayInput
+                        {...defaultProps}
+                        optionText={choice => choice.foobar}
+                        choices={[{ id: 'programming', foobar: 'Programming' }]}
+                    />
+                )}
             />
         );
         getByRole('button').click();
-        expect(queryByText('Male')).not.toBeNull();
+        expect(queryByText('Programming')).not.toBeNull();
     });
 
     it('should use optionText with an element value as text identifier', () => {
         const Foobar = ({ record }) => <span>{record.foobar}</span>;
         const { getByRole, queryByText } = render(
-            <SelectArrayInput
-                {...defaultProps}
-                optionText={<Foobar />}
-                choices={[{ id: 'M', foobar: 'Male' }]}
+            <Form
+                onSubmit={jest.fn()}
+                render={() => (
+                    <SelectArrayInput
+                        {...defaultProps}
+                        optionText={<Foobar />}
+                        choices={[{ id: 'programming', foobar: 'Programming' }]}
+                    />
+                )}
             />
         );
         getByRole('button').click();
-        expect(queryByText('Male')).not.toBeNull();
+        expect(queryByText('Programming')).not.toBeNull();
     });
 
     it('should translate the choices', () => {
         const { getByRole, queryByText } = render(
-            <SelectArrayInput
-                {...defaultProps}
-                choices={[
-                    { id: 'M', name: 'Male' },
-                    { id: 'F', name: 'Female' },
-                ]}
-                translate={x => `**${x}**`}
-            />
+            <TranslationContext.Provider
+                value={{
+                    translate: x => `**${x}**`,
+                }}
+            >
+                <Form
+                    onSubmit={jest.fn()}
+                    render={() => <SelectArrayInput {...defaultProps} />}
+                />
+            </TranslationContext.Provider>
         );
         getByRole('button').click();
-        expect(queryByText('**Male**')).not.toBeNull();
-        expect(queryByText('**Female**')).not.toBeNull();
+        expect(queryByText('**Programming**')).not.toBeNull();
+        expect(queryByText('**Lifestyle**')).not.toBeNull();
     });
 
-    it('should displayed helperText if prop is present in meta', () => {
+    it('should display helperText if prop is specified', () => {
         const { queryByText } = render(
-            <SelectArrayInput {...defaultProps} helperText="Can I help you?" />
+            <Form
+                onSubmit={jest.fn()}
+                render={() => (
+                    <SelectArrayInput
+                        {...defaultProps}
+                        helperText="Can I help you?"
+                    />
+                )}
+            />
         );
         expect(queryByText('Can I help you?')).toBeDefined();
     });
 
     describe('error message', () => {
         it('should not be displayed if field is pristine', () => {
+            const validate = () => 'Required field.';
             const { queryByText } = render(
-                <SelectArrayInput
-                    {...defaultProps}
-                    meta={{ touched: false, error: 'Required field.' }}
+                <Form
+                    onSubmit={jest.fn()}
+                    render={() => (
+                        <SelectArrayInput
+                            {...defaultProps}
+                            validate={validate}
+                        />
+                    )}
                 />
             );
-            expect(queryByText('Required field.')).toBeNull();
+            expect(queryByText('ra.validation.required')).toBeNull();
         });
 
         it('should be displayed if field has been touched and is invalid', () => {
+            const validate = () => 'Required field.';
             const { queryByText } = render(
-                <SelectArrayInput
-                    {...defaultProps}
-                    meta={{ touched: true, error: 'Required field.' }}
-                />
-            );
-            expect(queryByText('Required field.')).toBeDefined();
-        });
-
-        it('should be displayed even with an helper Text', () => {
-            const { queryByText } = render(
-                <SelectArrayInput
-                    {...defaultProps}
-                    helperText="Can I help you?"
-                    meta={{
-                        touched: true,
-                        error: 'Required field.',
-                    }}
+                <Form
+                    onSubmit={jest.fn()}
+                    render={() => (
+                        <SelectArrayInput
+                            {...defaultProps}
+                            validate={validate}
+                        />
+                    )}
                 />
             );
             expect(queryByText('Required field.')).toBeDefined();
-            expect(queryByText('Can I help you?')).toBeNull();
         });
     });
 });

From d735cc401fb9d14444b236aead36e43ecfe366bd Mon Sep 17 00:00:00 2001
From: djhi <gildas.garcia@gmail.com>
Date: Thu, 22 Aug 2019 17:24:01 +0200
Subject: [PATCH 2/2] Simplify component

---
 .../ra-ui-materialui/src/input/SelectArrayInput.js  | 13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/packages/ra-ui-materialui/src/input/SelectArrayInput.js b/packages/ra-ui-materialui/src/input/SelectArrayInput.js
index 6a56dce5c1d..d798450b9a2 100644
--- a/packages/ra-ui-materialui/src/input/SelectArrayInput.js
+++ b/packages/ra-ui-materialui/src/input/SelectArrayInput.js
@@ -159,15 +159,6 @@ const SelectArrayInput = ({
         ...rest,
     });
 
-    const handleChange = useCallback(
-        event => {
-            input.onChange(event.target.value);
-            // HACK: For some reason, redux-form does not consider this input touched without calling onBlur manually
-            input.onBlur(event.target.value);
-        },
-        [input]
-    );
-
     const renderMenuItemOption = useCallback(
         choice => {
             if (React.isValidElement(optionText)) {
@@ -221,7 +212,6 @@ const SelectArrayInput = ({
                 autoWidth
                 multiple
                 input={<Input id={id} />}
-                value={input.value || []}
                 error={!!(touched && error)}
                 renderValue={selected => (
                     <div className={classes.chips}>
@@ -241,8 +231,9 @@ const SelectArrayInput = ({
                     </div>
                 )}
                 data-testid="selectArray"
+                {...input}
+                value={input.value || []}
                 {...options}
-                onChange={handleChange}
             >
                 {choices.map(renderMenuItem)}
             </Select>