Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Elements should support manual (async) validation #36

Open
jonjamz opened this issue Jan 30, 2015 · 11 comments
Open

Elements should support manual (async) validation #36

jonjamz opened this issue Jan 30, 2015 · 11 comments

Comments

@jonjamz
Copy link
Owner

jonjamz commented Jan 30, 2015

Imagine being able to call a Meteor method during validation--it would open up a world of possibilities for Elements.

ReactiveForms.createElement({
  template: 'someTemplate',
  validationEvent: 'change',
  validationFunction: function (value, callbacks) {
    Meteor.call('secureCheck', value, function (err, res) {
      if (res) {
        callbacks.valid(value);
      } else {
        callbacks.invalid();
      }
    }
  }
});

There would have to be a local {{validating}} state for when it's loading.

@jonjamz
Copy link
Owner Author

jonjamz commented Jan 30, 2015

Could go hand-in-hand with support for async custom validation in simple-schema (which would just hook into this functionality).

https://github.com/aldeed/meteor-simple-schema#asynchronous-custom-validation-on-the-client

@steph643
Copy link

This looks complex and I cannot imagine a real life use case for this. I don't see why you would want to perform per-element async validation when you do a per-form async validation anyway in your action function.
Maybe what is lacking in the package is just a way to set a per-element error message from the action function?

@jonjamz
Copy link
Owner Author

jonjamz commented Jan 30, 2015

Of course data should be validated on the server (or at least pass the allow/deny rules for a collection).

I think the classic use-case for something like this is to do real-time checks on unique values (like the availability of a username) but I could also see it being used for website validation and real-time checks using third-party HTTP APIs.

@steph643
Copy link

If, by real-time checks, you mean checking on each keypress over the Internet, I am doubtful this is reasonable in most real-life cases. If you mean checking on submit, then this is what the action function is for.

@jonjamz
Copy link
Owner Author

jonjamz commented Jan 30, 2015

I mean checking before submit, but not necessarily on every keypress. If it was based around keypresses, it would have to be throttled or debounced.

Anyway, you're right for the vast majority of cases--having an immediate remote check isn't that important and it can be handled inside the action function. In fact, doing a remote check at the time of input could end up being pointless where time-sensitivity is a factor.

It may be useful to be able to override validation on certain types of elements, but I think I'll shelve this issue until I or someone else encounters a real-world use case where it's required.

@jonjamz jonjamz closed this as completed Jan 30, 2015
@jonjamz
Copy link
Owner Author

jonjamz commented Feb 17, 2015

I've just run into a case where I'm using two elements as sub-elements inside another element. One element is used to provide a select box, and the selection from that box is used to show items in another list with checkboxes.

In this case, being able to control which change event triggers validationValue using a manual this.validate(val) call that short circuits the automatic validation is really useful.

I've added this functionality for the next release--it's invisible unless someone wants to use it.

@steph643
Copy link

steph643 commented Apr 7, 2015

I have found a use case: I want the user to input an image URL and I want to validate it client-side (i.e. checking that it actually points to an image). This validation is asynchronous. How should I move forward with this?

@jonjamz
Copy link
Owner Author

jonjamz commented Apr 8, 2015

You'll want to return this.stop in validationValue and call this.validate(/* URL */) from the async callback in your HTTP call if it exists (you'll probably need to use var self = this;). From there, the URL will be stored and reactively validated using SimpleSchema.

The current implementation can be improved. For example, right now there's no this.invalidate() method. I look forward to your comments. Thanks!

@steph643
Copy link

steph643 commented Apr 8, 2015

It works, thanks!

However, it is a bit hacky, because now my UI element contains a validation function, meaning it is not re-usable anymore.

