NewOldMax / react-form-validator-core

Core validator component for react forms
MIT License
95 stars 44 forks source link

'required' validation not working #4

Closed ariesxje closed 6 years ago

ariesxje commented 6 years ago

https://github.com/NewOldMax/react-form-validator-core/blob/10347666389d78ad4c6660ee86903a29e80e0d50/src/ValidatorComponent.jsx#L35

validate() is never called with includeRequired, and https://github.com/NewOldMax/react-form-validator-core/blob/10347666389d78ad4c6660ee86903a29e80e0d50/src/ValidatorForm.jsx#L38 never validates required

NewOldMax commented 6 years ago

You can call it manually for now

<TextValidator
   ref={(r) => { this.input = r; }}
   ...
/>

onHandleSomeValue(value) {
    this.input.validate(value, true);
}
ariesxje commented 6 years ago

But if you specify in the validators e.g. validators={['required']}, it won't work.

NewOldMax commented 6 years ago

Yes, it's 'by design', because people don't expect that they will see required error always

ariesxje commented 6 years ago

Sorry, I'm not quite following. According to the documentation, I can set a field to be required by setting the validators to include 'required'. If a field is not mandatory, then I don't set required in the validators. What is the use case that a field is mandatory but we don't show required error?

If we do

onHandleSomeValue(value) {
    this.input.validate(value, true);
}

is it validating outside the default behavior? Even though, I think we should still allow to set 'required' in the validator if we want to always validate a required field. What we have now is always ignoring 'required' in the validators, which doesn't seem right.

NewOldMax commented 6 years ago

@ariesxje required validation isn't ignored, it's called in submit case It's more logical for common users to see required validation message only on input, not on every time edit

ariesxje commented 6 years ago

@NewOldMax Do you mean you want to isolate the required validation to only happen on submit even the instanceValidation is set to true? I added my own customised required validator, and I find it works perfectly fine. The user will only get the required error after they interact with the field (e.g. they enter something then delete it to make the field empty again). If you really want to make required an exception that it won't be validated by instanceValidation, I suggest to make it an independent prop, e.g.

<MyInput validators={['required','isEmail']} instantValidateRequired={false} />

Then you won't block the ability to instantly validate required if the user wants to. Although, I still find this use case not very common...

NewOldMax commented 6 years ago

You can create a PR for it if you have time, or wait some time till I found time

ariesxje commented 6 years ago

Cool. Will do it soon. Nice work by the way.

NewOldMax commented 6 years ago

implemented in 0.2.0