Adding event Listener to elements on click of another in loop

user_14579719 0
Viewed 426

I have quite annoying problem with my typescript projekt. Nothing to fancy but for some reason once i click one of the elements in any of these 3 menus the onclick is executed as many times as the number of elements in particular menu instead of just once per click.

Ive been trying for some time therefore in comments or in different adding Listeners the functions differs a bit.

How to make these EventListeners to execute only one time choosing an option in the menu

onst menu1 = document.getElementById("menu1") as HTMLDivElement;
const menu2 = document.getElementById("menu2") as HTMLDivElement;
const menu3 = document.getElementById("menu3") as HTMLDivElement;



menu1.addEventListener("click",  function () {

  let elements = menu1.querySelectorAll("input");
  for(let i=0; i< elements.length; i++) {
      elements[i].addEventListener("click", function () {
        if (elements[i].checked) {
          let val = elements[i].value;
          dataSet.setEffect(elements[i].value);
          console.log(dataSet);
        }
      });
    }
});


menu2.addEventListener("click", function(){

  const elements2 = menu1.querySelectorAll("label");
  // for (let element of elements2) {
  //   element.addEventListener("click", function () {
  //       dataSet.setEffect(element.value);
  //       console.log(dataSet);
  //   });
  // }
  elements2.forEach( function (ele,index) {
      // ele.addEventListener("click", function () {
      //         dataSet.setEffect(ele.innerText);
      //         console.log(dataSet);
      // });
  ele.onclick = function () {
    dataSet.setEffect(ele.innerText);
    console.log(dataSet,ele);
  }
  })

});


menu3.addEventListener("click", ()=>{
  let elements3 = menu3.children;
  for(let i=0; i< elements3.length; i++)
  {
    elements3[i].addEventListener("click", ()=>{
      dataSet.setBGC(elements3[i].id);
      console.log(dataSet);
    });
  }
});

html for a single menu

 <div class="container">
        <div class="header-text">
            <h2>Effect</h2>
        </div>
        <div class="select" id="menu1" tabindex="1">
            <input class="options-select"  value="solidColor" name="selectors1" type="radio" id="opt1" checked>
                <label for="opt1" class="option">Solid Color</label>
            <input class="options-select" value="EQCenter" name="selectors1" type="radio" id="opt2">
                <label for="opt2" class="option">EQ Center</label>
            <input class="options-select" value="Strobe" name="selectors1" type="radio" id="opt3">
                <label for="opt3" class="option">Strobe</label>
            <input class="options-select" value="Sparkel" name="selectors1" type="radio" id="opt4">
                <label for="opt4" class="option">Sparkel</label>
            <input class="options-select" value="swicth" name="selectors1" type="radio" id="opt5">
                <label for="opt5" class="option">Switch</label>
        </div>
        
    </div>
addeventlistener dom html javascript
2 Answers

The problem is one of event propagation (also called "bubbling", as in the linked MDN article). Each time a click on one of the inputs happens, it also triggers the event on each parent element. Since this includes the menu itself, your event handler for that runs again - adding further event listeners to each item.

The simplest code change to fix this is simply to use the built-in stopPropagation method of the event object to prevent this:

  elements[i].addEventListener("click", function (event) {
    event.stopPropagation();
    if (elements[i].checked) {
      let val = elements[i].value;
      dataSet.setEffect(elements[i].value);
      console.log(dataSet);
    }
  });

However, while this should fix the problem, I strongly encourage you to have a think about different ways to achieve your aim. Adding an event listener inside another event listener is basically an anti-pattern - it can easily lead to issues like the one you had here, and I'm struggling to think of any cases where it's needed or useful. Since your inputs are right there in the HTML source, rather than dynamically added with Javascript, I don't see any reason why you couldn't simply add the event listeners to each input directly when the page is loaded. That would lead to cleaner and simpler code.

user_84750540

You wrote:

each time menu1 is clicked, then attach a new onClick listener to each of his children.

menu1.addEventListener("click",  function () {
  let elements = menu1.querySelectorAll("input");
  for(let i=0; i< elements.length; i++) {
      elements[i].addEventListener("click", function () {
        if (elements[i].checked) {
          let val = elements[i].value;
          dataSet.setEffect(elements[i].value);
          console.log(dataSet);
        }
      });
    }
});

Just replace with

let elements = menu1.querySelectorAll("input");
for(let i=0; i< elements.length; i++) {
  let elem = elements[i]; //To avoid the common closure problem
  elem.addEventListener("click", function () {
    if (elem.checked) {
      let val = elem.value;
      dataSet.setEffect(elem.value);
      console.log(dataSet);
    }
   });
}
user_31263230
Related