2

I am trying to make a script that injects interactable object information in a list of the markup page. Whenever I try to add an onclick event on a div, it works fine, however whenever I try to add more within a for loop, it does not work the way I intended.

I took a look of what is going on using breakpoints in the webpage debugger, and I see that the problem is that it seems to delete the event on the previous div before adding to the next div. In the end, the only event remaining is the last div after the loop exits.

I want to keep these events on all my divs, not just the last one... what seems to be the problem here?

var objects = ['Tom', 'Sauna', 'Traum'];

for (var i = 0; i < objects.length; i++){
    document.getElementById('list').innerHTML += "<div class='item' id='"+ i +"'>" + objects[i] + "</div>";
    document.getElementById(i).addEventListener("mouseup", function() {
        Select(this);
    });
}

function Select(char) {
    console.log(char);
}
div.item {
    border: 1px solid black;
    padding: 4px;
    margin: 4px;
}
<div id="list"></div>

enter image description here

1
  • I posted the answer suggesting event delegation, which now has a downvote and a deletion request. I'm happy to delete it if there's a reason the answer is not appropriate. Could whoever requested this please explain your reasoning? (I can't currently fathom why that answer might not be considered useful to OP. Am I overlooking something?) Thanks. Commented Mar 13, 2022 at 17:22

3 Answers 3

3

When you change innerHTML browser reconstructs the element's contents, throwing away all event handlers attached. Use DOM methods instead:

    for (let i = 0; i < objects.length; i++){
        var block = document.createElement('div');
        block.setAttribute('id', i);
        document.getElementById('list').appendChild( block );
        block.addEventListener("mouseup", function() {
            Select(this);
        });
    }

UPD: alternatively use a insertAdjacentHTML method instead of redefining innerHTML:

document.getElementById('list').insertAdjacentHTML(
    'beforeend', "<div id='"+ i +"'>" + i + "</div>");
Sign up to request clarification or add additional context in comments.

Comments

2

The reason is the way you are appending. innerHtml += effectively overwrites the existing content in the list. So, any elements that you added and bound are simply gone, and new items are added each time.

There are a couple ways to make this work.

First instead of assigning an innerHtml you can append elements.

const items = ['taco', 'apple', 'pork'];
const list = document.getElementById("list");

for (const item of items) {
  const el = document.createElement("div");
  el.addEventListener('click', (e) => console.log(`clicked ${item}`));
  el.innerText = item;
  
  list.appendChild(el);
}
<div id="list"></div>

Since we are appending an explicit element and not overwriting content, this will work.

A better approach would be to use delegation. We assign a single event handler onto the list and listen for any clicks. We then figure out what specific element was clicked.

const items = ['taco', 'apple', 'pork'];
const list = document.getElementById("list");
const add = document.getElementById("add");

list.addEventListener('click', (e) => {
 const parent = e.target.closest("[data-item]");
 
 if (parent != null) {
   console.log(`clicked on ${parent.dataset['item']}`);
 }
});

for (const item of items) {
  list.innerHTML += `<div data-item="${item}">${item}</div>`;
}

add.addEventListener('click', () => {
  const item = `item ${Date.now()}`;
  list.innerHTML += `<div data-item="${item}">${item}</div>`;
})
<div id="list"></div>

<button id="add">add</button>

The magic here is we assign a single event handler on the parent, and use closest to figure out what item was clicked. I'm using innerHTML here for simplicity but it should be avoided for security reasons.

Comments

-1

A good pattern to use when appropriate is event delegation. It allows following the Don't Repeat Yourself principle, making code maintenance considerably easier and potentially making scripts run significantly faster. And in your case, it avoids the pitfalls of an element being responsible for modifying its own content.

For example:

const container = document.getElementById('container');
container.addEventListener("click", toggleColor); // Events bubble up to ancestors

function toggleColor(event) { // Listeners automatically can access triggering events
  const clickedThing = event.target; // Event object has useful properties
  if(clickedThing.classList.contains("click-me")){ // Ensures this click interests us
    clickedThing.classList.toggle("blue");
  }
}
.click-me{ margin: 1em 1.5em; padding 1em 1.5em; }
.blue{ color: blue; }
<div id="container">
  <div id="firstDiv" class="click-me">First Div</div>
  <div id="firstDiv" class="click-me">Second Div</div>
</div>

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.