-
Notifications
You must be signed in to change notification settings - Fork 176
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
Added support for return select options with promises. #231
base: master
Are you sure you want to change the base?
Conversation
Checks for .then function as all promises including jQuery deffered are 'thenable'
Just to make sure I understand, the api should look something like this: var View = Backbone.View.extend({
initialize: function(options) {
this.someCollection = new SomeCollection;
},
bindings: {
selectOptions: {
collection: function() { return this.someCollection.fetch(); }
}
}
}); or collection: function() { return $.getJSON('endpointToAnArray.json'); } right? |
Yes, right... in theory any function that returns a promise-like ('thenable') object (such as jQuery's deferred) should work.. doesn't matter if the wanted list/collection already existed or will exist in the future... |
hmm the bug is actually not a bug, because |
|
I'm not sure if stickit should 'fix' that or let it up to the user... |
Yeah Deferreds are ugly beasts. Can you layout what you envision the API would look like? |
I'm not sure if i understand your question. Which API should i layout? |
I don't think this needs to be changed on Backbone because it would break a major feature that fetch just returns the ajax deferred and its original data. What if the |
It already checks for getting a collection.. that's supported. the 'issue' is that |
Wrapping in a generic Collection is backwards for the reasons you mentioned, but I think maybe allowing the user to pass back the collection directly should work: collection: function() { return this.collection.fetch().then(function() { return this.collection; }); } Still uglier than just passing a collection instance, but may be required here. |
var $group = Backbone.$('<optgroup/>').attr('label', label); | ||
addSelectOptions(optList[label], $group, val); | ||
$el.append($group); | ||
var doAddList = function(listToAdd) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about just addList
for consistency?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or createSelectOptions
like from #203.
This should not be needed using normal promises which resolve the collection itself, only for funky implementations like jQuery.
imho, pretty clean, but the beauty is that the
the call there was to set the context, i noticed in the callbacks the context is often |
Not sure I follow. I think I may've gotten confused about passing the If you want to merge in the latest changes from master I'll give this a spin. Thanks for the pull. |
I don't think this is doing that. |
Conflicts: backbone.stickit.js
Hmm travis timed out... Maybe you could restart the build? |
You got it. |
I think you can add support for 'doneables' as well like 'thenables', as promises like 'q' are implemented 'done' instead of 'then'. |
As far as i can tell 'q' also offers a |
All promise implementations should expose a |
As previously suggested by @sandeepgs3 in #230
This version checks for results being 'thenable' this should, in my understanding, be the common factor in Promises/A, jQuery deferred, and polyfills