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

Doesn't trigger non-change item events like a collection does. #25

Closed
latentflip opened this Issue Nov 26, 2014 · 1 comment

Comments

Projects
None yet
3 participants
@latentflip
Contributor

latentflip commented Nov 26, 2014

In a collection I can fire a custom event on an item in the colleciton and it will fire on the collection, this doesn't happen on a subcollection.

myCollection = new MyCollection([ myModel ]);
myCollection.on('all', console.log.bind(console));

myModel.trigger('an-event', "foo") //console logs 'an-event', 'foo'

//vs...

mySubCollection = new SubCollection(myCollection);
mySubCollection.on('all', console.log.bind(console));

myModel.trigger('another-event', 'bar') //doesn't log 
@aaronmccall

This comment has been minimized.

Show comment
Hide comment
@aaronmccall

aaronmccall Dec 12, 2014

The issue here appears to be that the subcollection has no way to know whether the event originated from a model that is in fact in its filtered version of the collection.

We could go the route of #26, or we could clarify exactly what events we want to exclude from bubbling. We already listen for every event via the collection, so it seems we should use that if we can instead of adding and removing handlers to/from every model.

Below is the line that currently decides what to bubble.

// conditions under which we should proxy the events
if ((_.contains(['sync', 'invalid', 'destroy']) || eventName.indexOf('change') !== -1) && this.contains(model)) {
    this.trigger.apply(this, arguments);
}

Can we change the eventName checks to a blacklist and trigger if no match?

aaronmccall commented Dec 12, 2014

The issue here appears to be that the subcollection has no way to know whether the event originated from a model that is in fact in its filtered version of the collection.

We could go the route of #26, or we could clarify exactly what events we want to exclude from bubbling. We already listen for every event via the collection, so it seems we should use that if we can instead of adding and removing handlers to/from every model.

Below is the line that currently decides what to bubble.

// conditions under which we should proxy the events
if ((_.contains(['sync', 'invalid', 'destroy']) || eventName.indexOf('change') !== -1) && this.contains(model)) {
    this.trigger.apply(this, arguments);
}

Can we change the eventName checks to a blacklist and trigger if no match?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment