JoseSilva JoseSilva - 6 months ago 18
jQuery Question

clone with onclick handler with args

i am cloning an element and that element will have a onclick handler. That event handler will send as argument an ID.
My problem is when i click on the element, he sends the last ID of the cicle for.

Here is my javascript code:

function cloneActions(idRoutine){ //clone elements
var actionDiv = document.getElementById("actionsImages-"+idRoutine).getElementsByClassName("actionImage"); //parent with content that must be cloned
var allActions = document.getElementById("actions"); //parent div
while (allActions.firstChild) { //remove all existing imgs
allActions.removeChild(allActions.firstChild);
}
for(var i=0; i < actionDiv.length; i++){ //set all images and handlers
var image = actionDiv[i].cloneNode(true);
var id = image.id;
image.onclick = function() { selectAction(id);};
allActions.appendChild(image);
}
}
function selectAction(id){ //handler function
///jquery
}


And the html code:

<div id="actionsImages-«id»" class="tile__list actionsImages">
<img src='sortable/st/foo.jpg' class='actionImage' id=«id» title='foo'/>
</div>


What am i doing wrong??

Answer

This is your lesson in for loops, variable hoisting, and asynchronous events in javascript. Don't worry, we all go through this lesson. Here goes:

Here's your for loop:

for(var i=0; i < actionDiv.length; i++) {
   var image = actionDiv[i].cloneNode(true);
   var id = image.id;
   image.onclick = function() { selectAction(id);};
   allActions.appendChild(image);
}

This is the same as doing this:

var image;
var id;
for(var i=0; i < actionDiv.length; i++) {
   image = actionDiv[i].cloneNode(true);
   id = image.id;
   image.onclick = function() { selectAction(id);};
   allActions.appendChild(image);
}

This is because variables in javascript are hoisted to the the top of their scope during run time. What this means is that your for loop runs, your variables image and id are set to a new value on every iteration of the loop, and when the loop is done, the variables are holding the value of the last iteration of your loop.

(read that last part in bold again)

Then, some time in the future, your user clicks on the image, and your click handler calls the function selectAction and passes the id. Well, if you read that text in bold once again, your id variable is going to be holding the value from the last iteration of your for loop.

(read that last part in bold again)

To illustrate the problem, lets strip it down to the bare bones. Copy this code into your console:

for (var i = 0; i < 5; i++) {
    var someVar = i;
    setTimeout(function () {
        console.log("The value of 'someVar' is " + someVar);
    }, i * 300);
}

See how someVar is equal to 4 every time. This is the crux of your problem.

To fix your problem, you can you use the "bind" function like this:

var image, id, i;
for(i=0; i < actionDiv.length; i++) {
   image = actionDiv[i].cloneNode(true);
   id = image.id;
   image.onclick = selectAction.bind(id)
   allActions.appendChild(image);
}

I'll let you read about how function binding works here.

Another solution is to have a function that you call inside your for loop:

function doCoolStuff (i) {
   var image = actionDiv[i].cloneNode(true);
   var id = image.id;
   image.onclick = function () { selectAction(id); };
   allActions.appendChild(image);
}

for(var i=0; i < actionDiv.length; i++) {
   doCoolStuff(i);
}