backbone-paginator / backbone-pageable

DEPRECATED!!! backbone-pageable and backbone.paginator have merged. Please use backbone-paginator/backbone.paginator instead.
MIT License
343 stars 64 forks source link

deferred object for fetch on pageableCollection with state of fetch #84

Closed mastef closed 11 years ago

mastef commented 11 years ago

I used to check for the 'done' state of the active fetch by creating a deferred object returned by the fetch method ( see second answer on http://stackoverflow.com/questions/9220092/backbone-js-view-renders-before-model-fetch )

fetch: function(){
    this.fetching = Backbone.Collection.prototype.fetch.apply(this, arguments);
    return this.fetching;
},

to check later for

    collection.fetching.done( /** ... **/ );

But I had no luck yet with this method on the PageableCollection as it returns duplicate results. Am I maybe trying to reinvent the wheel here?

wyuenho commented 11 years ago

Why can't you listen to the reset event instead?

wyuenho commented 11 years ago

Do you still have problems working with Backbone.PageableCollection#fetch ?

mastef commented 11 years ago

Sorry I have been implementing reset first. It seems to be doing the job well. I'm wondering about any caveats of that implementation, but for now it's all good :)

wyuenho commented 11 years ago

Ok. I'm closing this for now.

mastef commented 11 years ago

I haven't found any documentation on using reset for this, and it seems reset is also fading out from use with backbone 1.0 using collection.set instead when updating the collection, and having to explicitly set {reset:true} for old behaviour ( http://backbonejs.org/#upgrading )

I found this solution though :

this.listenTo(this.collection, 'request', this.loading);
this.listenTo(this.collection, 'sync', this.render);

request is being fired on any request start, and sync when the request finished

wyuenho commented 11 years ago

Defaulting to reset:true != mean fading out the reset event. It's incredibly useful. You really really don't need to listen to request and sync. It's also wrong to listen to request and sync because they are also used by model.save and model.fetch and bubble up the parent collection.

All the get*page methods delegate to fetch. The reset event is implicit and documented by backbone.js.

mastef commented 11 years ago

Thanks, that's some good feedback. I didn't know about the model bubble ( that scenario doesn't apply in my case ), but it's good to know this caveat.

And reset seems to be a good scenario when you only want to display the 'loading' screen on a full collection fetch. So far I haven't found any down-sides yet. Maybe one downside would be in case the collection is being updated, but that could be handled with the change event if applicable.

mastef commented 11 years ago

Actually it seems the promise is available in the collection without having to override fetch. Such as :

var promise = myCollection.fetch();
promise.done(function() { removeLoadingSpinner(); })

Haven't tested yet if possible with pageable.