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

Return the newly added view from addItemView in CollectionView. #851

Merged
merged 1 commit into from
Jan 13, 2014

Conversation

ericmatthys
Copy link
Contributor

In situations when the CollectionView is extended heavily, it makes life easier to be able to get a reference to the view created when calling addItemView.

@samccone
Copy link
Member

@ericmatthys can you provide a use case for this. I am not sure I fully understand what this adds.
You can already hook into the view added via onAfterItemAdded on the collectionView

@ericmatthys
Copy link
Contributor Author

I think it's good practice to return the newly created item if the function's primary purpose is to create that item.

Here is my use case though (which I'll admit is unique):

I have a CollectionView that is not mapped 1:1 to a collection. The collection may have a total size of 100 models, but only 20 models are initially fetched so the length of the collection initially is 20. The CollectionView has the capability of rendering all 100 list items (80 of which don't have data initially, but can be populated with data as new pages are requested).

This CollectionView must also be sorted and must keep its items in order if new items are added to the beginning or middle of the collection. I need to be able to accurately get a reference to a list item at a specific index. Babysitter uses an index for child views, but this index is not accurate if you are trying to add views to the middle or beginning of the CollectionView. Therefore, I am having to track the correct indices of each item myself.

All of this requires me to 1) call addItemView myself to create list items and 2) add the item view to my own sorted array of items at a particular index. Using onAfterItemAdded doesn't seem like a good solution to me since I lose the index, which is very important, and I can't get a reliable index from babysitter.

itemView = this.addItemView(item, this.getItemView(item), i);

// Add the newly created view to our sorted array at the index it was
// created.
this.sortedChildViews.splice(i, 0, itemView);

@samccone
Copy link
Member

Ok, thanks for the rational. Can you add a unit test for this PR, and then we can get this merged.

Thanks

@ericmatthys
Copy link
Contributor Author

I added the unit test. Let me know if there's anything else I need to do.

@samccone
Copy link
Member

@ericmatthys Can squash c450010 in 5267750

thanks!

@ericmatthys
Copy link
Contributor Author

Squashed and moved those lines to inside the beforeEach.

@samccone
Copy link
Member

👍

samccone added a commit that referenced this pull request Jan 13, 2014
Return the newly added view from addItemView in CollectionView.
@samccone samccone merged commit f600037 into marionettejs:master Jan 13, 2014
@samccone
Copy link
Member

@ericmatthys just wondering where in PLEX are you guys using marionette

@ericmatthys
Copy link
Contributor Author

plex.tv/web

@samccone
Copy link
Member

Awesome!

You should add the plex logo to https://github.com/marionettejs/www

@ericmatthys
Copy link
Contributor Author

Sure, here you go: marionettejs/www#21

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

Successfully merging this pull request may close these issues.

None yet

2 participants