backbone-forms
backbone-forms copied to clipboard
powmedia
Use collections for lists of nested models.
Also prevents modal boxes from popping up on form load. Should have been a separate commit, sorry.
Do you have any instructions for running and writing tests? I would have done otherwise. No idea what I'm doing with this "sinon" though.
Just run test/index.html in your browser and add qunit tests to the appropriate files (e.g., test/editors/list.js)
It's just QUnit tests - sinon is a little add-on to help with some things. Reading through some of the existing ones should be helpful.
http://qunitjs.com/ http://sinonjs.org/
Thanks for adding the tests @mitchellrj - just looking into merging but wanted to confirm what the use case is?
I can see why you would pass an instance of a collection in to populate the list (a little like how the Select editor works) but what are the advantages of passing in a collection definition?
In my particular case, I'm using backbone-relational and have a custom collection type to help integrate with the API on the server side. One might have different collection types depending on the model contained, particularly if using Backbone.Collection.model, or even in particular contexts.
Refactored implementation added so that we don't depend on editor type names for behaviour.
@mitchellrj I tried a merge earlier today - there was a missing opening curly brace (have added a line comment) which was breaking the script. That wasn't too much of a problem, I fixed that; the main thing was that List.NestedModel was no longer returning a object literal value, so when you do Form.getValue() or Form.commit() it's not receiving the full form value.
There would need to be a way of keeping the original value being returned as before.
Refactoring the code into separate List.Object and List.NestedModel editors was good, and I've done this. I also added a whole bunch of tests for List.Modal to master so if you continue work on this best to rebase off master and use those tests, maybe adding where appropriate.
Thanks for that. I'm not sure I follow what you mean when you say
List.NestedModelwas no longer returning a object literal value, so when you doForm.getValue()orForm.commit()it's not receiving the full form value.
I think I should have made my refactoring clearer, in that the classes are distinguished in the following ways:
List.Modal: an abstract base class ofList.ObjectandList.NestedModel.List.Object: an editor which accepts and returns Javascript hashes and requires a schema to be provided.List.NestedModel: an editor which accepts and returns Backbone models and requires a model to be provided.
The updated tests I've just pushed should help clarify the situation.
What I mean is that every editor needs to return a simple object from the getValue() method, because when you do form.commit() or form.getValue() it's just going through each editor and combining them.
In your pull request, List.NestedModel#getValue() returns a model, and if the List#getValue() returns a collection when the itemType is NestedModel.
This breaks the way form.getValue() works - you get an empty object ({}) for the list field, instead of an array of objects.
So the way it's intended to work and indeed does work in my use-case at least is as follows:
I have a model Parent which, using Backbone-relational has an attribute as part of its schema, children which contains a Backbone.Collection of child Backbone.RelationalModel objects.
Feeding this model to a form using a Backbone.Form.List with item editor Backbone.Form.List.NestedModel sets the form values appropriately, and indeed getValue returns and updates the model appropriately, too.
When it comes to saving the model as JSON, the default handlers for Backbone.Collection.toJSON and Backbone.Model.toJSON do all the hard work and give you back the right thing anyway.
I think the case you're talking about requiring an array of Javascript hashes to be input and returned should use the Backbone.Form.List.Object editor. For me, it makes sense that setValue and getValue should always be of the same type. Inputting a Backbone.Model and getting back a hash isn't desirable IMO.
Sounds like @mitchellrj is describing what I was in https://github.com/powmedia/backbone-forms/issues/81#issuecomment-6665415
Ok yeah that all makes sense. I suppose that internally the form should check if the editor is for a nested model or a list of them (collection) and handle them differently.
But I think this is a bigger job than just this pull request as we'll need to work on some of the form internals too.
The form should probably use each child model's save() method rather pulling out the values and sending them through as part of the main model.
Is everyone here using backbone-relational? I tend to just put nested models and collections on the parent directly so will need to look into what backbone-relational actually does. Seems odd that collections contain instances of backbone-relational models as mitchellr points out.
On 11 Ma 2013, at 17:14, Phil Freo [email protected] wrote:
Sounds like @mitchellrj is describing what I was in #81
— Reply to this email directly or view it on GitHub.
Is everyone here using backbone-relational? I tend to just put nested models and collections on the parent directly so will need to look into what backbone-relational actually does.
I use backbone-relational a lot, though other times I just setup the relations manually. In either case a model attribute is an instance of a Backbone.Model.
Seems odd that collections contain instances of backbone-relational models as mitchellr points out.
Why is this odd?
The form should probably use each child model's save() method rather pulling out the values and sending them through as part of the main model.
yes this makes sense (in form.commit())
Seems odd that collections contain instances of backbone-relational models as mitchellr points out. Why is this odd?
- Only because I'd expect an instance of your app model to be returned from the collection. How does that work - do the backbone-relational models wrap them?
yes, like this:
var Foo = Backbone.RelationalModel.extend(),
foo = new Foo();
ok(foo instanceof Foo);
ok(foo instanceof Backbone.RelationalModel);
ok(foo instanceof Backbone.Model);
@mitchellrj All tests are passing for me now but have you tried editing an existing item in a List.NestedModel? I get Object has no method 'get'.
This seems to be because the editors.Base is looking up this.model (for the parent model) but this has been overwritten with the data from the List.NestedModel instance.
I'm also thinking that when this is all sorted it might make sense to rename the List.NestedModel editor to something like List.RelationalModel to make it clearer that it depends on backbone.relational-model.
I'm working on a different editor for working with collections directly without backbone.relational-model though it will have to be separate from the List editor.
@powmedia It doesn't actually depend on backbone-relational, it should work with any model where nested models are held in collections. I'm working on implementing some changes to how commit works, as suggested and then I'll add some more tests.
I'm not sure how I'm going to go about it yet. I was going to make Form.commit call the commit methods on each field, which in turn call the commit methods of their respective editors, and for the List editor, make it call the commit for each modal form and so on.
The problem with this approach is that the change event is triggered for each individual attribute which has changed, rather than just once per model. I don't want to change the behaviour or signature of commit, so another approach might be to add a new method called prepareCommit or something similar which returns the attributes & values to change for each model and in the case of the list editor, calls the commit method of each nested form.
I don't particularly like either of these solutions. I need to think about it some more.
I think it's because the List editor isn't right for this use case.
I've started some work on a Collection editor which adds, edits and removes items from a collection atomically. I.e. when you edit an item in a collection, it modifies that model directly; when you remove it, it removes it from the collection etc. In a way this separates the collection from the parent model but I think it makes sense to use the collection to it's full potential like this.
That does sound better.. and so when the modal is closed or in some other way submitted, the data is then committed to those individual items?
Are you working on this completely separately from this pull's branch, or are you basing some of it on what exists here?
I've started some work on a Collection editor which adds, edits and removes items from a collection atomically. I.e. when you edit an item in a collection, it modifies that model directly; when you remove it, it removes it from the collection etc. In a way this separates the collection from the parent model but I think it makes sense to use the collection to it's full potential like this.
I do something similar to this already and it would be great to see it standardized. Just need to make sure it works properly in a setting where you might want to "cancel" your changes made to the list.
I am using backbone relational as well and would find the items mitchellrj and philfreo are introducing to be useful. If not this, is there a preferred way to handle a sub collection of nested models?
