...

View Full Version : adding event handler to javascript createed imgs



keajav
02-02-2006, 01:17 AM
hi all,

i'm adding imgs with an onclick to expand/retract child nodes on a ajax contstructed tree.

here's the code:

for (i=0;i<ds.Tables[0].Rows.length;i++){
var imgPlus = document.createElement('img');
imgPlus.id = "nodesx" + itemID;
imgPlus.src = "images/btn_plus.jpg";
imgPlus.onclick = _ function(){expandRetractNode(imgPlus)};
tcell.appendChild(imgPlus);


and of course, appendChild for the cell and the row to the table body.

so then i'm using the expandRetractNode function to determine the id of the parent from the id of the img (splitting a string), however the problem is that the function is associating every button as the id of the last one created.

is this because i need to use attachEvent instead of the "imgPlus.onclick =" ?

i believe the id's are setting properly becuase i alerted the value being set in the image creation loop.

thanks for any advice, if you need me to provide further info/code let me know

straycat264
02-02-2006, 12:32 PM
The problem is that when your onclick functions are called, they are called with the latest value of imgPlus - which in this example, will be where it was left at the end of the loop.

You have two options: create a closure:


...
imgPlus.onclick = createImgOnclick(imgPlus);
...
...

function createImgOnclick(img)

{
return function() { expandRetractNode(img); };
}


or the much simpler option of using this instead of imgPlus, thus:



imgPlus.onclick = function() { expandRetractNode(this); };

Kor
02-02-2006, 01:39 PM
The problem is that when your onclick functions are called, they are called with the latest value of imgPlus - which in this example, will be where it was left at the end of the loop.

You have two options: create a closure:


...
imgPlus.onclick = createImgOnclick(imgPlus);
...
...

function createImgOnclick(img)

{
return function() { expandRetractNode(img); };
}


or the much simpler option of using this instead of imgPlus, thus:



imgPlus.onclick = function() { expandRetractNode(this); };


I guess that you make a confusion...

imgPlus.onclick = createImgOnclick(imgPlus);

woun't never ever work, it is an illegal code, as createImgOnclick(imgPlus) is not a variable/object;

The two DOM 1 methods to dynamically self changing the object's attributes are:
1. using an annonymous simple:

object.onclick=function(){
this.attribute=value;
}

...and using an anonnymous with a closure:

object.onclick=function(){
foo(this);
}
function foo(obj){
obj.attribute=value;
}

jkd
02-02-2006, 03:40 PM
I guess that you make a confusion...

imgPlus.onclick = createImgOnclick(imgPlus);

woun't never ever work, it is an illegal code, as createImgOnclick(imgPlus) is not a variable/object;

Uh, it was clearly defined in their code, and does work. It returns a function to use as the event listener. I use closures like that all the time, though I tend to keep them anonymous:



imgPlus.onclick = (function(img) {
return function() {
expandRetractNode(img);
}
})(imgPlus);


I do agree that "this" is the best solution for this instance, but the closure technique is often the best way to go.

Kor
02-02-2006, 04:57 PM
are u sure? Compare this first - an incorrect uppon my oppinion - (in IE and Moz);


<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN" "http://www.w3.org/TR/html4/loose.dtd">
<html>
<head>
<title>Untitled Document</title>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1">
<meta http-equiv="Content-Style-Type" content="text/css">
<meta http-equiv="Content-Script-Type" content="text/javascript">
<script type="text/javascript">
onload=function(){
var root=document.getElementsByTagName('body')[0];
var obj = document.createElement('div');
obj.appendChild(document.createTextNode('blah'));
obj.onclick = myFunction(obj);
root.appendChild(obj);
}
function myFunction(obj){
obj.style.color='#cccccc'
}
</script>
</head>
<body>

</body>
</html>

with this, I beleive, correct one:


<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN" "http://www.w3.org/TR/html4/loose.dtd">
<html>
<head>
<title>Untitled Document</title>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1">
<meta http-equiv="Content-Style-Type" content="text/css">
<meta http-equiv="Content-Script-Type" content="text/javascript">
<script type="text/javascript">
onload=function(){
var root=document.getElementsByTagName('body')[0];
var obj = document.createElement('div');
obj.appendChild(document.createTextNode('blah'));
obj.onclick = function(){myFunction(obj);}
root.appendChild(obj);
}
function myFunction(obj){
obj.style.color='#cccccc'
}
</script>
</head>
<body>

</body>
</html>


Are u sure that

object.onevent =somefunction(someparameter)

is a correct code?

straycat264
02-02-2006, 05:30 PM
Absolutely sure. It all depends what somefunction(someparameter) does and returns.

In the example you give, it will not work, as:

obj.onclick = myFunction(obj);

will execute myFunction instantly, and assign null to obj.onclick.

In my example, createImgOnclick(img) does nothing but return a function object. We're talking about two completely different cases.

I agree with jkd that the this method is the better one in this case, but I gave the closure example partly to illustrate how to overcome this particular problem - which is a common one.

I'd suggest having a look at closures (http://jibbering.com/faq/faq_notes/closures.html). They're very very useful.

Kor
02-02-2006, 05:33 PM
I know closures. I know how to avoid them... This was not the main point of the thread...

Kor
02-02-2006, 05:37 PM
using a closure and return the value of the function looks quite an intricate and redundant way to self pass an object to another function... I have not found yet a case in which the simple assigment or a double closure should have not resolve elegant the problem...

straycat264
02-02-2006, 05:53 PM
Well - I did wonder, given your initial response. I stand corrected :)

There are certainly times when a double closure is better. There are times when it might result in too much code duplication, so separating it into a function is better. When giving examples like this, I'll always separate it, as I believe it's easier to understand.

Kor
02-02-2006, 06:04 PM
Yes I got the point. :)

Speaking about closures... Here's a good article written by a fellow countryman about the way some closures may affect browers (IE especially) and some interesting solutions to avoid them....

http://www.bazon.net/mishoo/articles.epl?art_id=824

jkd example follows exactly one of those ways to avoid harmful closures.

keajav
02-02-2006, 07:19 PM
thanks for all your suggestions, i changed one word in the code i presented initially:

imgPlus.onclick = function(){expandRetractNode(imgPlus)}; into->
imgPlus.onclick = function(){expandRetractNode(this)};

and it is working properly.

Kor
02-02-2006, 09:07 PM
you see, in your case it was a matter of logical understanding of a loop. When using a closure whithin a loop and trying to do something with an element which depends on the indent of that loop, all you will achieve will be always the last element of the loop. The loop is executed before, so that when the closure tries to reference the element, will find always the array.length-1 value as i. This kind of problem might be solved even in a third way, by re-building the loop (in fact a new loop) inside the annonymous function. This solution will avoid the closure lack of memory problem as well. It is to be used mainly when dealing with 2nd deg. arrays (array of arrays)



EZ Archive Ads Plugin for vBulletin Copyright 2006 Computer Help Forum