Adding event triggers to add/remove views

[tragiclifestories]

Motivation: I've recently had to deal with a third party media API that accepts an id string as an input for the element in which to render the media, which means I need to be able to guarantee that the element is in the DOM before I pass it through. Custom events is the cleanest way to do this as things stand.

I'm putting it here in case anyone else thinks it would make their lives easier more than anything else: it's mostly useful I'd guess for irritating edge cases like mine.

[latentflip]

I've had to do this before too, so this might be worth adding, I think there's been some discussion about adding more events to views anyway for hooking into things like this. Would like to hear other's thoughts though? @AmpersandJS/core-team

Couple of suggestions on this PR:

1) I'm not sold on the event name, generally we haven't been using camelcased names for events, maybe child-view:added / child-view:removed or something similar?

2) In the tests I think we should check that the view is actually inserted/removed into the dom when the event handler's called. Looking at the code, I think it would be, but given dom timing etc, I think we should test it explicitly. Adding a unique id to the rendered child view, and checking whether it's visible from document.getElementById would be how I'd do it.

[tragiclifestories]

Good points, thanks. I didn't want to use basic 'add'/'remove' event names as these would be ambiguous (is it the whole view or a model view, etc), but I'm not sure of the proper style here, so I just copied the method names...

I will update the tests.

[tragiclifestories]

Any further comments on this? If a bigger piece of work adding more event triggers around the place is envisaged, I'm happy to have a look at it if somebody can point me to relevant discussions.

[wraithgar]

I can't think of a better name for the event, and this is a really simple addition that adds a lot of visibility into what's happening under the hood.

To my knowledge the discussion about eventing is happening around the render method itself, not when it's inserted or removed from the DOM. This is a good place to start for this kind of eventing.

:+1:

[wraithgar]

@latentflip you cool w/ this iteration?

[tragiclifestories]

Last night I dreamt that this was merged. (I am that much of a sad case.)

[latentflip]

I'd still like to tweak the tests to be sure that the timing is right, I don't think I explained what I meant very well previously. I'll find some time today to write it up. Thanks @tragiclifestories

[tragiclifestories]

Thanks @latentflip. My understanding of the code at present is that the event is guaranteed to fire after the update to the collection view itself, but of course there is no guarantee here that it is in the DOM as the collection view's el may or may not already be there. This is good enough for my use case (I always append the damn things manually) but may not be good enough for others.

[tragiclifestories]

somehow I get the feeling that this is not going to be merged. And I'm not using this library myself at the moment so closing.