-
Notifications
You must be signed in to change notification settings - Fork 945
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
Migrate rest of forms to Final Form #845
Conversation
c84538a
to
baf91a1
Compare
3768228
to
e821221
Compare
<FormSpy onChange={handleChange} subscription={{ values: true, dirty: true }} /> | ||
<FieldCheckboxGroup | ||
name={name} | ||
id={`${name}-filter`} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have used either same string as in the name props or if the form is used several times, I have passed in a formId
id={formId ? `${formId}.phoneNumber` : 'phoneNumber'}
(If the name prop is already a good enough string, there's probably no need to differentiate name and id. One is used to identify form field "keys" and another for DOM element selection.)
<FinalForm | ||
{...rest} | ||
mutators={{ ...arrayMutators }} | ||
onSubmit={vals => console.log('SelectMultipleFilterPlainForm', vals)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is probably unnecessary
I didn't check this yet fully but looks OKish (you need to update changelog.md) |
e821221
to
7fd76f1
Compare
Move SelectMultipleFilterForm and SelectMultipleFilterPlainForm to the forms folder.
7fd76f1
to
8671675
Compare
return ( | ||
<FinalForm | ||
{...props} | ||
render={({ rootClassName, className, intl, handleSubmit }) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We haven't used this spread format of props with other components. It helps to debug code if there's full prop list available.
<Field | ||
name="location" | ||
format={null} | ||
render={({ input, meta }) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We haven't used this spread format of props with other components. It helps to debug code if there's full prop list available.
{...rest} | ||
onSubmit={() => null} | ||
mutators={{ ...arrayMutators }} | ||
render={({ className, name, options, twoColumns, onChange }) => ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We haven't used this spread format of props with other components. It helps to debug code if there's full prop list available.
<Field | ||
name="location" | ||
format={null} | ||
render={({ input, meta }) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We haven't used this spread format of props with other components. It helps to debug code if there's full prop list available.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good!
The one thing that I'd want to change is props spreading. Now that I started to think about this, we have had a rule to not spread props since that makes debugging harder (only picked props are easily available in inspector).
And one ToDo thing (sometime later) is to fix my mistake of using the word fieldRenderProps
in all the previous forms.
Change props spreading so that the whole props object is available to the form and required props can be destructured from that.
@Gnito The form props spreading/destructuring has now been changed so that all the forms provide the |
Migrate remaining forms from Redux Form to Final Form and remove the
redux-form
dependency. Also now all the form components can be found in the src/forms folder.The gzipped bundle size was reduced from 501.46 KB to 474.07 KB.