Should we extend ftdomdelegate to provide a better API for inter component communication

[wheresrhys]

I'm using it in my refactor of o-modal. One pattern we're going to see repeated is a module needing to listen to events on body (for inter module communication) and on its owned DOM (for UI interactions). Listening to events on body doesn't really need the delegate pattern, but the on/off API & listener management is useful, so it makes sense to define a delegate on body. And for the module's owned DOM adding a delegate there is a no brainer. So modules will often end up having to define two delegates.

Would it make sense to fork ftdomdelegate and build in this requirement. I suggest an API exactly like the existing one but where document.body can be passed instead of a selector as the second parameter (or, more generally, any element can be passed as the second parameter - a less obvious use case, but probably a doddle once we've implemented for document.body)

@matthew-andrews - thoughts? Am I missing something about ftdomdelegate again?

[matthew-andrews]

listen to events on body (for inter module communication)

I'd be really interested in seeing what events those are in a bit more detail because we use ftdomdelegate heavily for very similar components and applications and haven't needed this requirement before - my gut instinct says that it may be that some of the actions that are being modelled as events here may be more appropriately implemented as API.

[wheresrhys]

Two so far:

1.oViewport.resize, oViewport.scroll - events fired by throttled functions listening to native window events in order to implement the throttle/debounced listeners in one place. These could be implemented with an API, but then we'd need to be careful with garbage collection

2. oLayers.open - any layer (e.g. a modal dialog) can fire this event in order to notify other modals that it's opening and trigger their closure. Definitely needs the sort of loose coupling that events gives us

[matthew-andrews]

Hi Rhys,

I've assumed you need to listen to resize events in order to recalculate dimensions of a component but I'm not sure about scroll - what's the use case for a component needing to listen to the window scroll event?

1. Resize is tricky - especially if you're running javascript that affects the DOM on that event because each module needs to wait for its parent to settle down before it can resize itself - otherwise it'll make invalid measurements. Our components (eg FT Scroller) tend to have an updateOnWindowResize (or equivalent) option which default to true but in a large and complex app that's normally switched off and it's the responsibility of its immediate parent component (or the product) to call updateDimensions (or equivalent) on it. Avoiding running javascript on window resize is very much preferable :-).
2. We've had a lot of problems with this sort of thing before... With the Economist app - maybe @georgecrawford can comment more than me - it's rarely that simple. We found it is a pretty unsatisfactory user experience to just close an overlay another opens - eg. if the user is in the middle of interacting with one overlay (signing in / searching, in the case of the ft app) you don't want to interrupt that to show them something potentially unhelpful (eg. an unrelated error). Also we very, very often have use cases for overlays stacked on top of other overlays (usually a smaller one on top of a bigger one). You wouldn't want the latter to close down the former when it could just sit there harmlessly in the background*.

* I think the pattern we settled for was to have a queue of overlays - where the new one would be added to that and showed when the currently on screen was dismissed - and have one queue for small overlays and one for big ones. Personally I would leave this for the product to worry about...

[wheresrhys]

Leaving aside specific use cases (you make some good points, but this suggestion isn't about resize/scroll or overlays exclusively), custom events on body is how origami components will be doing decoupled communication, so the scenario outlined in the original post is one that will be expected to be used quite widely. So unless the spec for inter component communication gets changed, 2 delegates per module will often be needed, so it's worth considering ways to avoid this.

[matthew-andrews]

Ok I see. In that case I would recommend not forking ftdomdelegate and instead wrapping it (pull in as a dependency into a new project - o-ftdomdelegate or something) so it provides the clean API you want but internally instantiates two dom dels.

[wheresrhys]

So here's a rough implementation https://github.com/Financial-Times/o-modal/blob/event-driven-instance-management/src/js/events.js

The API is identical other than that a second parameter can be passed to the constructor to indicate a body delegate should also be added, document.body can be passed as the selector, and off can be passed true to turn events on body off as well as the local ones.

[matthew-andrews]

That looks good to me!

[triblondon]

I'm struggling to follow the code. Could do with some comments in the code to document why it is being done like this. In principle, I'm still unclear on why you need two delegates if you're binding one to the body. Everything will bubble to the body, so why do you need another one?

[wheresrhys]

Because if you have two instances of a module they will both use the same selectors for delegate but will only want to listen to e.g. click events on their owned dom, so need a delegate tied to their owned Dom element. Could be done by passing the instance in the event detail, but delegating on a subsection of the DOM is cleaner and more efficient.

Will write comments before thursday

[wheresrhys]

commented now https://github.com/Financial-Times/o-modal/blob/event-driven-instance-management/src/js/events.js

[triblondon]

OK, so let me check I understand this - you want to bind to some events emitted by other components at the body level, eg oViewport.resize, and you want to bind to your own events only if emitted within your owned DOM, because otherwise you'll also hear events from other instances of the same component.

But I still don't understand why you can't do this with one delegate on the body, you'd just need to specify a selector in your on() call that included the root of your owned DOM in the case of your own events. Is that right, and you simply don't want to do that, or is that not possible for some reason?

Is it because you may not have a selector string that targets your owned DOM, but you do have an element reference? If so, could we maybe add support in ftdomdelegate for using parent elements rather than string selectors?

delegate.on('oMyComponent.myEvent', ownedDomRootEl, function() { ... });

[matthew-andrews]

Ah - with the last point - that might be trickier to implement than what @wheresrhys has done with the double delegate.

[wheresrhys]

Also your example would typically need to combine the owned dom root and a selector when listening for native events

delegate.on('click', ownedDomRootEl, '.click-target', function() { ... });

[triblondon]

OK, and it does seem a bit pointless to set the root of the delegate to an element only to say you only want events inside another element. So OK, I'm with it now - two delegates seems the right solution. One per root.

[matthew-andrews]

If there are concerns about memory usage / causing GC - i've been thinking about adding a pool of destroyed dom delegates that get returned here: https://github.com/ftlabs/ftdomdelegate/blob/master/lib/index.js#L16 instead of always creating a new one.

(Will profile it before merging it to prove it actually makes a difference first :smile:)

[matthew-andrews]

something like this, maybe? https://github.com/ftlabs/ftdomdelegate/compare/gc?expand=1

[triblondon]

So there are no changes to make here? Can we close?

[matthew-andrews]

From my perspective we can close this, @wheresrhys ?

[wheresrhys]

There is a wider issue of origami's use of ftdomdelegate though. On https://github.com/ftlabs/ftdomdelegate/issues/43 we've settled on putting in a couple of small fixes that will tackle the majority of the ie8 bugs, and to document its remaining shortcomings. But if ie8 is to be a primary support browser should an event library that's buggy in ie8 be on our A-list? Also there's https://github.com/ftlabs/ftdomdelegate/issues/44 which needs investigation as it affects all ie versions.

Finally, the code sample I posted a few comments ago is at the moment untested and doesn't exist as its own module. My original point was that we could avoid 2 delegates per module by turning that code into a module. Are we agreed on that in principal? in which case there are changes needed - create the module and update the A-list.

[triblondon]

I am against having a separate module that wraps domdelegate for the purpose of binding delegates at two points in the DOM. Just do two delegates inside of the module that needs it. The other two issues are being tracked as domdelegate bugs, and we're separately considering dropping support for IE8, so I don't think we need this thread anymore.