indexiatech / ember-forms

Smart, Intuitive forms for Ember.js styled with Bootstrap, Multi layouts and Validation support.
http://indexiatech.github.io/ember-forms
Apache License 2.0
218 stars 45 forks source link

Validating Select elements does not work correctly #75

Open jackmatt2 opened 9 years ago

jackmatt2 commented 9 years ago
validations : {
    status : {
          presence : true
    }
}

does not work on a select element. you need to use.

validations : {
    'status.id' : {
          presence : true
    }
}

but then the validaition message does not appear because the path is now pointing to the id instead of the status property. The submit button will however remain disabled as expected using the second syntax as it correctly determines that the model is invalid.

This appears to be due to the fact that we need to bind to the .content property when an item is selected.

asaf commented 9 years ago

@jackmatt2 What Ember/HB do you use? for me a select validation works just fine on the property level ( no need property.id)

jackmatt2 commented 9 years ago

I am using ember-cli@0.1.4 which includes:

 ember 1.8.1
 handlebars 1.3.0

I should have mentioned that I am binding the selection to a DS.Model using my pull request. I guess that may be the issue here. It would be good if the ember-forms select element supported binding to a model instead of a value.

Ember.Select supports two syntaxes.

The following is currently supported by ember-forms.

{{view "select"
       content=programmers
       optionValuePath="content.id"
       optionLabelPath="content.firstName"
       value=currentProgrammer.id}}

The following is not supported:

{{view "select"
       content=programmers
       optionValuePath="content.id"
       optionLabelPath="content.firstName"
       selection=selectedPerson}}

I will almost always use the second syntax where I bind the selection to a DS.Model.

If I am incorrect here and there is actually a way to bind directly to the selected model please enlighten me.

asaf commented 9 years ago

Fair enoug

Lets support this on the upcomming release

jackmatt2 commented 9 years ago

@asaf OK cool. Can you give me any idea of your release cycle? The reason being I will try to put off working with the library until support for this is added.

jackmatt2 commented 9 years ago

@asaf I actually think my pull request should change a little.

I think ember forms should follow the Ember.Select convention and use value and selection as the as opposed to property which is currently being used. property can still be an alias for value thus not breaking existing code. This would also help new users get going more quickly as the API is exactly the same.

property : Ember.computed.alias('value')

The reason is that currently ember-data models require '.content' as part of the property for an async relation. I hard coded this into the code but there are cases where you are using a selection that is not binding to an ember-data model.

I propose that ember-forms does what Ember.Select should do and actually correctly resolve .content property. see here

bindPath = selection;
if(selection instanceof  DS.PromiseObject) { //this probably will not work but you get the idea
  bindPath = selection + '.content';
}

The above would allow the user to switch between a Ember Data model with something else without modifying their template code (This is an exact issue I am having with some generic code I have created whereby sometimes it is an ember-data model an other times it is a regular Ember.Object)

So if you had a person property you could bind to it in the following ways:

//same thing
property=persion   <-- for backward compatibility
value=person`

//to bind to the backing model
selection=person     <-- this will automatically add the .content for a DS.PromiseObject