0

So here's my code:

for(var i = 0; i < clickCell.length; i++) {
  clickCell[i].addEventListener("click", function(x) {
    showModal(i);
  });
}

function showModal(x) {
  switch (x) {
    case 1:
      image.style.background = "url('image1.jpg') center/cover no-repeat";
      break;
    case 2:
      image.style.background = "url('image2.jpg') center/cover no-repeat";
      break;
    case 3:
      image.style.background = "url('image3.jpg') center/cover no-repeat";
      break;
    default:
      break;
  }
  modal.style.display = "block";
}

What I'm trying to do is display a modal with a different image based on what cell was clicked on in a table element. The for loop adds an event listener to each cell in the table. The showModal() function picks which image should be displayed based on which cell was clicked. My problem is that it seems only the last iteration of the loop counter i is being passed as an argument to showModal(). Now from what I've searched up, the solution seems to be something to do with closure. I've done some reading and I just can't seem to make the connection in my head. If someone could explain how I could get this piece of code to work and help me understand closure that would be greatly appreciated.

mmulenga
  • 1
  • 1
  • In your code above, where and how is `image` and `modal` defined. – Leeish Jul 26 '17 at 22:15
  • Replace `var i = 0` with `let i = 0`, then fix your cases; x is going to be 0, 1 or 2. –  Jul 26 '17 at 22:16
  • 1
    It should also be noted that you can simply insert x into the style string, which means you can replace the switch block with: `image.style.background = "url('image" + (x + 1) + ".jpg') center/cover no-repeat";` –  Jul 26 '17 at 22:18
  • @chrisG using let will indeed fix the problem, but OP never stated that he was able to use es2015, and on top of that, I don't think making functions inside loops should be encouraged, even more if you don't know why this will work with let and not with var – Logar Jul 26 '17 at 22:25
  • @ChrisG Thanks for the `let` answer! It ended up working! – mmulenga Jul 27 '17 at 06:02
  • @Logar There aren't any restrictions on what I can or can't use. From my understanding `let` works because it restricts the scope of the variable to the for loop, whereas var is more global. Could you elaborate on why you think making functions inside loops should be discouraged? Also the reason I wrapped the `showModal()` function in the loop was because click was firing immediately upon the page loading, I don't know if that affects your answer. – mmulenga Jul 27 '17 at 06:04
  • Making functions inside loops is apparently considered bad practice, the alternative is to use a single function and assign that to all elements as click handler. Example: https://jsfiddle.net/vbrwhkda/1/ –  Jul 27 '17 at 10:32
  • @mmulenga I think it is considered a bad practice because what your code actually does (with the var keyword) is a bit counter intuitive. This is why the original question is so much upvoted. If you don't wonder why it works this way, you will intuitively expect another behavior from your closure. So, personnal point of view, this should be avoided when possible. – Logar Jul 27 '17 at 13:46

0 Answers0