Dynamically adding div's and setting onclick value in JavaScript for loop

Question

I'm using vanilla JavaScript to create a photo carousel; reading in values from an external JSON file, then looping through the JSON data to dynamically add thumbnails, a title, and an onclick to the carousel div. All the images and titles work fine, but the onclick passes only the last value used regardless of which thumbnail is clicked.

Here's the JSON file

var Scenes = {
    "Title":["St. Mary Magdalene","Black Mountains","Pen Twyn Glas","Carreg Yr Ogof Cave","Beinn Narnain"],
    "Thumbnail":["TNStMary.jpg","TNMountains.jpg","TNPenTwynGlas.jpg","TNCarregYrOgof.jpg","TNBeinnNarnain.jpg"],
    "PanoSet":["001x","145+","183+","400x","500x"]
};

Here's the code that is dynamically adding the thumbnails, titles, and onclick.

var Scenes = {
    "Title":["St. Mary Magdalene","Black Mountains","Pen Twyn Glas","Carreg Yr Ogof Cave","Beinn Narnain"],
    "Thumbnail":["TNStMary.jpg","TNMountains.jpg","TNPenTwynGlas.jpg","TNCarregYrOgof.jpg","TNBeinnNarnain.jpg"],
    "PanoSet":["001x","145+","183+","400x","500x"]
};

var Repeat = (Scenes.Title.length);

for (i = 0; i < Repeat; i++) {
  NewDiv = document.createElement('div');
  NewDiv.id = 'Scene' + i;
  document.getElementById('Carousel').appendChild(NewDiv);
  document.getElementById('Scene' + i).className = "Scene";
  document.getElementById('Scene' + i).style.position = "absolute";
  document.getElementById('Scene' + i).style.top = "5px";
  document.getElementById('Scene' + i).style.left = 5 + (170 * i) + "px";
  document.getElementById('Scene' + i).onclick = function() {
    addScene(Scenes.PanoSet[i]);
  };

  NewDiv = document.createElement('div');
  NewDiv.id = 'SceneThumbnail' + i;
  document.getElementById('Scene' + i).appendChild(NewDiv);
  document.getElementById("SceneThumbnail" + i).className = "SceneThumbnail";
  document.getElementById("SceneThumbnail" + i).style.backgroundImage = "url(" + Scenes.Thumbnail[i] + ")";

  NewDiv = document.createElement('div');
  NewDiv.id = 'SceneTitle' + i;
  document.getElementById('Scene' + i).appendChild(NewDiv);
  document.getElementById("SceneTitle" + i).className = "SceneTitle";
  document.getElementById('SceneTitle' + i).innerHTML = Scenes.Title[i];
}

<div id='Carousel'><div>

This is the line that appears to be the problem (I included all above so as to give context)

document.getElementById('Scene'+i).onclick = function() {addScene(Scenes.PanoSet[i]);};

Which ever of the newly created divs I click on, the code "500x" is passed to my addScene function.

I'm expecting that I've over looked something simple. Can someone offer a solution?

You don't need to call document.getElementById('Scene'+i) over and over, you have the element at NewDiv just use that — Liam
– Liam, Commented Jul 24, 2020 at 7:18
Does this answer your question? JavaScript closure inside loops – simple practical example — Billy Brown
– Billy Brown, Commented Jul 24, 2020 at 7:18
i is muted by you loop, so each click has the same reference to i. As above, this is a dupe — Liam
– Liam, Commented Jul 24, 2020 at 7:19
Other than fixing the i you could also look into event delegation (onclick event listener on the parent element, get what you need from the event target). — pawel
– pawel, Commented Jul 24, 2020 at 7:27

Josh Wulf · Accepted Answer · 2020-07-24 07:42:06Z

2

Classic JS gotcha. Do this:

document.getElementById('Scene'+i).onclick = ((id) => () =>addScene(Scenes.PanoSet[id]))(i);

You need to bind in the value of i, rather than a reference. The handler code doesn't get evaluated until it gets executed, so the value of i at that time will be the value it has when the loop has completed.

Doing it using an IIFE (immediately-invoked function expression) creates a context with the correct static value of i bound into the scope of the handler that it returns.

answered Jul 24, 2020 at 7:42

Josh Wulf

4,9122 gold badges24 silver badges35 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

Karan · Accepted Answer · 2020-07-24 07:37:09Z

You can find index like below. Inside onclick using this you will get clicked div. Then get id with this.getAttribute('id') and replace Scene with ''.

let x = this.getAttribute('id').replace('Scene', '');

Try it below.

let Scenes = {
  "Title":["St. Mary Magdalene","Black Mountains","Pen Twyn Glas","Carreg Yr Ogof Cave","Beinn Narnain"],
  "Thumbnail":["TNStMary.jpg","TNMountains.jpg","TNPenTwynGlas.jpg","TNCarregYrOgof.jpg","TNBeinnNarnain.jpg"],
  "PanoSet":["001x","145+","183+","400x","500x"]
};


var Repeat = (Scenes.Title.length);

for (i = 0; i < Repeat; i++) {
  NewDiv = document.createElement('div');
  NewDiv.id = 'Scene' + i;
  document.getElementById('Carousel').appendChild(NewDiv);
  document.getElementById('Scene' + i).className = "Scene";
  document.getElementById('Scene' + i).style.position = "absolute";
  document.getElementById('Scene' + i).style.top = "5px";
  document.getElementById('Scene' + i).style.left = 5 + (170 * i) + "px";
  document.getElementById('Scene' + i).onclick = function() {
    let x = this.getAttribute('id').replace('Scene', '');
    console.log(x);
    addScene(Scenes.PanoSet[x]);
  };

  NewDiv = document.createElement('div');
  NewDiv.id = 'SceneThumbnail' + i;
  document.getElementById('Scene' + i).appendChild(NewDiv);
  document.getElementById("SceneThumbnail" + i).className = "SceneThumbnail";
  document.getElementById("SceneThumbnail" + i).style.backgroundImage = "url(" + Scenes.Thumbnail[i] + ")";

  NewDiv = document.createElement('div');
  NewDiv.id = 'SceneTitle' + i;
  document.getElementById('Scene' + i).appendChild(NewDiv);
  document.getElementById("SceneTitle" + i).className = "SceneTitle";
  document.getElementById('SceneTitle' + i).innerHTML = Scenes.Title[i];


}

<div id='Carousel'></div>

Collectives™ on Stack Overflow

Dynamically adding div's and setting onclick value in JavaScript for loop

2 Answers 2

Comments

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related