Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Form-wide validation #86

Closed
maackle opened this Issue · 7 comments

4 participants

@maackle

It seems like the validators can only apply to particular fields, and while each validator has access to the whole form, it would be useful to have form-wide validation for more complex rules. For instance, I have a form that requires exactly one out of a list of fields to be filled and the rest empty. Having something like a second argument to forms.create where such a validator could be specified would be really helpful. Just a function(form, callback) would be fine. What do you think?

@ljharb
Collaborator

Interesting idea. Typically each validation error is tied to a field. While a form-level validator could certainly attach an error to a field, how would we enforce that a validator with any errors set at least one of them on a field?

In your use case, why wouldn't you have one text field, with a radio button for choosing the type? Then you'd just require the text field to not be empty, and the radio button to be selected.

@maackle

Form-wide errors would have to show up in a form-wide location, not tied to any field. In the HTML there would be one extra div to hold these errors at the top or bottom of the form. Errors attached to fields could be thought of as a convenient special case (though extremely common). That way any possible validation can be created with custom rules.

In my use case, I'm using three different fields -- input[type=text], input[type=file], and textarea -- which ideally would each have their own individual validation rules after it's decided which to use (but that would require some kind of validation chaining which is a different issue altogether). Just to get an idea, I want the user to be able to attach a document to the form, which can either come from a URL, a file upload, or plain text. This is the kind of form for which I would eventually write my own validation and flow by hand, but while prototyping it, it would be nice to have that validation in place.

@ljharb
Collaborator

I think that might be a conceptual issue - it seems very unusable to have an error that's not attached to user input. Errors attached to fields is absolutely the normal case - errors not attached to fields are very rare - and I'd like the library to reflect that.

What it sounds something like is that the format of the "content" field, and its validation, would be decided by the value of a "type" radio button - so you'd want the error associated with the "content" field.

I'm certainly open to API proposals - but at a certain point, what you want is beyond the scope of this library.

@stevetweeddale

I might be missing something here, but I'm implementing a login form using this library, and form-wide validation is what I'll have to fake up as it stands.

I'm validating an account number and postcode against a private API, which will either succeed and log the user in, or raise an error on the form that the pair of fields provided aren't a valid set of credentials. In the case of a login form like this, you don't really want your error raised on any specific field, lest you confirm/deny the validity of any specific field to nefarious users ("The postcode you provided didn't match the VALID account number we have on our system that you now know exists… oops"). Instead, you just want a big "Nope, something wasn't right, try again".

@ljharb what would the recommended way of doing this?

For now, what I'll probably do is implement corresponding validation handlers on both fields, assuming I can mark both as invalid without having to print multiple error messages to the page. But my point is that a second argument to the form creator function as @maackle suggested, or a post-form-creation form.addValidationHandler(callback) would make sense to me, and would be much nicer for this use case; which I don't think is that rare.

@ljharb
Collaborator

The use case for this is certainly a login form, which generally has only 2 important fields - username and password (in your case, account and postcode are the two factors you're requiring). I suppose I can see the usefulness of adding something like form.addValidationHandler(callback) etc, but it would be a rarity imo since although everyone is likely to have a login form, on a given site, you're likely to have many more forms that are not login forms. I'll think about how this should look - please feel free to submit a PR in the meantime :-)

@ljharb ljharb was assigned
@artnez

I'm leaning with @ljharb on this one. It is a conceptual issue and IMO validating the login form itself is orthogonal to testing the combined results of the fields.

I think this issue can be solved using callback chaining. I'll submit a PR in a minute. It's a minimal backwards compatible change and adds composability which addresses this use case and more.

@artnez

With callback chaining, here's how you would do the login thing (in Express, for example):

login: function(req, res) {
  forms.login.handle(req, {
    success: function(form, callbacks) {
      if (checkCredentials(form)) {
        res.redirect('/dashboard');
      } else {
        form.yourCustomErrorIndicator = 'login_failed';
        (callbacks.error || callbacks.other)();
      }
    },
    other: function(form) {
      res.render('login', {form: form});
    }
  });
}

yourCustomErrorIndicator is defined on the bound form. Use it in templates however you like.

@ljharb ljharb closed this in #129
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.