0

I have a parent div to which I add children divs in a loop. For each child I try to add an onclick event - so that the child div would change its color when clicked on:

    var div;
    for (var i=1; i<=size*size; i++)  {
        div = document.createElement("div");
        div.id = i;
        div.style.width = 480/size + "px";
        div.style.height = 480/size + "px";

        div.onclick= function()  {
            div.style.backgroundColor="orange";
        }
        resultNode.appendChild(div);
    }

However only the last div changes its color, no matter what child is clicked on. I suspect its because var div's last value is the last child, but shouldn't the onclick added to its previous objects remain with those objects once they are added inside resultNode?

enter image description here

I also tried adding this loop:

    var children=resultNode.children;
    for (var i=0; i<children.length; i++)  {
        children.item(i).onclick = function()  {
            children.item(i).style.backgroundColor="blue";
            alert(i);
        }
    }

However, it only works for a single child which index varies, depending of the amount of children. If there are only 25 (its the value of size*size in the code) children, the child with index 6 would get colored. If there are 100 children, the 11th child would get colored, if there are 400 children, 21th child would get colored. That is always the second div of the second row (there are size*size divs which form a square with size rows and size columns, meaning the div that gets colored is in the size+1 position):

enter image description here

I don't understand this behavior. Why does this happen and how should I edit the code to achieve the desired result (any child div when clicked on changes its color)?

parsecer
  • 4,758
  • 13
  • 71
  • 140
  • 1
    The reason why the snippets are not working is explained [in this post](http://stackoverflow.com/questions/750486/javascript-closure-inside-loops-simple-practical-example). – Teemu Feb 18 '19 at 16:53

2 Answers2

2

div does not reference what you are expecting within the click handler, see JavaScript closure inside loops – simple practical example. You can use event.target to reference the element where the event was dispatched

for (let i = 1; i <= 5; i++) {
  let div = document.createElement("div");
  div.id = i;
  div.textContent = i;
  div.onclick = e => e.target.style.backgroundColor = "orange";
  resultNode.appendChild(div);
}
<div id="resultNode"></div>

Alternatively

for (let i = 1; i <= 5; i++) {
  let div = document.createElement("div");
  ((div) => {
    div.id = i;
    div.textContent = i;
    div.onclick = e => div.style.backgroundColor = "orange";
    resultNode.appendChild(div);
  })(div)
}
<div id="resultNode"></div>
guest271314
  • 1
  • 15
  • 104
  • 177
  • 1
    Note, due to using `let` `div` will be the same `div` in first example within `onclick` handler. – guest271314 Feb 18 '19 at 17:07
  • Thank you. Coming from Java, Javascript is hard. Storing functions in array, primitives like `i` being seemingly passed be reference instead of by value. Mind-blowing. – parsecer Feb 18 '19 at 17:32
1

Your code would also work if you just tweak it as below:

var div;
    for (var i=1; i<=size*size; i++)  {
        div = document.createElement("div");
        div.id = i;
        div.style.width = 480/size + "px";
        div.style.height = 480/size + "px";

        div.onclick= function()  {
            this.style.backgroundColor="orange";
        }
        resultNode.appendChild(div);
    }

Inside onclick function just change it to this.style.background = 'orange' instead of div.style.background = 'orange' object. Your code was not working because div object is pointing to your last div.

Mahesh More
  • 821
  • 1
  • 8
  • 23