Skip to content

Commit

Permalink
Updated build with latest backbone-collection-proxy.
Browse files Browse the repository at this point in the history
  • Loading branch information
@jmorrell
jmorrell committed Oct 30, 2013
1 parent 101fe17 commit 86ef135
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 12 deletions.
41 changes: 30 additions & 11 deletions backbone-paginated-collection.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,9 @@ _.extend(Paginated.prototype, methods, Backbone.Events);
module.exports = Paginated;


},{"backbone":false,"backbone-collection-proxy":2,"underscore":false}],2:[function(require,module,exports){
},{"backbone":false,"backbone-collection-proxy":3,"underscore":false}],"backbone-paginated-collection":[function(require,module,exports){
module.exports=require('PxKHrf');
},{}],3:[function(require,module,exports){

var _ = require('underscore');
var Backbone = require('backbone');
Expand All @@ -237,23 +239,41 @@ var blacklistedMethods = [
"listenTo", "listenToOnce", "bind", "trigger", "once", "stopListening"
];

var eventWhiteList = [
'add', 'remove', 'reset', 'sort', 'destroy'

This comment has been minimized.

Sign in to view

Copy link
@vincemtnz

vincemtnz Oct 31, 2013

does this mean the paginated collection will be updated on 'sort'? (i see sort is still in the blacklistedMethods list).

This comment has been minimized.

Sign in to view

Copy link
@jmorrell

jmorrell Oct 31, 2013

Author Owner

The paginated collection should be updated on the parent collection's sort, but it looks like it's currently not. I'll file that as a bug and get a fix out today. Thanks for pointing it out!

Adding the .sort() method is a lot more complicated I'm afraid. The current design only allows forwarding of read-only methods, while .sort() modifies the original collection. I'm going to be exploring adding these other methods (append, add, remove) as they would be really nice to have, but they bring up a whole host of edge cases.

  • If I add a model at a specific location on the third page of a paginated collection, where should that model go in the original collection?
  • Same for push, pop, shift, unshift
  • Should I allow a comparator to be set on the proxy? If it differs from the original collection, which takes precedence?

If I can find reasonable solutions to these issues, I'll remove those functions from the black list :)

This comment has been minimized.

Sign in to view

Copy link
@jmorrell

jmorrell Nov 1, 2013

Author Owner

With the last update the paginated collection should update when the original collection is sorted :)

This comment has been minimized.

Sign in to view

Copy link
@vincemtnz

vincemtnz Nov 1, 2013

Awesome! I also ended up adding a listenTo 'sort' in my local copy of the lib, but I wasn't sure if the events would be piped and didn't get around to writing any tests. Good job on this and Obscura :)

I think that if you allow comparator to be set on the proxy, it should be an override for the default behaviour (which could be to copy the comparator function from the original collection). Backbone.Obscura's sort is kind of buggy when you combine sorted and paginated, in my case, in part because of this. The PaginatedCollection would still need to listen to the sort method since when a model is updated in the original collection, you still have to manually trigger sort (on the original collection), and that's where you'd want to recalculate.

Sorry, this should probably be discussed inside a issue on enhancement :-)
];

function proxyCollection(from, target) {

function updateLength() {
target.length = from.length;
}

function pipeEvents() {
function pipeEvents(eventName) {
var args = _.toArray(arguments);
var isChangeEvent = eventName === 'change' ||
eventName.slice(0, 7) === 'change:';

// replace any references to `from` with `this`
for (var i = 1; i < args.length; i++) {
if (args[i] && args[i].length && args[i].length === from.length) {
args[i] = this;
}
// In the case of a `reset` event, the Collection.models reference
// is updated to a new array, so we need to update our reference.
if (eventName === 'reset') {
target.models = from.models;
}

this.trigger.apply(this, args);
if (_.contains(eventWhiteList, eventName)) {
if (_.contains(['add', 'remove', 'destory'], eventName)) {
args[2] = target;
} else if (_.contains(['reset', 'sort'], eventName)) {
args[1] = target;
}
target.trigger.apply(this, args);
} else if (isChangeEvent) {
// In some cases I was seeing change events fired after the model
// had already been removed from the collection.
if (target.contains(args[1])) {
target.trigger.apply(this, args);
}
}
}

var methods = {};
Expand All @@ -270,6 +290,7 @@ function proxyCollection(from, target) {

target.listenTo(from, 'all', updateLength);
target.listenTo(from, 'all', pipeEvents);
target.models = from.models;

updateLength();
return target;
Expand All @@ -278,9 +299,7 @@ function proxyCollection(from, target) {
module.exports = proxyCollection;


},{"backbone":false,"underscore":false}],"backbone-paginated-collection":[function(require,module,exports){
module.exports=require('PxKHrf');
},{}]},{},[])
},{"backbone":false,"underscore":false}]},{},[])
;
return require('backbone-paginated-collection');

Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,6 @@
"dependencies": {
"underscore": "~1.5.1",
"backbone": "~1.0.0",
"backbone-collection-proxy": "~0.1.0"
"backbone-collection-proxy": "~0.2.0"
}
}

0 comments on commit 86ef135

Please sign in to comment.