an attempt at differentiating event handlers via an optional namespace #106

Closed
wants to merge 1 commit into
from

Projects

None yet

3 participants

@chrisdeely

I've made an attempt at solving the problem raised in issue #90. This approach allows the user to specify an optional 'namespace' parameter to the on & off methods which allows for multiple independent handlers to be registered for each event.

I wanted to get your feedback before pursuing this approach further. There is at least one immediate issue in that the 'load' event is fired without a 'currentElement' being specified but I didn't want to spend time addressing that if you don't agree with the overall direction.

Let me know your thoughts.

@JamesMGreene
Member

Thank you for the PR! I plan to solve #90 legitimately without namespacing, though I think it wouldn't hurt to offer event handler namespacing as an option (a la jQuery's event system).

However, the behavior of jQuery's event namespacing is different from that of this PR. In jQuery, the event namespacing serves as an optional convenience grouping that can be used for easy handler management filtering but it is not a hard grouping , i.e. handlers added with a namespace can be removed and triggered without referencing that namespace again. On the other hand, this PR utilizes a hard grouping and therefore requires that the namespace be used to remove the handlers if it was used to add them. The PR's current triggering (dispatching) also assumes that the namespace is always equal to the element's ID, that the namespace is only applied to a single element, and that the element has an ID... but without these assumptions being true, the handlers will never be triggered.

If you remove that hard grouping behavior, I would be open to merging the event namespacing concept if @jonrohan is also on-board with it. Again, though, I will insist we fix #90 without requiring workarounds like this.

@joelzimmer

It'd be great to get this fixed. There's been a couple potential fixes pushed here, and the fact that it worked in the previous version is a bit of a let down.

@JamesMGreene JamesMGreene was assigned May 7, 2013
@JamesMGreene
Member

We really do appreciate the effort but we're going to pass on this PR for now. Thanks anyway!

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