Use jQuery namespaces for all binded events

[mitar]

Annotator should really use jQuery namespaces for all binded events, so that it is easy to possible to unbind events as well without interfering with other things.

[tilgovi]

+1

[mitar]

The rationale in our case is that we have a single-page application, so we never reload the page. So we have to be able to clean everything between moving through different documents.

[tilgovi]

This would be an easy update to the Delegator (class.coffee). Changes to addEvents (or maybe _addEvents) and the corresponding remove.

[mitar]

So there should be a way to unload annotator completely.

[tilgovi]

Agreed. There is, in recent tree. The destroy() method should propagate event unbinding to all the plugins. It's not guaranteed to work, but it's an attempt. The Delegator class is tracking the closures itself right now, but a namespace might be better if jQuery has an easy way to unbind all events from a given namespace.

[mitar]

but a namespace might be better if jQuery has an easy way to unbind all events from a given namespace.

Exactly this was motivation for this ticket. :-) .off('.annotator') unbinds all on the element. Hm, probably then $('*').off('.annotator') unbinds everything?

[mitar]

Is there any cleanup code already in place? Or should I improvise for now?

[tilgovi]

Yes, there is. See my previous comments. I mentioned the important points.

You might also want to audit some plugins for events set on document or window.

[tilgovi]

@mitar was that clear?

[mitar]

I definitely see bindings with jQuery oustide events of Delegator. I had in mind those. For example, see _setupDocumentEvents.

The issue is also that there is no consistency in how code binds events. So we have Delegator's events. Then I saw addEventListener and jQuery bind.

Why not just use jQuery on everywhere, with namespaces, and for custom events use jQuery as well.

[tilgovi]

_setupDocumentEvents is the exception, not the rule. Lots of things in Annotator use the Delegator events feature.

To answer question, I've already voiced support for doing exactly what you suggest. I'm asking if my answer was clear enough such that you feel you can now begin to implement that, and if you have interest in doing so.

[mitar]

So exceptions should stay exceptions? Anyway, in our extended Annotator class we are already doing this.

[tilgovi]

I made no claim about whether they should stay the way they are. I suspect not. I said already I like the namespace idea. Would you like to work on this?

[mitar]

I think I already did it. :-) So I think it would be enough just to define a destroy method on Annotator which makes sure everything is cleared up? I would leave events in Delegator as they are, but add a destroy there as well. And I would unify to use .on everywhere. And namespaces.

Where should this code be? Which branch? Against hypothesis/annotator?

[csillag]

Where should this code be? Which branch? Against |hypothesis/annotator|?

We are working on several different branches in hypothesis/annotator.
Hopefully, we will consolidate all work on a single branch soon, and
then start merging it into okfn/annotator.

I wouldn't recommend adding any further divergent branches, at this point.

(That being said, I fully support the idea of consolidating the mixed
browser event / jquery event / Delegator binding usage.)

[tilgovi]

Definitely on okfn/master!

[tilgovi]

I mean okfn/annotator@master

[tilgovi]

At this point, I believe master accomplishes this.