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) => ( ); -``` \ No newline at end of file +``` + +## helperText is handled the same way in all components + +Some components (such as ``) 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..d798450b9a2 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,140 @@ 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 || [], - }; - - componentWillReceiveProps(nextProps) { - if (nextProps.input.value !== this.props.input.value) { - this.setState({ value: nextProps.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 }); - 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 translate = useTranslate(); - renderMenuItemOption = choice => { - const { optionText, translate, translateChoice } = this.props; - if (React.isValidElement(optionText)) { - return React.cloneElement(optionText, { - record: choice, - }); - } + const { + id, + input, + isRequired, + meta: { error, touched }, + } = useInput({ + onBlur, + onChange, + onFocus, + resource, + source, + validate, + ...rest, + }); - 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 ? ( - - {this.renderMenuItemOption(choice)} - - ) : 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 . 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 ? ( + + {renderMenuItemOption(choice)} + + ) : null; + }, + [optionValue, renderMenuItemOption] + ); - return ( - + + + + } - value={this.state.value} - error={!!(touched && error)} - renderValue={selected => ( -
- {selected - .map(item => - choices.find( - choice => - get(choice, optionValue) === item - ) - ) - .map(item => ( - - ))} -
- )} - data-testid="selectArray" - {...options} - onChange={this.handleChange} - > - {choices.map(this.renderMenuItem)} - - {helperText || (touched && error) ? ( - - - - ) : null} -
- ); - } -} + + ) : null} + + ); +}; 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 +265,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 +276,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('', () => { 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( - +
} + /> ); expect(queryByTestId('selectArray')).toBeDefined(); }); it('should use the input parameter value as the initial input value', () => { const { getByLabelText } = render( - } /> ); - 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( - } /> ); expect(queryByText('Programming')).toBeNull(); @@ -62,136 +60,189 @@ describe('', () => { it('should use optionValue as value identifier', () => { const { getByRole, getByText, getByLabelText } = render( - ( + + )} /> ); 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( - ( + + )} /> ); 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( - ( + + )} /> ); 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( - ( + + )} /> ); 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( - choice.foobar} - choices={[{ id: 'M', foobar: 'Male' }]} + ( + 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 }) => {record.foobar}; const { getByRole, queryByText } = render( - } - choices={[{ id: 'M', foobar: 'Male' }]} + ( + } + 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( - `**${x}**`} - /> + `**${x}**`, + }} + > + } + /> + ); 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( - + ( + + )} + /> ); 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( - ( + + )} /> ); - 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( - - ); - expect(queryByText('Required field.')).toBeDefined(); - }); - - it('should be displayed even with an helper Text', () => { - const { queryByText } = render( - ( + + )} /> ); expect(queryByText('Required field.')).toBeDefined(); - expect(queryByText('Can I help you?')).toBeNull(); }); }); });