12

I've this layout that was created dynamically:

for (let i = 1; i < 10; i++) {
  document.querySelector('.card-body').innerHTML += `<div class="row" id="img_div">
        <div class="col-12 col-sm-12 col-md-2 text-center">
          <img src="http://placehold.it/120x80" alt="prewiew" width="120" height="80">
        </div>
        <div id="text_div" class="col-12 text-sm-center col-sm-12 text-md-left col-md-6">
        <h4 class="name"><a href="#" id="title` + i + `">Name</a></h4>
          <h4>
            <small>state</small>
          </h4>
          <h4>
            <small>city</small>
          </h4>
          <h4>
            <small>zip</small>
          </h4>
        </div>
        <div class="col-12 col-sm-12 text-sm-center col-md-4 text-md-right row">
        </div>
      </div>
     `
  document.getElementById("title" + i).addEventListener('click', function() {
    console.log(i)
  });
}
<link rel="stylesheet" href="https://stackpath.bootstrapcdn.com/bootstrap/4.3.1/css/bootstrap.min.css" integrity="sha384-ggOyR0iXCbMQv3Xipma34MD+dH/1fQ784/j6cY/iJTQUOhcWr7x9JvoRxT2MZw1T" crossorigin="anonymous">
<div class="card-body">
  <!-- person -->
</div>

And I want to get the event click on each h4 class="name" and show a log with the number i related.

However, console.log shows only the last i related (i=9 in this case), and doesn't work with the other i numbers. Why does this happen? What do I have to do?

double-beep
  • 5,031
  • 17
  • 33
  • 41
Luiz Adorno
  • 127
  • 8
  • 3
    Maybe use [Document.createElement()](https://developer.mozilla.org/en-US/docs/Web/API/Document/createElement) instead of passing html-string & you'll have a much easier time & i think you'll end up with better code in the end. – admcfajn Feb 14 '20 at 22:58
  • 1
    you''ll have to loop through your nodes again and attach an event you cannot attach into a string – EugenSunic Feb 14 '20 at 22:59
  • Nice question, it's kinda tricky for sure, I think it might be possible to attatch an event post rendering.. For example after your `for` loop calling a method to apply events to those elements, but I don't know. will jsFiddle to test. – Pogrindis Feb 14 '20 at 23:02
  • Created a delegated event handler for `.card-body` and check if the element (`target`) is an anchor with and ID that starts with `title`. – hungerstar Feb 14 '20 at 23:03
  • 1
    Ah, just about to post an answer. Voted to reopen. Closures aren't necessary, the problem is `innerHTML += ...` killing the previously set event listeners, not the closure. Use `document.createElement` instead. See [this thread](https://stackoverflow.com/questions/38361875/element-innerhtml-getting-rid-of-event-listeners) – ggorlen Feb 14 '20 at 23:05
  • You are attaching the event listener inside the loop. You loose the event listener when you recreate the elements. 2 solutions: move the creation of the event listeners outside the first loop, or instead of building the entire innerhtml, build each div and append it to the dom. – Diogo Gomes Feb 14 '20 at 23:12

2 Answers2

9

Using += on innerHTML destroys event listeners on the elements inside the HTML. Using document.createElement ensures event listeners are preserved after appending children to a container.

Here's a minimal, complete example:

for (let i = 1; i < 10; i++) {
  const anchor = document.createElement("a");
  document.body.appendChild(anchor);
  anchor.id = "title" + i;
  anchor.href = "#";
  anchor.textContent = "link " + i;
  document.getElementById("title" + i)
    .addEventListener("click", e => console.log(i));
}
a {
  margin-left: 0.3em;
}

Of course, the document.getElementById isn't necessary here since we just created the object in the loop block. This approach might help avoid generating ids dynamically, which seems like an antipattern.

for (let i = 1; i < 10; i++) {
  const anchor = document.createElement("a");
  document.body.appendChild(anchor);    
  anchor.href = "#";
  anchor.textContent = "link " + i;
  anchor.addEventListener("click", e => console.log(i));
}
a {
  margin-left: 0.3em;
}

If you have arbitrary chunks of stringified HTML as in your example, you can use createElement("div"), set its innerHTML once to the chunk and add listeners as needed. Then append the divs as children of your container element.

for (let i = 1; i < 10; i++) {
  const rawHTML = `<div><a href="#" id="link-${i}">link ${i}</a></div>`;
  const div = document.createElement("div");
  document.body.appendChild(div);
  div.href = "#";
  div.innerHTML = rawHTML;
  div.querySelector(`#link-${i}`)
    .addEventListener("click", e => console.log(i));
}

Other approaches include appending to the innerHTML but providing a class name on the links, then using a document.querySelectorAll(className) after the HTML is built to add listeners to each link, or using event delegation to add a listener to the parent.

As a final note, usually styled <button> elements are preferred over links that have a no-action # href.

ggorlen
  • 44,755
  • 7
  • 76
  • 106
  • 1
    This is the approach I was considering, and I would hands down give this the solution, but the other answer is great too. – Pogrindis Feb 14 '20 at 23:09
  • Thanks for de answer, but i tried to build the layout with createElement and i was unsuccessful – Luiz Adorno Feb 14 '20 at 23:15
  • 1
    No problem. Hopefully you realize if you have a big chunk of HTML, you can create a root element container like a `
    `, then `div.innerHTML += yourHugeChunkOfHTML`. So there's no reason this shouldn't work for any use case.
    – ggorlen Feb 14 '20 at 23:16
  • 1
    I will try to build this layout with your recommendation, I am afraid to use "document.getElementById" several times and slow the performance – Luiz Adorno Feb 14 '20 at 23:23
7

innerHTML redraws the full html, as a result all the previously attached events are lost. Use insertAdjacentHTML()

for(let i=1;i<10;i++){
    document.querySelector('.card-body').insertAdjacentHTML('beforeend',`<div class="row" id="img_div">
        <div class="col-12 col-sm-12 col-md-2 text-center">
          <img src="http://placehold.it/120x80" alt="prewiew" width="120" height="80">
        </div>
        <div id="text_div" class="col-12 text-sm-center col-sm-12 text-md-left col-md-6">
        <h4 class="name"><a href="#" id="title`+i+`">Name</a></h4>
          <h4>
            <small>state</small>
          </h4>
          <h4>
            <small>city</small>
          </h4>
          <h4>
            <small>zip</small>
          </h4>
        </div>
        <div class="col-12 col-sm-12 text-sm-center col-md-4 text-md-right row">
        </div>
      </div>
     `);
   document.getElementById("title"+i).addEventListener('click', function () {
    console.log(i)
  });
}
<link rel="stylesheet" href="https://stackpath.bootstrapcdn.com/bootstrap/4.3.1/css/bootstrap.min.css" integrity="sha384-ggOyR0iXCbMQv3Xipma34MD+dH/1fQ784/j6cY/iJTQUOhcWr7x9JvoRxT2MZw1T" crossorigin="anonymous">
<div class="card-body">
<!-- person -->
</div>
Mamun
  • 66,969
  • 9
  • 47
  • 59
  • 3
    Wow.. I've been messing around with simular things and never once came accross `insertAdjacentHTML` - Everydays a schoolday, really nice. – Pogrindis Feb 14 '20 at 23:08
  • @ggorlen I kinda agree on the perf area out of interest would using the class `name` and attatching the event to just that class while passing through the element (or maybe just reading from the event itself) be more efficient ? – Pogrindis Feb 14 '20 at 23:18
  • 1
    Definitely worth a benchmark, I can't think of anything better to do tonight! :) – Pogrindis Feb 14 '20 at 23:24