Add target of selector from on.selector as property of event object

[Bryan Forbes]

Currently, on.selector() (and the 'selector:eventType' syntax of on()) set the target of the selector to the context (this) of the listener. For instance, imagine the following:

<ul id="parent">
    <li class="clickable" data-id="0"><span>Foo</span></li>
    <li data-id="1"><span>Bar</span></li>
</ul>

on(dom.byId('parent'), 'li.clickable:click', function (event) {
    // If the user clicks on the text "Foo", `event.target` will be
    // the <span> within the <li> so it won't have the `data-id`
    // attribute on it. To get the `data-id` attribute, one must use
    // the context of the function:
    var id = this.getAttribute('data-id');
});

This is fine and dandy until you want to bind the context of your listener. This is something that is done quite frequently in widgets:

on(this.containerNode, 'li.clickable:click', lang.hitch(this, 'onRowClick'));

In this scenario, the context of the listener is set to the widget and the target of the selector is lost. It would be nice if the event object passed to listeners would add a property set to the target of the selector:

on(node, 'selector:eventType', function (event) {
    event.selectorTarget === this
});

I'd like to get feedback on this before continuing with a patch.

[ido]

@BryanForbes?: What about re-assigning event.currentTarget in case of event delegation ? In you example, with the current dojo/on event.currentTarget will be 'node'. Maybe it can be changed to be selectorTarget.

Or maybe we could override a non standard property like ​https://developer.mozilla.org/en-US/docs/Web/API/event.originalTarget

If a new property is preferred, I propose 'matchTarget' as internally the dojo/on function is 'on.matches'

Anyhow, I am available and would be happy to work on a patch :)

[Bryan Forbes]

I think we should use a new property rather than overwrite a property that a browser already uses. I've seen too many cases in browsers (Chrome, IE) where overwriting properties on events causes the browser to throw errors. That being said, I prefer selectorTarget, but matchTarget is also a good name (since the node is the matching node of the selector) and I would not be opposed to it.

[ido]

@BryanForbes? : I have created pull request : ​https://github.com/dojo/dojo/pull/120 I named the property "matchTarget" because my patch returns it for any event on dom object. I though it would be more consistent to have this extra property for delegation as well as for direct events. "seletorTarget" is weird in case of non delegated event.

What do you think? Should I change the pull request to add the property only for delegated event?

[Bryan Forbes]

My preference is to only add matchTarget when there is a delegated event. The biggest reason is that even though you're adding it for most DOM events, matchTarget still won't show up in the general case (non-DOM events, events with a non-object as their first argument, etc.) and you're adding an extra function execution for every listener that goes through on() for a specialized property. I'd like to hear others weigh in on this, but it just seems like extra overhead for a property that may or may not be there anyway.

[ido]

You're right for the extra function, and I am mitigated as well... It is there for avoiding mistakes more than for technical reason. Actually the patch I propose is similar to what we have in our dojo version since quite long time. Our first patch was adding it only in case of event delegation. But it turns out that developers were always confused... It's a known fact that non-dom events are totally different from dom event object however people were expecting to have always the same type of event object with dom element. At some point we decided to add the property for every dom events to avoid this confusion. Since then, no more defect about undefined property.

So yeah, I totally agree with you about the overhead but it feels more convenient... I am 50/50...

I hope we will have other's opinion...

[Kris Zyp]

I would vote for the name selectorTarget, and only assigning on event delegation (and then assigning it on.selector, where it would just be a single line assignment, since you already have the reference).

[bill]

With event delegation in native javascript, the actual target is reported in evt.target. At least according to ​http://davidwalsh.name/event-delegate.

Why doesn't that approach work with dojo/on?

[Bryan Forbes]

The example that David shows is very simple: there is only text in the <li>s in the list which is why event.target is the <li>. From MDN's ​event.target reference:

This property of event objects is the object the event was dispatched on. It is different than event.currentTarget when the event handler is called in bubbling or capturing phase of the event.

If you add a more complex structure, David's example wouldn't work because the event is dispatched on the thing that is actually clicked on. As he says later in his article:

For example, if the A element had a SPAN tag in it, the SPAN tag would be the target element. In that case, we'd need to walk up the DOM tree to find out if it contained an A.classA element we were looking for.

You can see that here: ​http://jsfiddle.net/gva1mnwm/ by clicking on "click for span" and looking at the console. The extension event that on.selector produces walks up the DOM tree from event.target checking to see if any of the nodes between event.target and the node that the handler was attached to match the selector (just like David suggests).

The approach that David uses works with dojo/on and behaves exactly the same as using addEventListener, however since it's a simple example, it doesn't show the problem that this ticket is covering.

[bill]

Ah right, makes sense. So, having a new property name sounds good to me. (I have no preference about the name.)

[ido]

I changed the pull request according to comments. now "selectorTarget" is added only for delegated events

[dylan]

Fixed in ​https://github.com/dojo/dojo/commit/118120722bf6da3ff8a1d9d73eef55c27c6dc428