Skip to content
This repository

Events interface #4

Open
seanmonstar opened this Issue October 21, 2011 · 6 comments

3 participants

Sean McArthur Piotr Zalewa Daniel Buchner
Sean McArthur
Owner

Currently, the Events class is from MooTools 1.x. However, I somewhat question the API. addEvent and removeEvent seem somewhat poorly named, since you're not actually adding or removing events, but Listeners. Also, while it's how the spec works, it's inconvenient to hold onto the original function instance in order to remove it later, since they're often anonymous functions, or setup using Function.prototype.bind.

As is being considered in MooTools 2.0, I propose that the names be changed to something like addListener and removeListener, and that they return Listener objects, that easily provide attach and detach methods for later removal and re-adding.

Plus, the name Events seems wrong to me. It's not an object of multiple events. It's an EventEmitter.

/cc @pennyfx @zalun @csuwldcat

Sean McArthur
Owner

Also floating around in my brain, is how events are somewhat weak, at least as strings. Signals are a similar concept (event-driven programming) but require that actual objects exist, and you listen to those objects, instead of accidentally typing myEl.addEvent('cilck', myCallback); and then wondering why in the world your callback doesn't get called.

With signals, it would look like ClickEvent.add(myEl, myCallback), or something of that mindset. Even if we stick with Events with strings, it should probably be used similar to Java and Actionscript to prevent typos: myEl.addEvent(Event.CLICK, myCallback);

Piotr Zalewa

Events.Click.add(myEl, myCallback, scope, tag) would be fine IMO.
myEl could be a list of elements
adding scope so there would be no need to bind
tag would be a way to identify events for ev. future removal

Sean McArthur
Owner

That could work (Event objects (Signals)) also for Class Events. It could be like:

Model.Save = new Event;

//... later
var listener = Model.Save.addListener(module, myFn);
listener.detach() // for later removal

Perhaps every instance that inherits from the Class would have the Event somehow already bound to the instance, like so:

module.Save.addListener(myFn);

The addListener instance would return a Listener instance, that you could use to remove the event later. Regarding scope, I'm not convinced that binding a method is really that useful. 99% of the time, bind is only used like so: obj.myMethod.bind(obj), which I usually solve with a closure instead:

var obj = this;
el.Click.addListener(function() {
    obj.myMethod();
});
Daniel Buchner

I'm not sure I see the benefit from clobbering all the existing event infrastructure provided in MooTools. To be sufficiently decoupled from the DOM and autonomous, these modules need only know how to manage the addition and removal of their own events independent of other modules. I'm not sure I have ever personally experienced this use-case: "...instead of accidentally typing myEl.addEvent('cilck', myCallback); and then wondering why in the world your callback doesn't get called."

Can you provide a bullet point list of benefits from this abstraction, and perhaps where the current implementation is inadequate? Additionally, how much rework is this to achieve essentially what we already have?

Sean McArthur
Owner

I have typo-ed event strings before.

  • Reworking this module (Events) wouldn't take too long.
  • Reworking all the other modules to use this would mostly be a find and replace.
  • MooTools is considering doing similar (the returning of a Listener instance, at least) in 2.0
  • Biggest benefit: failing early. As I program more and more, I find that APIs that fail early are great, and those that don't fail until they're in the deepest bowels of their internal methods, suck. MooTools 1.x is guilty of this as well.
Sean McArthur
Owner

One more:

  • It removes the scaffolding of Custom Events to inheritance. No longer does addEvent need to check if there is event.base, and event.condition. Instead, that stuff can just happen by inheriting the Emitter.
Element.MouseEnter = new Class({
    Extends: Element.MouseOver,
    _condition: function() { 
        // this gets called naturally from .trigger(), or something.
    }
});
Sean McArthur seanmonstar referenced this issue from a commit November 14, 2011
Sean McArthur copied Custom Event support from MooTools
However, I quite hate this. I'd rather this get solved with the ideas from #4, and in the event-emitters branch.
f7d681d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.