Skip to content
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

_collection.set instead of _collection.reset on execFilter ? #8

Open
kinkykate92 opened this issue Oct 25, 2014 · 2 comments
Open

_collection.set instead of _collection.reset on execFilter ? #8

kinkykate92 opened this issue Oct 25, 2014 · 2 comments

Comments

@kinkykate92
Copy link

I'm using backbone-filtered-collection in combination with Marionette's CollectionView.
Originally, CollectionView renders one child view for every item in the collection. The views in my app are quite complex and some pages contain a few hundred of these in one CollectionView. Because Marionette does that synchronously, that blocks the browser for a few seconds (and that results in a bad user experience).

To fix that I came up with the following plan:

Instead of giving the original Backbone.Collection to the CollectionView I would give it a FilteredCollection to render, that has a filter including only the first 20 items (enough to fill the screen "above the fold"). Then, a few milliseconds after page load, add a filter that includes only the first 30 items, remove the first filter, so CollectionView would render additional 10 child views, and so on, somewhat like this:

var show_first = 20;
var increase_collection = function() {
  if (show_first > original_collection.length) {
    return;
  }
  show_first += 10;
  filtered_collection.filterBy("first_" + show_first, function(item) {
    return (item.collection.indexOf(item)) < show_first;
  });
  filtered_collection.removeFilter("first_" + (show_first - 10));
  setTimeout(increase_collection, 50);
};

increase_collection();

However ... in execFilter the underlying _collection is resetted every time a filter is applied:

this._collection.reset(filtered);

[https://github.com/jmorrell/backbone-filtered-collection/blob/880672e7edbc3eb2151f569fe009d62d03d989c4/backbone-filtered-collection.js#L87]

This causes the CollectionView to (following the example given above) remove 20 child views, then add 30 new childviews. Quite the opposite to what I want to achieve.

If I change my local copy so that the line reads

this._collection.set(filtered);

instead, everything is smooth, the proper 'add' events are fired and only additional child views are rendered.

Is there a particular reason to use reset here instead of set ?

@kinkykate92
Copy link
Author

Any ideas on this?

@jmorrell
Copy link
Owner

jmorrell commented Nov 4, 2014

Sorry for not responding earlier. You caught me on vacation, and I'm still catching up :)

Seems like this is a common request: jmorrell/backbone.obscura#27

Explanation: I originally built this with more of a focus on pagination. IIRC the case where the whole collection was being blown away and replaced was more common (in filtering you might run into this if you have lots of items), and I was running into the case where I'd have n item remove events followed by n item add events, causing lots of DOM manipulations, where blowing things away and re-rendering the whole thing was more performant.

I'm definitely not against switching this to set instead of reset, and I want to make sure this works well with Marionette's CollectionView. If my original thinking was off, or if things have changed since then I'd love to move it over. It could also perhaps be conditional depending on how many items have changed, or passed in as a preference.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants