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

Using form master with dockyard/ember-validations #15

Open
constantm opened this issue Mar 31, 2015 · 32 comments
Open

Using form master with dockyard/ember-validations #15

constantm opened this issue Mar 31, 2015 · 32 comments

Comments

@constantm
Copy link

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:

  • The new template starts out with validation failed. How do I start with a non-validated object that updates per field as I go along?
  • Thehas-error class never gets removed, even if validation passes. I had a look and the errors object it's looking at is empty. Why could it be that the class doesn't get removed?

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
Copy link
Owner

Emerson commented Apr 1, 2015

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
Copy link
Owner

Emerson commented Apr 1, 2015

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 👏 🎉 👏

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
Copy link
Contributor

srsgores commented Apr 7, 2015

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
Copy link
Owner

Emerson commented Apr 7, 2015

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
Copy link
Author

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
Copy link
Owner

Emerson commented Apr 7, 2015

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
Copy link
Author

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
Copy link

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
Copy link
Owner

Emerson commented Apr 13, 2015

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
Copy link
Contributor

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
Copy link
Owner

Emerson commented Apr 13, 2015

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
Copy link

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
Copy link
Author

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
Copy link
Owner

Emerson commented May 7, 2015

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
Copy link

@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
Copy link
Owner

Emerson commented May 7, 2015

@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
Copy link
Owner

Emerson commented May 7, 2015

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

@Emerson
Copy link
Owner

Emerson commented May 11, 2015

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

@ScottSmix
Copy link

Very cool! I'm excited to see this!

@constantm
Copy link
Author

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

@blimmer
Copy link
Collaborator

blimmer commented Jun 4, 2015

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
Copy link
Owner

Emerson commented Jun 4, 2015

@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
Copy link
Collaborator

blimmer commented Jun 14, 2015

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
Copy link

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
Copy link
Contributor

jelhan commented Nov 4, 2015

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

@blimmer
Copy link
Collaborator

blimmer commented Nov 4, 2015

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
Copy link
Owner

Emerson commented Nov 4, 2015

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
Copy link
Contributor

jelhan commented Nov 4, 2015

@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
Copy link

tobsch commented Nov 14, 2015

+1

@g-cassie
Copy link
Collaborator

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
Copy link
Contributor

jelhan commented Nov 15, 2015

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
Copy link

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 [email protected] 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.

jelhan added a commit to jelhan/ember-form-master-2000 that referenced this issue Nov 20, 2015
Errors aren't shown before a user interaction. If user interacts with a field
errors for that field are shown. If user submits the form, errors for all field
of the form are shown.

What is treated as an user interaction differs by field. Only if an interaction
could be seen as finished, it's causing errors to be shown. All focus out events
are treated as finished user interactions. A change event on an input field isn't
to prevent errors from popping up, while user inserts a valid string (e.g. if a
minimum length is required). On the other hand an change on select and checkbox
is.

This topic was discussed intensively in Emerson#15. There were many different ideas how
it could be implemented. This PR tries to reflect the last state of the debate.

In opposite to branch feature/delayed-validations this is not optional. I don't
think there are use cases where you want to show errors for untouched fields to
the user. If there are use cases one could pass `true` to `showErrors` property
of fields.
There shouldn't be any changes for people using server-side validations, since
there these ones aren't present before a form is submitted.
jelhan added a commit to jelhan/ember-form-master-2000 that referenced this issue Nov 25, 2015
Errors aren't shown before a user interaction. If user interacts with a field
errors for that field are shown. If user submits the form, errors for all field
of the form are shown.

What is treated as an user interaction differs by field. Only if an interaction
could be seen as finished, it's causing errors to be shown. All focus out events
are treated as finished user interactions. A change event on an input field isn't
to prevent errors from popping up, while user inserts a valid string (e.g. if a
minimum length is required). On the other hand an change on select and checkbox
is.

This topic was discussed intensively in Emerson#15. There were many different ideas how
it could be implemented. This PR tries to reflect the last state of the debate.

In opposite to branch feature/delayed-validations this is not optional. I don't
think there are use cases where you want to show errors for untouched fields to
the user. If there are use cases one could pass `true` to `showErrors` property
of fields.
There shouldn't be any changes for people using server-side validations, since
there these ones aren't present before a form is submitted.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

10 participants