Emerson / ember-form-master-2000

A simple form builder for Ember built as an Ember-CLI addon.
MIT License
40 stars 18 forks source link

Using form master with dockyard/ember-validations #15

Open constantm opened 9 years ago

constantm commented 9 years ago

Hi there!

I'm new to Ember and trying to get up and running with EFM2000 and ember-validations. I've gotten it to work up to a point, but I'm not sure where to go from here. At the moment, my validations work in the front end, which is great. The two problems I have are:

My code is as follows:

//app/controllers/clients/new.js

import Ember from 'ember';
import EmberValidations from 'ember-validations';

export default Ember.Controller.extend(EmberValidations.Mixin, {
  validations: {
    clientName: {
      presence: true,
      length: { minimum: 5 }
    },
    clientEmail: {
      presence: true,
      length: { minimum: 5 }
    }
  },

  actions: {
    saveClient: function(){
      var newClient = this.store.createRecord('client', {
          clientName: this.get('clientName'),
          clientEmail: this.get('clientEmail')
      });
      newClient.save();
      this.setProperties({
          clientName: '',
          clientEmail: ''
      });
    }
  }
});
//app/templates/clients/new.hbs

{{#fm-form action='saveClient'}}
    {{fm-field type='text' value=clientName label='Client Name' errors=controller.errors.clientName}}
    {{fm-field type='text' value=clientEmail label='Client Email' errors=controller.errors.clientEmail}}
    {{fm-submit value='Save Client'}}
{{/fm-form}}
//app/templates/models/client.js

import DS from 'ember-data';

export default DS.Model.extend({
  clientName: DS.attr('string'),
    clientEmail: DS.attr('string')
});

As said, I'm super new to Ember so please tell me if I'm being stupid. :)

Emerson commented 9 years ago

Looking into this and will get back to you soon. Funny enough, I've never used ember-validation, but it's about time!

Hope to have some examples of this in action soon.

Emerson commented 9 years ago

After doing some research I'm sad to report that I don't think this is possible using ember-validations. It turns out that ember-validations has strong opinions about how it's meant to be used, which you can read about in detail here: https://github.com/dockyard/ember-validations/issues/152. The idea that records start off in an invalid state is by design and there are currently no plans to change that.

The author of ember-validations (Brian) has been pretty clear that: "ember-validations is only concerned with the model layer. I suggest that you look into how ember-easyForm is displaying errors".

So basically, if you want to use ember-validations, you need to use ember-easyForms or I need to rework EFM2000 to be more inline with ember-validation's philosophy.

In my opinion there are very legitimate use cases for:

  1. Starting an entire record in an invalid state (ember-easyForm style)
  2. Transitioning an entire record to an invalid state on demand (on submit, but before server side validations)
  3. Transitioning individual fields to invalid states on demand (key-up validation or on blur)

Looks like it's time to write a validation library :clap: :tada: :clap:

I think I'm going to fork https://github.com/chriso/validator.js, convert it to an Ember-CLI addon, and write the code to handle these three use cases. I typically like validation libraries that provide validations (which is the hard part) and leave implementation up to the end user

srsgores commented 9 years ago

Ok, so I am just bringing this issue back up because I ran into the same problem.

Long story short: the validation methodology this library uses sucks. I think even @Emerson realizes this, as he suggested creating a validation library.

I am currently working on a re-work of validations, which use HTML5 validation: pattern, maxlength, required atrributes. It turns out that this library already implements some of these properties, as seen here.

Here is what I'm shooting for:

        {{#fm-form action="nextStep"}}
            {{fm-field type="text" label="First Name" value=applicant.firstName placeholder="First Name" required="required" autofocus="autofocus" title="What is your first name?"}}
            {{fm-field type="text" label="Last Name" value=applicant.lastName placeholder="Last Name" required="required" title="What is your last name?"}}
            {{fm-field type="email" label="Email" value=applicant.email required="required"  placeholder="Email" title="Please provide an e-mail"}}
            {{fm-field type="tel" label="Phone" value=applicant.phone required="required" placeholder="Phone" title="Please provide a valid phone number"}}
            <footer class="form-actions">
                {{#fm-submit title="Continue to Full Application"}}Continue Application{{/fm-submit}}
            </footer>
        {{/fm-form}}

Notice the autofocus and title attributes; when the required fails to validate, the error message displayed is the value of title. Note that this is the same functionality you get out of the box with HTML5.

For reference, I'm using ember-cli-html5 validations as a guide on this.

I will update as I get closer.

Emerson commented 9 years ago

Looking forward to hearing more @srsgores - I've been thinking things through and it's actually a really hard problem.

I can totally appreciate the logic behind ember-validations... which is basically "This library is used to determine validation states, not handle the display logic". On the other side of that, EFM2000 is kind of taking the same approach, which is "you figure out the validation states and just pass in the errors when you have them".

I've always used server side validations, which is why I never really considered this issue – but I fully agree with the sentiment that client side validations are an absolute must for any self-respecting form library and EFM2000 needs a way to accommodate this.

The big question I have right now is around API. What should it look like? I'm going to be putting more thought into this, but welcome any suggestions.

constantm commented 9 years ago

Okay so it seems I'm not completely stupid then. :) I'm just wondering what people generally use for validations with something like EFM2k. Like, you can't have form in an app without validations.

Emerson commented 9 years ago

I usually don't use live validations (on-keyup/blur)) and just let the server respond with the errors. Ember Data and EFM2000 should handle this pretty well out of the box, or you can explicitly attach the errors to your model object if you're not using Ember Data:

actions: {
  submit: function() {
    var user = this.get('currentModel');
    user.save().catch(function(xhr) {
        // Server returns something like: xhr.errors.firstName = ['required field', 'too short']
        user.set('errors', xhr.errors);
    });
  }
}

You can also see a manual version of this in the demo app here.

constantm commented 9 years ago

Aaah okay. That way is not ideal for a responsive user experience. I think in Ember it does actually make sense that a model object is invalid the moment it gets created. Perhaps an easier/more flexible solution would be for EFM2k to have options for when to display messages, along with some sort of good default. At the moment it shows all the time, but I think adding in an option to display the errors from the model per field on change/blur would go a long way. Then, if you don't want that per field functionality, errors could also be invoked for the whole form by toggleErrors. Would something like this be viable?

emablekos commented 9 years ago

Bump to this. I think it makes sense for the model to know its invalid, but the form view not to care until you have defocused.

Could get a little hairy on form reset, but seems like you just need a some internal state on each field to know if it has been defocused.

In the opensource spirit, it would be better to fit in with EmberValidations than add another validations lib to the mix.

For now i have a computed property proxying the errors property so i can toggle it: formSubmittedOnce:false, formErrors:Ember.computed('formSubmittedOnce', function() { return this.formSubmittedOnce ? this.errors : {}; }),

Emerson commented 9 years ago

Cool, agreed that we need to tackle this. Trying to think through the API before writing the code. I'm thinking this could be something like:

{{fm-form delayErrors=true}}
{{/fm-form}}

Which would prevent the form from render the initial errors.

What about individual fields? Maybe you can define it on the entire form or alternatively, define it on individual fields as needed...

{{fm-field delayErrors=true}}
srsgores commented 9 years ago

Out of the box, validation should occur on the client-side. delayErrors is misleading to me in that errors won't necessarily have to be delayed. Perhaps validateOnServer would be a better bet.

In general, though, I think the best validation pattern is to have validation on the client and on the server; in most cases, I think it makes most sense to have basic validation on the client, with any validation required by the server as an additional layer. So for this add-on, it might make sense to get rid of an option altogether, in favour of an approach which lists any server-side errors alongside the client-side ones.

Emerson commented 9 years ago

Not sure I fully agree with this. Validating on the client is nice UX, but validating on the server is required and needs to be done regardless of how you build your application. I've always seen client side validations as an enhancement rather than a requirement.

This being said, I think there is room for both options. This could be something that could also be set in the initializer. If there really are more people who feel that client side validation should be the default, I'd be fine with turning this delayedErrors option on by default and allowing an override in the initializer.

Busy for the next couple days, but planning on working through this soon. It's possible we'll find a more elegant solution once we start writing the code.

Happy to hear all sides of it!

ScottSmix commented 9 years ago

Just thinking out loud here... Because there can be "n" number of validations per input field each with their own default message, it seems like the pattern below might be desirable. Any number of "fm-validator" can be nested inside a field. Each type of validator (except custom) would have a standard message which could be overridden.

{{#fm-form action='submit'}}

{{#fm-field type='text' value=model.first_name label='First Name'}} {{fm-validator type="required" value=true}} {{fm-validator type="custom" function="myCustomTest" message="You cannot be named John"}} {{/fm-field}}

{{#fm-field type='password' value=model.password}} {{fm-validator type="regex" value="^[a-zA-Z]\w{3,14}$" message="Invalid password" more="The password's first character must be a letter, it must contain at least 4 characters and no more than 15 characters and no characters other than letters, numbers and the underscore may be used" }} {{fm-validator type="exclude" value="A123" message="A123 is just too simple"}} {{/fm-field}}

{{#fm-field type='text' value=model.age}} {{fm-validator type="between" minValue=18 maxValue=65}} {{/fm-field}}

{{fm-submit value='Create'}} {{/fm-form}}

Validators could initially be simple like required, minLength, maxLength, minValue, maxValue, "regex", "between", "outside", "numberOnly", "custom" (function) and maybe a couple more. The idea would be to loop through the form field components and each validator sub-component, test and ultimately show error message if any don't pass.

constantm commented 9 years ago

I feel like these should be set in the controller, and ideally be supported by the validation library used. That is if FM2k doesn't get it own baked in validator.

Emerson commented 9 years ago

I have a branch that I've been working on that offers delayed errors - so that fields/forms (can optionally) only display errors after blur or on update. This would give us better compatibility with the easyForm validations.

@ScottSmix - that's a pretty interesting idea, something I would have never considered, but I'm seeing some value in it. Love how explicit it is and how easy it would be to add your own validation components. I'm going to mull it over for a little.

It's really hard. I like the idea of validating everything in controllers or components, but it requires a lot of boiler plate if you want to add that to every form.

Looking at liquid-fire as a reference, I'm starting to think of a pattern that might define a few out of the box validations, but also allow users to add validators to their app as small ES6 modules. These modules could be generic and used across multiple form fields, or more specific if you need something really custom. Using standard ES6 modules for validations will also allow the validators to be used in different contexts, not just as template helpers.

// A specific validator to ensure only people named Tom are valid
// your-app/validators/name-equals-tom-validation.js
export default function(value, errors, event, model) {
  /*
  * value: The value of the field
  * errors: The errors object, allowing you to change other validation fields
  * event: The event that triggered this validation cycle (submit, change, keyUp, blur)
  * model: The model object so you can compare other values
  */
  errors.name = [];
  if(value !== 'Tom') {
    errors.name = ['Your name must be Tom']
  }
  return errors;
}

Meanwhile in your template

{{!-- Referencing a custom validation using the validate-with attribute --}}
{{fm-field validate-with='name-equals-tom' model=model}}

I think that pattern for specific validations could work, but I'm struggling to think of an API that would allow users to easily define multiple validations on a single field. Two options for generic validations that I can think of:

{{!-- comma separated attribute --}}
{{fm-field validate='minLength=10,maxLength=20,email=true,required=true'}}

{{!-- sub expressions --}}
{{fm-field (validate minLength="25" maxLength="20" email="true" required="true") }}

Any thoughts on this stuff? I'm personally feeling pretty good about it.

ScottSmix commented 9 years ago

@Emerson, thank you for taking the time to mull this over. I like the idea of defining validators as components. I think the problem with a pattern that includes the validators inside the fm-field element is that customizing messages or adding additional properties to each validator type would be messy. I think having the nested components is clean and would allow anyone to develop validators without syntax limitations. New validators should extend a validator base class.

So, I'm still leaning toward something like:

  {{#fm-field type="text" label="Name" value=model.name}}
    {{fm-field-validator type="required" value=true}}
    {{fm-field-validator type="custom" value="name-equals-tom"}}
    {{my-own-validator anyProperty=x someOtherProperty=y}}
  {{/fm-field}}

In the ES6 example, I think it would be good to consider updating ember-data .errors if the model is an ember-data model. By considering this, we can have server side errors and client side errors unified. Developers could also push errors into the unified errors from controllers too. So, validators would just run and push into a errors. Controllers could push there too. FM2K could have a standard template to display all this on the form. This template could be modified by individual developers as needed.

I also think that whatever the solution is that we should keep in mind that there can be several errors for the form and several errors for each input within the form. So the stock template for fm-field should show the label, the input and "n" number of errors.

Emerson commented 9 years ago

@ScottSmix - going to do some experiments and look into your suggestion, it seems super clear and very flexible.

I might start a branch where we can play around with the concept

Emerson commented 9 years ago

^ Feel free to hack on this - I imagine the internal API's will be changing significantly.

Emerson commented 9 years ago

@ScottSmix - made some initial progress using your syntax suggestion (not pushed yet). I think it's going to work.

ScottSmix commented 9 years ago

Very cool! I'm excited to see this!

constantm commented 9 years ago

This looks amazing. Yay for baked-in validation functionality!

blimmer commented 9 years ago

I have to say, I disagree with the direction this is going. Validations as components are unwieldy to change at runtime. Consider an app where validations often change (based on input from the server) and need to be computed. Listing them in block format in the template makes it really hard to do that.

Emerson commented 9 years ago

@blimmer - Server side errors will still be fully supported out of the box. These updates would really be based around client-side validations. I think the use case your talking about is pretty specific, which you'd probably end up handling in a custom validation component (it could listen to model changes and apply validation as needed). This being said, still happy to hear other suggestions. Ideally we can code for everyone's use case.

blimmer commented 9 years ago

There are times that a client-side validation can be affected by model state, or otherwise need to be computed.

Having to figure out how to make changes to helpers defined in a template seems like a strange abstraction to have to deal with. I don't see why the template is the right place to define these validations (as opposed to in a controller/component definition).

wvteijlingen commented 9 years ago

I'm currently searching for a nice form library to use, which brought me here. This looks like a great library, except for the validation troubles. I'm now using the feature/delayed-validation branch, which works nice. It'll already solve a lot of problems if this could be merged into master.

I agree with @blimmer that validations as components isn't the way to go. I think a template should mostly be concerned about the layout of an application, while validation is business logic IMHO. Having an errors property allows flexibility on where the validation is performed. You can then put the validation in the model by using something like dockyard/ember-validations, or in the controller when the form is backed by a model.

The API docs say: "An Ember.Component is a view that is completely isolated". This, I think, is a big pointer that components are not meant for this. Validations are not views.

jelhan commented 9 years ago

Just want to mention that ember-cp-validations using same approach as ember-validations.

blimmer commented 9 years ago

The big difference is that ember-cp-validations is using computed properties vs. observers and is much more well-maintained than the ember-validations package.

Emerson commented 9 years ago

Happy to look at that also - still have a bit more work to do to get full 2.0 support, then we can look at new features like validation :-)

jelhan commented 9 years ago

@Emerson I'm with @blimmer and @wvteijlingen that validation should not be part of ember-form-master-2000. As it's a form builder addon it should only care about displaying validation errors. Form validation is a complex topic. Not being coupled with a specific validation library or forced to a build-in validation logic is one of the main advantages of ember-form-master-2000 in my opinion.

tobsch commented 8 years ago

+1

g-cassie commented 8 years ago

Just chiming in to agree that validations should not be part of fm-2000. The one thing that might be helpful is some controls to make it easier to display/hide validations. I think that if you have a validation error you should be able to set the input components .errors to the error and then separately decide whether to actually display an error message yet. This will let you do things like show errors only after user has attempted to submit the form or show errors only after the user moves to the next input. Right now I am basically proxying the errors property with boolean switches to achieve this kind of functionality.

jelhan commented 8 years ago

Are there really use cases when errors should be shown before user interaction (focus out / submit)? For server-side validations showing errors only after submit should be fine cause before submit there aren't any server-side errors. Displaying errors in a form which wasn't touched yet, seems to be confusing. So if there aren't any use cases I would not make this an option.

wvteijlingen commented 8 years ago

Agreed, that should simplify things. You could expose a showErrors property that can be set to true right away. That way you can still show errors on a fresh form if you'd like to.

On 15 Nov 2015, at 16:20, jelhan notifications@github.com wrote:

Are there really use cases when errors should be shown before user interaction (focus out / submit)? For server-side validations showing errors only after submit should be fine cause before submit there aren't any server-side errors. Displaying errors in a form which wasn't touched yet, seems to be confusing. So if there aren't any use cases I would not make this an option.

— Reply to this email directly or view it on GitHub.