consider using event delegation in _ListMouseMixin

[bill]

_ListMouseMixin could leverage dojo/on's event delegation features to track events at the level of list items, without worrying about complications from sub-nodes within each list item: i.e. having to trace evt.target up the DOM tree, ignoring mouseout from a list item into a subnode of that same list item, etc.

I'm attaching a patch which seems to work but I don't understand the intricacies of the _ListMouseMixin code, in particular about the distinction between dragging and hovering.

[bill]

patch against [27509]

[bill]

PS: the downside of this is the dojo.query() dependency. I filed #14575 to see if that can be worked around.

[bill]

OK, Kris enhanced dojo/on so the dojo.query dependency is no longer necessary to do event delegation. But looking at _ListTouchMixin.js (and it's hard to test due to #14124), it doesn't seem like dojox/mobile needs event delegation at all.

Attaching a new patch.

If for some reason in dojox/mobile each list item has sub-nodes, and the click event might occur on a sub-node, then something like this should work:

var self = this,
	root = this.domNode;
this._connects.push(on(root, on.selector(function(evt){
	return evt.target.parentNode == root;
}, "click"), function(evt){
	self._onClick(this, evt);
}));

Perhaps that code is there for the case when list items are rich text, and a click event could occur on <b>, <i>, etc.?

[bill]

patch against [27513]

[Kris Zyp]

Bill, just a note, make sure your custom selector function actually returns a node, if it matches, as it is responsible for actually *finding* the node, not just indicating a boolean match (and perhaps my fix for #14575 was exactly what you intended).

[bill]

@kzyp - By

*finding* the node

I assume you mean just returning the node, rather than tracing up the tree looking for a node?

It's true that the example in your test file returns a node:

on.selector(function(node){
      return node.tagName == "BUTTON" && node; 
}, "click")

But I'm confused, I thought the custom selector function was supposed to match (no pun intended) the behavior of dojo.query.matches(), which returns a boolean:

query.matches = engine.match || function(node, selector, root){
	// summary:
	//		Test to see if a node matches a selector
	return query.filter([node], selector, root).length > 0;
};

[bill]

Reopened #14575 as it isn't working as I expected.

[Douglas Hays]

revised patch w/o query dep

[Douglas Hays]

added patch for testing

[bill]

I looked at it, and it seems to be working. A few things:

• I got very confused by the function name_onListItem(), which sounds like it's called every time an event happens (like _onClick()), but is actually a helper for setting up connections. I guess you should just call it something else, plus of course add a summary.
• The event.stop() calls look scary since I had that in _HasDropDown but had to change it to just preventDefault() instead. Another downside is that they introduce a dependency on dojo/_base/event.
• For some reason you added spurious spaces to some of the comments.

[Douglas Hays]

In [28829]:

Fixes #14574. Change this.connect to this.own(on(... and add list item target as second parameter to event handler callbacks.`