To somehow fix this, and in case it can help other folks, I added this function to all my elements:

    validationValue: function (el, clean, template) {
        var self = this;

        // Get the element value
        var value = $(el).val();

        // Support for the template argument 'customValidate'
        if (template.data.customValidate) {
            // 'customValidate' should be a function
            if (! _.isFunction(template.data.customValidate))
                throw new Error('customValidate should be a function');

            // Get the schema context and reset it
            var schemaContext = Template.parentData().context.schemaContext;
            schemaContext.resetValidation();

            // Get the field to check
            var field = template.data.field;

            // Put the form in the invalidated state
            // self.invalidate();

            // Inside the 'customValidate' callback (or asynchronously), call
            // "validationDone()" to indicate that the field is ok, or
            // "validationDone(errorType)" to indicate an error (errorType being a
            // simple-schema error message constant).
            var validationDone = function (errorType) {
                if (errorType)
                    schemaContext.addInvalidKeys([{ name: field, type: errorType }]);
                else
                    self.validate();
            };

            // Call the 'customValidate' callback
            // If 'customValidate' returns a falsey value, the initial schema
            // validation is not performed ('customValidate' replaces the original
            // schema validation). If 'customValidate' returns a truthy value, the
            // original schema validation is performed immediately, then
            // 'customValidate' is called to perform additional custom validation.
            if (template.data.customValidate(el, clean, template, validationDone)) {
                var objectToValidate = {};
                objectToValidate[field] = value;
                schemaContext.validateOne(objectToValidate, field);
            }

            // Quit, keeping validation in standby
            return this.stop;
        }

        // Standard behavior
        return clean(value, { removeEmptyStrings: false });
    }

So now I can use my elements like this

{{>bootstrapFormTextarea field="profile.picture" customValidate=th_validatePictureUrlFun}}
    th_validatePictureUrlFun: function () {
        return function (el, clean, template, validationDone) {
            var self = this;
            Meteor.setTimeout(function async() {
                var errorType= <my validation here, null if no error>
                self.validationDone(errorType);
            }, 3000);
            return true;
        };
    },

Maybe you should reopen this issue for a better fix in the future.

Some more thoughts : I am uncomfortable with the fact that validationEvent and validationValue are members of Element. It means I have to create a new element whenever I want to change validationEvent or validationValue. To me, elements are UI components that should be independent from data processing. You already made them independent from schema validation (which is a sub-part of data processing). But maybe they should also be independent from other data processing functions, such as validationEvent and validationValue.

@jonjamz
Copy link
Owner Author

jonjamz commented Apr 8, 2015

Ok, I'll reopen this because clearly there is more work to be done.

You've done something smart, essentially creating your own custom validation hook. This points me to the real issue, which is that custom validation shouldn't have to be done in validationValue methods.

I should probably take what you've done and work it into something more official, like an element-level validation hook that runs after validationValue, either extending or overriding whatever the default schema validation is for that field. And these hooks could potentially be queued. What do you think?

I'm already working on designing a schema API to decouple the package from SimpleSchema. I'll create another issue with those ideas so we can move this discussion there.

Some thoughts on validationValue and validationEvent being at the Element-level:

  1. Because an Element is a reusable UI component with a consistent DOM structure, the way an Element pulls data out of the DOM should be the same regardless of where it's instantiated.
  2. The validationValue function exists to allow component developers to specify how data is extracted from the DOM and how it's formatted before it gets sent to SimpleSchema (or, in the future, whatever type of validation is used). The validationEvent property also relates to this.

Because of the above points, it makes sense to keep validationValue and validationEvent at the Element-level, at least as options. It shouldn't be too hard for me to add support for template-level validationValue functions, though. Check this out:

https://github.com/meteortemplates/forms/blob/master/lib/module.coffee#L773

I could add self.data.validationValue into that expression, and then we could shift this discussion toward what kind of API improvements we can make inside validationValue for the time being.

@steph643
Copy link

steph643 commented Apr 9, 2015

I'm already working on designing a schema API to decouple the package from SimpleSchema

That is great news.

The validationValue function exists to allow component developers to specify how data is extracted from the DOM and how it's formatted before it gets sent to SimpleSchema.

Got your point.

I could add self.data.validationValue into that expression

Looks like a good idea. Not sure about the name, though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants