Refactoring #2

[ivan-kleshnin]

Implementation of https://github.com/jurassix/react-validation-mixin/pull/8 See also https://github.com/jurassix/react-validation-mixin/pull/10

[jurassix]

Thanks for your feedback Ivan. Always appreciated. I'll try to get some commits in this week to clean up the API and account for some of the discrepancies we have discussed.

[ivan-kleshnin]

I've added you as collaborator to my fork, so you may push right there and the changes will be reflected in this PR.

---

I'm not sure about this from a developers perspective, we cannot generically handle the submit without providing a hook or callback to execute if validation is successful. I have an idea about how to handle this but am still working it out. I'll try to push it soon and let you review.

I thought this simplest possible library version of handleSubmit can be useful as a reference at least. The user may copy-paste that code and override it when required. It may be treated as a special "refer & override me" kind of API :)

handleUnfocusFor does not account for the fact this event handler could be attached to an onChange event or really any event. I think we need to stick with the generic handleValidation or something like it to emphasize this fact.

React's onChange is something different from browser onChange, it's closer to smart keyUp. In this case, onChange version should include lodash.debounce wrapper, so I thought it's worth to split them. Maybe handleBlur + handleChange would be a better names (just mirror attribute names and are obvious about their intentions).

What I think is that API for any mixin can be designed in two ways.

1. Low-level, suitable for "every" use-case. Each method will contain only few lines of actual code (barely handy)
2. Middle level, closer to "reference and override if required" (mentioned above) kind. Each method is longer but with lower change to suit to every use-case.

Mixins are a special kind of libraries because they provide generally impure functions (which deal with state).

Back to the question. We discussed that mixin may be used not only for forms. The names of those helpers go against this statement cause they are named after HTML form actions. The more concrete names they have, the easier for end-user to pick them. But, at the same time, these concrete names push us to split the mixin in two parts or even projects like validation-mixin and form-mixin where second depends on the first. Am I right?

[jurassix]

I pushed my 4.0.0 release to ivan-kleshnin-master. I have solidified the API. Here are the release notes:

4.0.0 (Major release with breaking API changes):

• validatorData is now getValidatorData to be more idiomatic of React
• validate now takes an error-first callback, which allows for a simpler API when validating on Submit. validate also takes an optional key as a first param that can be used to validate a single form field.
• isValid is now a simple wrapper around this.state.errors see example for handling form submission in 4.x.x
• clearValidations has been added to reset all current errors.
• handleValidations now takes a second parameter error-first callback that can be used for custom handling of errors.

I'm going to release this version major tonight. I'll also update the gh-pages to match these changes.

Next I'll get the compound-keys #13 PR updated to match master and get it merged into 4.x.x

Let me know if there are any major objections to the next release. I've handled the isValid and the onSubmit cases to be clear. I'm adopting a node error-first callback style for validate calls.

[ivan-kleshnin]

Everything looks solid. I'm a bit puzzled by "validations" vs "errors" distinction. Is there any? P.S. Sorry, I forgot again that React does not allow same-named methods (no super) so my approach to handleSubmit and handleReset is surely wrong.

[jurassix]

By validations vs errors are your referring to this code in the demo?

    onValidate = function(error, validationErrors) {
      if (error) {
        this.setState({
          feedback: 'Form is invalid do not submit'
        });
      } else {
        this.setState({
          feedback: 'Form is valid send to action creator'
        });
      }
    }.bind(this);
    this.validate(onValidate);

Basically the second parameter, validationErrors is just the errors object returned by ValidationFactory.validate; it's equivalent to this.state.errors but is left in as a convenience. The real useful information to the user is errors parameter, which allows node style checking if there were errors during validation.

I've also beefed up the testing harness with the 4.0.3 release. I'll add a couple more UI tests to ensure any changes we make to the Factory doesn't break the users expectations. I'll also be working on merging in #11 after I have some time to digest and review.

[ivan-kleshnin]

I mean pure naming aspect. Somewhere it's errors like in this.state.errors, somewhere it's validations like in resetValidations / handleValidations, somewhere its validationErrors like in example above. Why not just errors so resetErrors / handleErrors / errors/ this.state.errors. I don't think there will be different errors in form components.