Opened 7 years ago

Closed 5 years ago

#15481 closed enhancement (wontfix)

dojo/on needs context argument

Reported by: Mike Wilcox Owned by: dylan
Priority: high Milestone: 1.11
Component: Events Version: 1.7.2
Keywords: Cc: bill
Blocked By: Blocking:

Description

As per the email discussion from 3/2/2012,

http://mail.dojotoolkit.org/pipermail/dojo-contributors/2012-March/026840.html

Dojo, being an object oriented framework should provide assistance to fix context (this), by providing a argument for it. As Bill pointed out, method.bind(this) does not always work.

Change History (6)

comment:1 Changed 7 years ago by Mike Wilcox

Cc: bill added
Owner: set to Kris Zyp
Priority: undecidedhigh
Status: newassigned

comment:2 Changed 7 years ago by bill

Component: CoreEvents

I thought there was another ticket for this but I can't find it. The corollary question is of course about a context for dojo/aspect::before/after/around. PS: you can see the whole thread from http://thread.gmane.org/gmane.comp.web.dojo.devel/17103.

comment:3 Changed 7 years ago by jwvmarkus

It would also be very helpful to have the actual eventTarget as an argument, because right now we cannot use lang.hitch in our on construction because the target is only available as the context object.

line 175 to 186 in dojo/on V1.7.2

return on(target, eventType, function(event){
    var eventTarget = event.target;
    // see if we have a valid matchesTarget or default to dojo.query
    matchesTarget = matchesTarget && matchesTarget.matches ? matchesTarget : dojo.query;
    // there is a selector, so make sure it matches
    while(!matchesTarget.matches(eventTarget, selector, target)){
        if(eventTarget == target || !children || 
            !(eventTarget = eventTarget.parentNode)){  // intentional assignment
            return;
	}
    }
    return listener.call(eventTarget, event);
});

The eventTarget could be changed depending on the HTML but is only used as context. This means when using lang.hitch in the on, to keep the current context, the eventTarget is lost. Having to calculate it again is inefficient.

Current situation:

activate: function() {
    var self = this;
    on(win.body(), '[monitorClick]:click', function(e) {self.monitorClick(this, e); })
},
monitorClick: function(target, e) {
    //handle click here...
}

Desired situation:

activate: function() {
    on(win.body(), '[monitorClick]:click', lang.hitch(this, 'monitorClick'));
},
monitorClick: function(e, eventTarget /* or as part of e somewhere! */) {
    //handle click here...
}

Desired situation 2 (with context):

activate: function() {
    on(win.body(), '[monitorClick]:click', this, 'monitorClick');
},
monitorClick: function(e, eventTarget /* or as part of e somewhere! */) {
    //handle click here...
}

comment:4 in reply to:  3 Changed 6 years ago by iDo

Previous patch from jwvmarkus was working only with on.selector. Here is a patch that can work for any on().

Index: on.js
===================================================================
--- on.js
+++ on.js
@@ -37,12 +37,17 @@
 		has.add("event-orientationchange", has("touch") && !has("android")); // TODO: how do we detect this?
 	}
 	var on = function(target, type, listener, dontFix){
-		if(target.on){ 
+		// We save the real event target into the event, instead of the context
+		var l = function(event){
+			if (typeof event === 'object') {
+				event.realEventTarget = this;
+			}
+			return listener.apply(this, arguments);
+		};
+		if(target.on){
 			// delegate to the target's on() method, so it can handle it's own listening if it wants
-			return target.on(type, listener);
+			return target.on(type, l);
 		}
 		// delegate to main listener code
-		return on.parse(target, type, listener, addListener, dontFix, this);
+		return on.parse(target, type, l, addListener, dontFix, this);
 	};
 	on.pausable =  function(target, type, listener, dontFix){
 		// summary:

Replying to jwvmarkus:

It would also be very helpful to have the actual eventTarget as an argument, because right now we cannot use lang.hitch in our on construction because the target is only available as the context object.

line 175 to 186 in dojo/on V1.7.2

return on(target, eventType, function(event){
    var eventTarget = event.target;
    // see if we have a valid matchesTarget or default to dojo.query
    matchesTarget = matchesTarget && matchesTarget.matches ? matchesTarget : dojo.query;
    // there is a selector, so make sure it matches
    while(!matchesTarget.matches(eventTarget, selector, target)){
        if(eventTarget == target || !children || 
            !(eventTarget = eventTarget.parentNode)){  // intentional assignment
            return;
	}
    }
    return listener.call(eventTarget, event);
});

The eventTarget could be changed depending on the HTML but is

Last edited 6 years ago by iDo (previous) (diff)

comment:5 Changed 5 years ago by dylan

Milestone: tbd1.11
Owner: changed from Kris Zyp to dylan

comment:6 Changed 5 years ago by Bryan Forbes

Resolution: wontfix
Status: assignedclosed

You have raised a good point about the selector target in the dojo/on API and we're going to track that going forward at #18355 since it is really a separate issue from the original intent of this ticket. We don't intend to fix the original ticket, so we will be closing it.

Note: See TracTickets for help on using tickets.