Proposed additions

[abalmos]

Per a Discord conversation instead of pulling my forms solution out of its project I refactored the project to use cerebral-module-forms because I liked its overall design more. However, the module is missing some features I need so here is an attempt at adding them.

I am aware that some or all of this may not make it or may need a second revision. Hopefully this PR will be a spring board for discussions. Sorry all of theses features are jammed into one PR, I have to go live with my project by the end of the week so I hacked together what I needed all at once :smile: That said, I tried my best to make one commit per feature.

@christianalfoni Let me know if you are not ready for PRs like this. I don't want to step on your new module too much!

What's in the PR

Bug Fixes:

1. equalsField was missing a .value

   New Features

2. Added a "Checkout" demo. It is based on one of the forms that inspired this PR.
3. Created an HOC. The checkout demo uses the HOC.
   • All the field's state becomes props of the wrapped component.
4. Added array field values to support multiple value inputs like <select multiple>
   • For an example see "How did you hear about us? (test select multiple)" of the Checkout demo
5. Added a defaultValue state key and provided a reset action and reset signal.
   • For an example see the reset buttons on all the demos.
6. Added a form validate action and signal.
   • For an example see the validate buttons on all the demos. Particularly the checkout demo.
7. Changed isTouched to only set for on blur and forced validations. We talked a little about this on Discord. I think it improves the UX.
   • Actually, I added a touched input key to the fieldChanged signal and then made the react parts do the right thing. There is probably a better solution.
   • For an example see any of the fields on all the demos. Particularly the checkout demo.
8. Added dependent fields for validation.
   • That is, when a field changes it causes a set of other fields in the form to re-validate.
   • Currently you can only specify what other fields to re-validate when the current field changes. You can not configure a field to re-validate when a one of a list of other fields change.
   • You can use relative paths, i.e., ., .., etc, or absolute paths to the dependent fields.
   • For an example see, "Email", "Verify Email", and "Verify Email, again..." of the checkout demo.

     New Features I Wanted To Get To But Ran Out Of Time, For Now...

9. Custom errors

   • Enable validators to returns error codes rather then just true/false. Use an object to configure the validator in state so that the developer can specify an error string for each error code. For example, 'isEmail' might return: 'invalid', 'missingAt', 'missingHost', etc.

   Form({
    email: {
      value: '',
      isRequired: true,
      validations: [{
        type: 'isEmail',
        errors: {
          invalid: 'Email is invalid',
          missingAt: 'Email is missing the required \'@\' sign',
          missingHost: 'Email s missing a host'
        }
      }
    }
   })

   i18n libraries could be used at this level.

10. Normalize functions.
    • A set of functions that take in the current value of field and then returns a new value. Right now I'm thinking they run before validation. This is mainly for formatting field values automatically, e.g., phone numbers, credit cards, etc. I think the function signature is something like function (value, previousValue, cursor, form, parameter). This could get somewhat complicated because the cursor position needs to be considered and updated appropriately too.

11. Updating the view based on form values. For example, show a picture of the credit card type as soon as it can be determined from the numbers entered. Change the label of the "CVV" field to "CID" for American Express, etc.

    • I don't think this requires any changes to the cerebral-module-forms. I just wanted the demo to include it so we have an example of how one could do it.
    • That said, in my uses cases the information needed to dynamically update/configure the view is also calculated by the relevant validators/normalizers. It could be nice to somehow let the validators/normalizers save its "meta" data to the state. Maybe validators/normalizers can return an object like:

    {
     valid: true,
     meta: { /* ... */ }
    }

    and

    {
      value: newValue,
      meta: { /* ... */ }
    }

    where the object at meta is stored in the field's state under a meta key. I want to avoid allowing the validators/normalizers to arbitrarily change a fields state because I think that is asking for trouble/abuse.

12. Update the README for the new features. I will wait to do that until it the various features are merged.
13. Figure out what the Cerebral style guide ended up being and apply it here.
14. Need to add tests.

    Open Questions/Thoughts:

15. Should we be suggesting to use cerebral-module-forms as a page sub-module and let the form state be owned by the cerebral-module-forms instance. Would/could that make the signals easier to use? We would avoiding having to register all the custom validators for an entire application in one place. However, then some rules might be included more than once across various forms.
16. Should add/remove field/form be signals? Should we instead just offer helper functions and let custom signals change the form state. At minimum people should be using the Form() function. Adding and removing fields/forms requires a custom view anyways.

[christianalfoni]

I really want to bring this in, though it would be nice to review it together maybe? Let me know when you have some spare time @abalmos. Would be cool to get it to a publishable version :)

[abalmos]

@christianalfoni Let me see if I can find a few hours this week to finish this up. I have some notes that I need to review from when I used it in a project.

[edgesoft]

@abalmos After resetting the Form options.map will be null when trying to render a select. options is undefined after reset. See Select/index.js

[edgesoft]

@abalmos We need to check the reset.js I think.

[christianalfoni]

Thanks for review @edgesoft ! I will wait for @abalmos final pull request before official release :)

[abalmos]

These changes (including fixes for the comments left by @edgesoft, thanks @edgesoft!) are now in the branch misc-features. I'm too chicken to update the branch this PR is based on because I don't want to break my production app :smiley:

I didn't merge into master because I know I will have trouble being around to support the code for a few weeks. The branch also lacks updated documentation (although there is a complete demo form). I will let @christianalfoni decide if he just wants to pull the trigger and merge or wait for some PRs on that branch to add docs.

[Guria]

@abalmos can you aslo make a rebase on top of latest master?

[abalmos]

@Guria the misc-features branch is this PR + a few commits rebased onto master

[Guria]

@abalmos ahh then we should open new pr instead of this one I suppose?

[abalmos]

I am closing this PR to avoid further confusion. Please track the misc-features branch for these changes. Unfortunately I can't update this PR because the current master uses cerebral API too new for my production application.

[abalmos]

@Guria I suppose we could. No semantic versioning here yet so I can just merge the branch into master when ready. The point of moving it into a branch on this repo was to enable some PRs on the feature branch from others in the meantime.

[edgesoft]

@abalmos @Guria @christianalfoni I'm using this in production now and It works great. No bugs so far. Could we pull misc-features into master branch or what is stopping us?

[abalmos]

@edgesoft probably the only thing that is stopping it is me not yet writing documentation on the new stuff. There is a breaking change or two. For example,touched changed meaning slightly.

I'll get doing this on my to-do list