Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFR] Refactor validators to not return translated errors #3339

Merged
merged 42 commits into from
Jun 25, 2019

Conversation

djhi
Copy link
Collaborator

@djhi djhi commented Jun 15, 2019

This is the first step to migrate from redux-form to final-form. Indeed, react-final-form validators won't receive props anymore, so our current third argument containing the translate function will always be empty.

This PR refactor the validators to return either a string which will be translated or an object with message and args properties.

I also introduced an unstyled ValidationError component which knows how to translate those errors. As it returns the resulting string inside a React Fragment, it can be defined in ra-core and used in any UI library.

Finally, I updated the official inputs to use this ValidationError. In the process, I have migrated their tests from enzyme to react-testing-library and ensured all inputs support the helperText prop.

  • Refactor validators
  • Introduced ValidationError component
  • Introduced InputHelperText component
  • Migrated inputs & their tests
  • Updated upgrade guide

@djhi djhi added this to the 3.0.0 milestone Jun 15, 2019
@djhi djhi requested a review from fzaninotto June 15, 2019 12:59
@djhi
Copy link
Collaborator Author

djhi commented Jun 15, 2019

I noticed that we don't handle errors and helperText the same way for all inputs. Some will display the error instead of the helperText, some will display both.

I think it would be better to handle this in a consistant way by introducing an InputHelperText component which will display both the helper text and the error.

@djhi djhi force-pushed the refactor-validators branch from 87baf9b to 5e7fa46 Compare June 19, 2019 14:08
@djhi djhi force-pushed the refactor-validators branch from 5e7fa46 to 9b02c03 Compare June 19, 2019 14:13
@djhi djhi changed the title Refactor validators to not return translated errors [WIP] Refactor validators to not return translated errors Jun 19, 2019
Copy link
Member

@fzaninotto fzaninotto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed first part

packages/ra-core/src/form/ValidationError.tsx Outdated Show resolved Hide resolved
packages/ra-core/src/form/ValidationError.spec.tsx Outdated Show resolved Hide resolved
packages/ra-core/src/form/ValidationError.tsx Show resolved Hide resolved
packages/ra-core/src/form/ValidationError.tsx Outdated Show resolved Hide resolved
packages/ra-ui-materialui/package.json Outdated Show resolved Hide resolved
packages/ra-ui-materialui/src/input/ArrayInput.spec.js Outdated Show resolved Hide resolved
packages/ra-ui-materialui/src/input/BooleanInput.js Outdated Show resolved Hide resolved
packages/ra-ui-materialui/src/input/DateInput.js Outdated Show resolved Hide resolved
packages/ra-ui-materialui/src/input/InputHelperText.tsx Outdated Show resolved Hide resolved
packages/ra-ui-materialui/src/input/NumberInput.js Outdated Show resolved Hide resolved
});

describe('error message', () => {
it('should not be displayed if field is pristine', () => {
const { container } = render(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please keep this test

packages/ra-ui-materialui/src/input/SelectInput.js Outdated Show resolved Hide resolved
packages/ra-ui-materialui/src/input/TextInput.js Outdated Show resolved Hide resolved
this.props.input.onBlur(value);
this.setState({ value });
};
useEffect(() => {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Obviously a code smell. This is the implementation of the previous componentWillReceiveProps. Hopefully this will not be needed after migrating to final-forms

@fzaninotto fzaninotto changed the title [WIP] Refactor validators to not return translated errors [RFR] Refactor validators to not return translated errors Jun 25, 2019
@fzaninotto fzaninotto merged commit 7bed423 into next Jun 25, 2019
@fzaninotto fzaninotto deleted the refactor-validators branch June 25, 2019 17:56
@fzaninotto
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants