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

Improve onError validation #1730

Merged
merged 27 commits into from
May 6, 2020
Merged
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
5563142
Introduce reason-based validation
dmtrKovalenko Apr 30, 2020
70b66d4
Remove dead validation code
dmtrKovalenko Apr 30, 2020
e9a0687
Implement new validation for TimePicker and DateTimePicker
dmtrKovalenko Apr 30, 2020
4f2207f
Remove redux form docs example
dmtrKovalenko Apr 30, 2020
868358a
Add Formik with validation schema example
dmtrKovalenko Apr 30, 2020
679ea8d
More tests
dmtrKovalenko Apr 30, 2020
a501984
TimePicker validation tests
dmtrKovalenko Apr 30, 2020
a1a6b42
Move parsing min/max date up to the root component
dmtrKovalenko May 1, 2020
5ca8ef2
Use touched state in the formik example
dmtrKovalenko May 1, 2020
3af9888
Remove console.logs
dmtrKovalenko May 1, 2020
71981e0
Merge conflicts
dmtrKovalenko May 1, 2020
cbe5fc4
Fix lint and build errors
dmtrKovalenko May 1, 2020
3eddd8b
Remove visual regression flakiness with time validation
dmtrKovalenko May 1, 2020
f21609c
Remove emptyInputText
dmtrKovalenko May 1, 2020
754a95b
Fix validation tests
dmtrKovalenko May 1, 2020
58f12c7
Commit .size-snapshot
dmtrKovalenko May 1, 2020
e349496
Implement validation for DateRangePicker.tsx
dmtrKovalenko May 1, 2020
5bce1e4
Add DateRange validation tests
dmtrKovalenko May 1, 2020
e4166a7
Fix linter
dmtrKovalenko May 1, 2020
860d6f5
Fix broken design of date rangepicker input parsing
dmtrKovalenko May 5, 2020
c73d4e7
Merge conflicts
dmtrKovalenko May 5, 2020
8dedc04
Remove <Code> from formik examples
dmtrKovalenko May 5, 2020
66eb022
Update yarn.lock
dmtrKovalenko May 5, 2020
af381ef
Update docs/pages/guides/FormikOurValidation.example.tsx
dmtrKovalenko May 5, 2020
1201eef
Update docs/pages/guides/FormikOurValidation.example.tsx
dmtrKovalenko May 5, 2020
79cea16
Update docs/pages/guides/FormikOurValidation.example.tsx
dmtrKovalenko May 5, 2020
806b63c
Update new forms example to be more consolidated with @materail-ui/core
dmtrKovalenko May 6, 2020
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/layout/components/navigationMap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export const navItems = [
title: 'Guides',
children: [
{ title: 'Accessibility', href: '/guides/accessibility' },
{ title: 'Form integration', href: '/guides/form-integration' },
{ title: 'Form integration', href: '/guides/forms' },
{ title: 'CSS overrides', href: '/guides/css-overrides' },
{ title: 'Passing date adapter', href: '/guides/date-adapter-passing' },
{ title: 'Date management customization', href: '/guides/date-io-customization' },
Expand Down
8 changes: 3 additions & 5 deletions docs/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@
"@types/prismjs": "^1.9.1",
"@types/react": "^16.8.13",
"@types/react-kawaii": "^0.11.0",
"@types/react-redux": "^7.1.7",
"@types/sinon": "^7.0.13",
"@types/yup": "^0.28.0",
"@zeit/next-bundle-analyzer": "^0.1.2",
"@zeit/next-css": "^1.0.1",
"@zeit/next-mdx": "^1.2.0",
Expand Down Expand Up @@ -65,15 +65,13 @@
"react-jss": "^8.6.1",
"react-kawaii": "^0.16.0",
"react-markdown": "^4.2.2",
"react-redux": "^7.2.0",
"redux": "^4.0.5",
"redux-form": "^8.3.0",
"remark-slug": "^6.0.0",
"safe-json-stringify": "^1.2.0",
"sinon": "^9.0.2",
"styled-jsx": "^3.2.5",
"ts-loader": "^6.0.2",
"typescript": "^3.8.3"
"typescript": "^3.8.3",
"yup": "^0.28.5"
},
"devDependencies": {
"dotenv": "^8.2.0",
Expand Down
2 changes: 1 addition & 1 deletion docs/pages/demo/timepicker/TimeValidation.example.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { TextField } from '@material-ui/core';
import { TimePicker } from '@material-ui/pickers';

function TimeValidation() {
const [selectedDate, handleDateChange] = useState(new Date());
const [selectedDate, handleDateChange] = useState(new Date('2020-01-01 12:00'));

return (
<Fragment>
Expand Down
57 changes: 0 additions & 57 deletions docs/pages/guides/Formik.example.jsx

This file was deleted.

109 changes: 109 additions & 0 deletions docs/pages/guides/FormikOurValidation.example.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
import React from 'react';
import Grid from '@material-ui/core/Grid';
import TextField from '@material-ui/core/TextField';
import { Formik, Form, Field, FieldProps } from 'formik';
import { format, isWeekend, isWednesday } from 'date-fns';
import { DatePicker, DatePickerProps } from '@material-ui/pickers';

interface DatePickerFieldProps extends FieldProps, DatePickerProps {
getShouldDisableDateError: (date: Date) => string;
}

const DatePickerField: React.FC<DatePickerFieldProps> = ({
form,
field: { value, name },
maxDate = new Date('2100-01-01'),
minDate = new Date('1900-01-01'),
getShouldDisableDateError,
...other
}) => {
Copy link
Member

Choose a reason for hiding this comment

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

For consistency

Suggested change
const DatePickerField: React.FC<DatePickerFieldProps> = ({
form,
field: { value, name },
maxDate = new Date('2100-01-01'),
minDate = new Date('1900-01-01'),
getShouldDisableDateError,
...other
}) => {
function DatePickerField(props: DatePickerFieldProps) {
const {
form,
field: { value, name },
maxDate = new Date('2100-01-01'),
minDate = new Date('1900-01-01'),
getShouldDisableDateError,
...other
} = props;

Copy link
Member Author

Choose a reason for hiding this comment

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

What do you mean under consistency? function vs const?

Copy link
Member

Choose a reason for hiding this comment

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

I mean that we systematically write private components following this pattern:

function DatePickerField(props: DatePickerFieldProps) {
  const {
    form,
    field: { value, name },
    maxDate = new Date('2100-01-01'),
    minDate = new Date('1900-01-01'),
    getShouldDisableDateError,
    ...other
  } = props;

and for public ones:

const DatePickerField = React.forwardRef(function DatePickerField(
  props: DatePickerFieldProps,
  ref: React.Ref<HTMLElement>,
) {
  const {
    form,
    field: { value, name },
    maxDate = new Date('2100-01-01'),
    minDate = new Date('1900-01-01'),
    getShouldDisableDateError,
    ...other
  } = props;

const currentError = form.errors[name];
const toShowError = Boolean(currentError && form.touched[name]);

return (
<DatePicker
autoOk
clearable
minDate={minDate}
maxDate={maxDate}
value={value}
onError={(reason, value) => {
switch (reason) {
case 'invalidDate':
form.setFieldError(name, 'Invalid date format');
break;

case 'disablePast':
form.setFieldError(name, 'Values in the past are not allowed');
break;

case 'maxDate':
form.setFieldError(name, `Date should not be after ${format(maxDate, 'P')}`);
break;

case 'minDate':
form.setFieldError(name, `Date should not be after ${format(maxDate, 'P')}`);
break;

case 'shouldDisableDate':
// shouldDisableDate returned true, render custom message according to the `shouldDisableDate` logic
form.setFieldError(name, getShouldDisableDateError(value));
break;

default:
form.setErrors({
...form.errors,
[name]: undefined,
});
}
}}
// Make sure that your 3d param is set to `true` on order to not clear errors
onChange={date => form.setFieldValue(name, date, false)}
renderInput={props => (
<TextField
{...props}
name={name}
error={toShowError}
helperText={toShowError ? currentError ?? props.helperText : undefined}
// Make sure that your 3d param is set to `true` on order to not clear errors
onBlur={() => form.setFieldTouched(name, true, false)}
/>
)}
{...other}
/>
);
};

const FormikExample = () => {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const FormikExample = () => {
export default FormikOurValidation() {

Copy link
Member Author

Choose a reason for hiding this comment

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

I prefer export default to be always the last export. It is more readable.

Copy link
Member

Choose a reason for hiding this comment

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

We systematically use the other way around to 1. increase the code density of the demos, 2. reduce the duplicate of the name of the demo. I think that it's important we still to a single convention.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, but for now pickers are standalone project. And if we are not planning to migrate docs before v5 we could stick to the our internal conventions for now. (before we merge docs)

Copy link
Member

Choose a reason for hiding this comment

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

I think that we can break down the dicussion in 3 different topics:

  1. Standaloneness. The picker components are part of Material-UI, like any other official component. I think that we should treat the component here as they were inside the mono repository. Where the code is a hosted shouldn't influence the developer experience. I think that it should be to the own discretion of how the core team wants to work.
    Why? The consistency of the components is one of the important value propositions of the library, I think that steps in this direction improve the DX.
  2. Progressiveness. I think that we should leverage all the opportunities to reduce the gap in the "standard way". I think that any step in the long term direction is good to take. What the advantage of delaying it? I find it interesting to handle it as we go, as it will be one less change to handle in the future.
  3. Convention evolution. I think that we should evolve the convention as we learn from our users and the environment change. RFC sounds like the best framework to make changes.
    For history, we used to follow the approach of the pull request, it was optimized for reducing the switching cost between class and functional components. With hooks, this requirement is no longer relevant.

return (
<Formik onSubmit={console.log} initialValues={{ date: new Date() }}>
dmtrKovalenko marked this conversation as resolved.
Show resolved Hide resolved
{({ values, errors }) => (
<Form>
<Grid container>
<Grid item container justify="center" xs={12}>
<Field
name="date"
disablePast
component={DatePickerField}
shouldDisableDate={(date: Date) => isWeekend(date) || isWednesday(date)}
getShouldDisableDateError={(date: Date) => {
return isWeekend(date)
? 'Weekends are not allowed'
: 'Wednesdays are not allowed';
}}
/>
</Grid>

<Grid item xs={12} sm={12} style={{ margin: '24px' }}>
<pre>
<code>{JSON.stringify({ errors, values }, null, 2)}</code>
</pre>
</Grid>
</Grid>
</Form>
)}
</Formik>
);
};

export default FormikExample;
72 changes: 72 additions & 0 deletions docs/pages/guides/FormikValidationSchema.example.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
import React from 'react';
import { date, object } from 'yup';
import { Grid } from '@material-ui/core';
import { TextField } from '@material-ui/core';
import { Formik, Form, Field, FieldProps } from 'formik';
import { DatePicker, BaseDatePickerProps } from '@material-ui/pickers';

interface DatePickerFieldProps extends FieldProps, BaseDatePickerProps {
getShouldDisableDateError: (date: Date) => string;
}

const DatePickerField: React.FC<DatePickerFieldProps> = ({
field,
form,
maxDate = new Date('2100-01-01'),
minDate = new Date('1900-01-01'),
getShouldDisableDateError,
...other
}) => {
const currentError = form.errors[field.name];

return (
<DatePicker
autoOk
clearable
minDate={minDate}
maxDate={maxDate}
value={field.value}
onChange={date => form.setFieldValue(field.name, date, true)}
renderInput={props => (
<TextField
name={field.name}
{...props}
error={Boolean(currentError)}
helperText={currentError ?? props.helperText}
/>
)}
{...other}
/>
);
};

const schema = object({
date: date()
.required()
.min(new Date())
.max(new Date('2100-10-10')),
});

const FormikExample = () => {
return (
<Formik validationSchema={schema} onSubmit={console.log} initialValues={{ date: new Date() }}>
{({ values, errors }) => (
<Form>
<Grid container>
<Grid item container justify="center" xs={12}>
<Field name="date" disablePast component={DatePickerField} />
</Grid>

<Grid item xs={12} sm={12} style={{ margin: '24px' }}>
<pre>
<code>{JSON.stringify({ errors, values }, null, 2)}</code>
</pre>
</Grid>
</Grid>
</Form>
)}
</Formik>
);
};

export default FormikExample;
90 changes: 0 additions & 90 deletions docs/pages/guides/ReduxForm.example.jsx

This file was deleted.