jurassix / react-validation-mixin

Simple validation mixin (HoC) for React.
MIT License
282 stars 38 forks source link

isValid() method bug? #73

Open Mottoweb opened 8 years ago

Mottoweb commented 8 years ago

Hi,

I my app isValid returns false even when there are no errors in this.props.errors. It gets invalidated if i put valid input into any field, once all fields are with valid inputs, isValid returns true. I want to disable button only if a field is invalid. If user enters valid i want to keep the button active.

<button type="submit" className={this.getButtonClasses()}>
          Continue registration
</button>
getButtonClasses() {
    return classnames({
      'ui-btn': true,
      'ui-btn__disabled': !this.props.isValid(),
    })
}
jurassix commented 8 years ago

I'm a little confused by the wording, but I'll do my best to help; please clarify if I misinterpret what you are asking.

First - Which version of this library are you using? Also, which validation strategy and version are you using?

isValid returns false even when there are no errors in this.props.errors.

I have not seen this behavior before, can you put together a simple example case to exploit this? isValid simply examines the this.props.errors data structure and returns if it is empty or empty for the key provided. (https://github.com/jurassix/react-validation-mixin/blob/master/src/validationFactory.js#L22)

I want to disable button only if a field is invalid. If user enters valid i want to keep the button active.

Based on your sample code it looks like you are trying to disable the Submit button if the form is invalid?

There is currently a tradeoff with this library. The expectation currently, is that Submit can only be disabled if the form has been validated. In my demo app I allow the form to have an initial valid state, enabling the Submit button. If the user simply presses Submit without filling out the form, I call validate() to determine the form has many errors and abort the submit. (See onSubmit() - https://jurassix.gitbooks.io/docs-react-validation-mixin/content/overview/basic-example.html)

If you want to disable the Submit button when the form has an invalidation but there is no visible feedback to the user then you could change the way you display individual errors, but simply that is a current tradeoff with this library.

You can manually call validate() on componentDidMount() to initialize the form to an invalid state, which will disable the Submit button.

Hope this helps.

Mottoweb commented 8 years ago

Thanks for your reply.

I am using @5.4.0 and joi validation strategy.

I don't need to prevalidate the form to disable the button immediately on componentWillMount(), instead my goal is to have active submit button until there are errors on blur. Which should be achievable with isValid().

Unfortunately i cannot put together a working example to reproduce atm. Will try to do it asap. Maybe you have an idea where i can put a basic example together without spending much time?

jurassix commented 8 years ago

@Mottoweb here is the source to the demo if that helps you out.

Mottoweb commented 8 years ago

Ok, i am running your source locally. Here are the steps to reproduce the issue.

Note: i am using react dev tools.

  1. Launch your code in the browser and select Signup component in the react dev tools.
  2. Call $r.props.isValid(). Outputs true.
  3. Then enter a valid username, blur input. No errors shown.
  4. Call $r.props.isValid(). Outputs false. With no errors in form.

I expect isValid() to return true in step 4. Simply because there are no errors.

In my case i am using validorjs strategy in similar form. Calling isValid() on valid form returns false.

jurassix commented 8 years ago

Ok I can reproduce this too. I can get the correct response when i ask for a single element like: $r.props.isValid('username')

The bug seems to be inside the validator: https://github.com/jurassix/react-validation-mixin/blob/master/src/components/validationMixin.js#L89

Which validator are you using?

jurassix commented 8 years ago

oops looks like it in here instead: https://github.com/jurassix/react-validation-mixin/blob/master/src/validationFactory.js#L22

I'm taking a look to see if anything odd stands out.

jurassix commented 8 years ago

Ok this is probably inside the validator. I'm seeing the following:

When I focus on Username field and then blur away:

$r.props.errors = {username: ['"Username" must be a string']}

Then when I satisfy the username requirements my errors object is not properly cleared:

$r.props.errors = {username: undefined}

This will fail the isEmpty(errors) call in the isValid function.

jurassix commented 8 years ago

We could handle this with a change to isValid to check that all values are empty or fix this up stream in the validator. I'll take a look at the joi validator and see what the impact there would be. But since this library supports multiple strategies it may need to be fixed centrally, here.

Mottoweb commented 8 years ago

I think it is better to fix this centrally, because i am using validator js strategy, found joi to be a bit heavy for browser