Skip to content
This repository has been archived by the owner on Mar 20, 2021. It is now read-only.

CollectionView not removing items from this.children on sort event #404

Closed
bengladwell opened this issue Dec 29, 2014 · 1 comment
Closed
Labels
Milestone

Comments

@bengladwell
Copy link

TLDR;
a sort event on a CollectionView's collection adds new ItemViews for each model to this.children and to the DOM, but does not remove the old ItemViews from this.children; the old ItemViews are retained indefinitely.

Here is a jsfiddle showing the problem.

I'm unsure if this is a bug or if I am doing something wrong.


sort event on collection -> CollectionView#onCollectionReset()
CollectionView#onCollectionReset() -> CollectionView#renderCollection()
CollectionView#renderCollection() -> CollectionView#appendItem() for each model

appendItem uses CollectionView#renderItem to produce a new view for the model.
https://github.com/walmartlabs/thorax/blob/v3.0.0-beta.5/src/collection.js#L226

itemView = this.renderItem.call(this, model, index);

If the new view has a cid, View#_addChild adds the new view to this.children, among other things.
https://github.com/walmartlabs/thorax/blob/v3.0.0-beta.5/src/collection.js#L230-233

if (itemView.cid) {
  this._addChild(itemView);
  itemView.ensureRendered();
}

Why not reuse the existing ItemViews in this.children in response to a sort event? Am I missing something?

In general, it looks like old ItemViews are not removed in CollectionView#onCollectionReset.

@kpdecker
Copy link
Contributor

kpdecker commented Jan 3, 2015

I don't see anything wrong with your code. Will take a look and see if there is anything telling.

@kpdecker kpdecker added the bug label Jan 3, 2015
@kpdecker kpdecker added this to the 3.0 milestone Jan 3, 2015
@kpdecker kpdecker self-assigned this Jan 7, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

No branches or pull requests

2 participants