christianalfoni / formsy-react

A form input builder and validator for React JS
MIT License
2.59k stars 436 forks source link

Add optional prop 'validateOnSubmit' to Form component #147

Open idream3 opened 9 years ago

idream3 commented 9 years ago

Add an optional property on to the Form component so we can disable the default 'validate on change' scenario and only validate on submit.

I will submit a pull request if I get some time :)

richardaday commented 9 years ago

I think this needs to be thought out some more. For my application, I would like to validate on blur of specific inputs. I'm in the initial stages of setting up my first Form with Formsy and am finding the error notification show up too quickly.

A good example is an email input with a validation of isEmail. As soon as you type the first letter of your valid email, showError() goes true.

I can also think of many situations where the current behavior makes sense: If you have an input with an isNumeric validation, then showError() should go true as soon as someone types in a letter.

richardaday commented 9 years ago

I did some browsing through the formsy-react code, and I do have one question: What is the purpose of formsy-react being able to manage both isValid and isRequired? Couldn't we have treated isRequired just like any other validation and make it set isValid to false when the value is not provided?

silvenon commented 9 years ago

@richardaday it is confusing, yeah :+1:

silvenon commented 9 years ago

What about the isExisty validator? Doesn't that do the same thing?

twisty commented 9 years ago

I suppose it could be technically possible to ditch the required prop, but I think it's there for convenience and discoverability...

It's a common UI pattern to present asterisks next to required fields, using the isRequired() function is perhaps "cleaner" than inspecting a component's validators to work out if the component is required or not (particularly as presenting a "required" indicator in the UI is often separate from presenting other validation errors).

Also, as there is a required attribute for HTML5 form controls, developers coming to this library are already likely to be familiar with setting a required attribute separately to other input constraints (specific input types, validations, etc.).

christianalfoni commented 9 years ago

Hi guys!

This is a very typical question, "why required?". The thing is that indicating if something is required is different than indicating validation. For example an empty input that validates as email is not invalid because it it empty, but it is invalid if it also is required :-) Confusing, yes, but let me try this:

Required defines what a missing value is, (no text, is false, is undefined etc.) and invalidates the input as "missing value" when that occurs.

Validations defines what values are valid, WHEN there is value in the input.

It is really difficult to express this in a good way :-)

christianalfoni commented 9 years ago

Adding a "validateOnSubmit" prop to the form is a good idea. My personal opinion is that users hate filling out a complete form and submit just to get an error in their face and figure out where it happened. Its better to notify at once... or on blur. But yeah, have to get new version out first, but lets keep this open :)

silvenon commented 9 years ago

@christianalfoni totally agreed, the difference is clear.

I think these are the 4 most common events to validate:

  1. on change (immediately as you type) – annoying because it screams before you had a chance to type it right
  2. when you stop typing (debounced) – IMO the coolest, but toughest to implement :grin:
  3. on blur – very cool, but works well only if you move focus to an element which is close, otherwise you might miss the error
  4. on submit – a bit too late and can be annoying for larger forms (convenient for server validations)

It would be nice if all 4 are implemented eventually.

mguterl commented 9 years ago

We are also interested in validate on submit and have been trying to get the current project to work this way and we're not having much luck.

idream3 commented 9 years ago

@silvenon nice summation of form events!

I didn't know about the debounced behaviour, but now I do I want to explore it.

@christianalfoni I don't understand why the required prop can take a validator. Shouldn't the required prop simply be present or not present (true or false) on the InputComponent?

Isn't the use case for required to:

  1. be able to style differenty
  2. be able to prevent submit if no value is present and then highlight validation error messages such as notEmpty, isNumeric or whatever is defined for the input.
silvenon commented 9 years ago

There are a few use cases, I outlined one of them in #174.

christianalfoni commented 9 years ago

@idream3 Its because a formsy element can be any kind of input. Lets say you have an element that is a list and you want it to be required, which is when the length of the value is 0 :-) You need required="hasLength" or something. Does that make sense?

But this feature is at the top of the priority list now, so it will be implemented very soon.

@silvenon Great tip on the debounce thingy, never thought of that :-)

christianalfoni commented 9 years ago

Hi guys!

I got a really interesting suggestion on a different issue. You should easily be able to achieve "validate on submit" by just adding a class to the form. This class hides all validation messages. When you hit submit, you remove the class and any validation errors is now displayed.

Does this solve the issue?

dptoot commented 9 years ago

I think this does allow for the functionality but doesn't solve the issue. The functionality you describe to remove the classes or render outright would need to be called in an onInvalidSubmit callback. At that point Formsy knows that the form is invalid and has errors. A possible built in method supplied as a 4th argument to show errors on child Formsy components could facilitate this globally.

My concern is having to code this behaviour in to all our forms.

dptoot commented 9 years ago

@christianalfoni Per your suggestion on Sept. 12th I am not sure that addresses the issue because a user would have had to interact with that input in some capacity. Whether it be onChange or onBlur any pristine but required validations aren't going to get processed. Unless I am doing something wrong (likely) it seems that required is actually falling back on HTML5 validation and bypassing formsy all together. If we are to use isExisty or something similar to enable custom and identical validation messages then we need some hook to process those rules.

uglow commented 8 years ago

I'm new to React, and the thing that's bugging me the most is the lack of a decent form library that allows custom components, showing errors when you want to (which is what this issue is about) and doesn't have the "disable submit button until valid" example which is a total UX anti-pattern.

Is anyone interested in creating a framework-agnostic form-policy implementation (such as the one here: https://odecee.github.io/angular-form-lib/#/formPolicy (note: I'm the main contributor)), which would allow React/Ember/Polymer/Angular users to specify when errors should appear?