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

Already on GitHub? Sign in to your account

What is the current best practice for handling forms with the new router? #1692

Closed
darthdeus opened this Issue Jan 5, 2013 · 6 comments

Comments

Projects
None yet
6 participants
Member

darthdeus commented Jan 5, 2013

With all those changes to how {{action}} works and changes like 8fec737, I'm not really sure how to approach basic forms with validation. Here's an example JSFiddle, and now to the source code.

Validations

Many people on IRC suggested that using a computed property for client side validation is a good way to go, so I came up with this

{{#if controller.usernameInvalid}}username must be at least 5 characters{{/if}}

I suspect that if this is the correct pattern, then there should also be a isValid computed property which is a result of all of these property-specific validation properties.

Is there also a best practice for handling failed validations that come from the server (in Rails that would be 422 status code). The only thing that comes to mind here is putting an event handler on becameInvalid right in the form event handler, such as this

App.NewUserController = Ember.ObjectController.extend({

  createUser: function() {
    this.get("content").on("becameInvalid", function() {
      // possibly set some view property here to display additional errors?
    });

   App.store.commit();
  }
});

Router lookup

Since App.container.lookup is unsupported since 8fec737, how should we approach the case of a redirecting to a the newly created record from the controller? Is binding to a didCreate on the new record even considered a good practice?

General event handling for forms

Should the form even be submitted as a controller event? People often say that the controller is responsible for current client session while Ember Data is responsible for persistent session.

We could theoretically set the handlers in the router directly, so the code would look like

App.NewUserRoute = Ember.Route.extend({
  model: function() {
    var record = App.User.createRecord(),
        route = this;

    record.on("didCreate", function() {
      route.transitionTo("user", record);
    }

    return record;
  }
});

which is a solution to accessing the router form a controller, but now I still need to bind to the becameInvalid event from the controller. This means I'm binding to two different events (didCreate and becameInvalid) from two different places (router and controller).

At the moment this seems like the proper solution, but it also feels like I'm accessing a global user record in multiple places of the app and binding events wherever and however I like.

Property-specific validation attributes and the view

One last thing that pops up is when the usernameInvalid property gets calculated. For this simple example it works well in sense that the error message is only displayed after the user starts typing, but that is not good enough.

Even if I prevent the user from submitting the form somehow, I still want to display the error messages, which I can't do if he doesn't touch the username form field, because the usernameInvalid property won't get calculated.

An alternative option would be to have a form field which knows how to validate itself, something like App.RequiredTextField, but then again, how do I tell it to force-validate itself when the user clicks the submit button?

These questions often pop up on IRC, especially since the new router release, and everyone has a different answer. It would be good if we could come up with a general best practice, since forms are something which every web app will have.

Owner

wycats commented Jan 7, 2013

Use of controller in templates

You don't need to do {{#if controller.usernameInvalid}}. The default context is the controller so you can just do {{#if usernameInvalid}}

Validity in Ember Data

When using the RESTAdapter with Ember Data, there is an isValid flag that will become false when the server returns a 422. You can access the specific errors (from the server's error hash, assuming it provides one), via model.errors.fieldName.

Redirection from the Router

You probably want to have your router handle the transition:

App.NewUserRoute = Ember.Route.extend({
  model: function() {
    return App.User.createRecord();
  },

  events: {
    save: function(user) {
      user.one('didCreate', function() {
        this.transitionTo('showUser', user);
      }, this);

      this.get('store').commit();
    }
  }
});

In your template, you will have a {{action save user}}. Just a note that you can do <form {{action save user on="submit"}}>.

This will improve over time, as the most common use-cases become more fleshed out.

Validation Timing

What is the case where the user hasn't made any changes to the text field, but the field has become invalid between when the object was created and when the form was submitted?

@wycats wycats closed this Jan 7, 2013

Member

darthdeus commented Jan 7, 2013

Wow thanks for a detailed response, this helps a lot :)

As for the validation timing, the issue was that one of the properties actually changed from null to "", which caused the validation property to re-calculate and show the error. There is still one question though.

If I already have client side validations, I probably want to prevent the user from submitting the form if it is invalid and show all of the errors.

So basically the question is how do I trigger calculation for usernameInvalid-like properties, so that the errors show, even if the property it is bound to did not change?

If I add something like this to the handler

save: function(user) {
  if (user.get("usernameInvalid")) {
    alert("username is invalid");
    return false;
  }

  user.one('didCreate', function() {
    this.transitionTo('showUser', user);
  }, this);

  this.get('store').commit();
}

and the user just loads the form and clicks on submit, it won't trigger the conditional, because the property is not calculated yet.

Contributor

mikegrassotti commented Jan 7, 2013

AFAIK best practices for this kind of validation is still evolving. It would be really helpful to know how other's are approaching the problem, maybe we can establish patterns for the most common use cases.

@darthdeus sounds like the root issue you are getting at is how to handle validation separate from display of errors. In other words, just because a field is invalid (or blank) does not mean you want to tell the user about it. So maybe you'd want to have something at the view layer that hides error messages until a particular field has been modified.

Owner

machty commented Jan 7, 2013

@mikegrassotti I'd also like to hear how others handle these sorts of things, particularly multipage forms with menu items that let you navigate to a previously visited step in the form, but not past the current step until the form's been filled out and validations passed. I have something working that involves connectOutlet's setting a property on the controller that a particular step has been visited, and the links in the view can display themselves differently depending on that, but it's all a little messy IMO. Would love to see how others handle this sort of thing.

raila commented Jan 8, 2013

When using:

user.one('didCreate', function() {
this.transitionTo('showUser', user)

I end up with an url like this /users/null

although the server returns an id
If I transitionTo the user from the users list I get the url with the id!

console.log user shows the id too. but it gets lost on transitionTo, all the other user data is there and gets rendered correctly.

Contributor

bcardarella commented Jan 8, 2013

@wycats I have to agree with @raila as I am seeing the same. It appears the didCreate event is firing before the AJAX request cycle has completed. Instead I have attached observers onto the id attribute and waiting for a change on that.

Is this a bug?

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