Migrate rest of forms to Final Form #845
Conversation
lyyder
force-pushed
the
final-form-migration
branch
from
c84538a
to
baf91a1
Compare
lyyder
force-pushed
the
final-form-migration
branch
from
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.
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.
This is probably unnecessary
|
I didn't check this yet fully but looks OKish (you need to update changelog.md) |
lyyder
force-pushed
the
final-form-migration
branch
from
e821221
to
7fd76f1
Compare
Move SelectMultipleFilterForm and SelectMultipleFilterPlainForm to the forms folder.
lyyder
force-pushed
the
final-form-migration
branch
from
7fd76f1
to
8671675
Compare
| return ( | ||
| <FinalForm | ||
| {...props} | ||
| render={({ rootClassName, className, intl, handleSubmit }) => { |
There was a problem hiding this comment.
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.
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.
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.
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.
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-formdependency. 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.