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: add into sorted position" + index is passed in 'add' event #673

Closed
wants to merge 1 commit into from
Closed

Conversation

lawrencepit
Copy link

When a Collection has a +comparator+ defined, then when adding an object the +add+ event handler needs to know at which position the object was inserted into. This patch passes the +index+ to the +add+ event handler.

@wookiehangover
Copy link
Collaborator

The model's index is readily accessible in an add event handler via indexOf:

bind('add', function( model, prop ){
    console.log( model.collection.indexOf(model) )
});

@snoe
Copy link

snoe commented Oct 29, 2011

+1 for the patch

The index is commonly needed so that you can insert the new info in the correct place

$("list li").get(index).after(view.el)

In a large sorted collection model.collection.indexOf(model) could be expensive. Backbone is already calculating this index and then throwing it away.

This is good for the same reason that Array.forEach passes the item and its index to the handler.

@wookiehangover
Copy link
Collaborator

this is probably a good candidate to consider along side a fix for #689

iros pushed a commit to iros/backbone that referenced this pull request Oct 29, 2011
… gets passed when the add/remove callbacks get triggered on a collection.
iros pushed a commit to iros/backbone that referenced this pull request Oct 29, 2011
… gets passed when the add/remove callbacks get triggered on a collection.
tbranyen added a commit that referenced this pull request Oct 29, 2011
#673 - Adding index as a property on the options object that gets passed
@jashkenas
Copy link
Owner

Backbone is no longer calculating the index on a per-model basis. For the time being, if you need it, you can look it up.

@jashkenas jashkenas closed this Jan 13, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants