Event firing multiple times

Viewed 341

I have a little problem that I replicated in the little code snippet below (in the most simple way possible, however it still shows the problem I am facing).

Here is the snippet :

const searchBar = document.getElementById('search');
const resBox = document.getElementById('results');

searchBar.addEventListener('input', function handler(e) {
    if (e.target.value === '') {
        resBox.innerHTML = '';
        return false;
    }
  
    setTimeout(() => populate(e), 300);
});

function populate(e) {  
    const btnBox = document.createElement('div');
  
    for (let i = 1; i <= 3; i++) {
        const btn = document.createElement('button');
        btn.classList.add('js-click')
        btn.innerText = 'Click me';
   
        btnBox.appendChild(btn);
    }
  
    resBox.appendChild(btnBox);
  
    dynamicBtnClickListener();
}


function dynamicBtnClickListener() {
    resBox.addEventListener('click', function handler(e) {
        console.log('You clicked me !');
    });
  
  // THE SOLUTION I FOUND FOR THE MOMENT :
  
  //const btns = document.querySelectorAll('button.js-click');
  //btns.forEach(btn => {
  //    btn.addEventListener('click', function handler(e) {
  //        console.log('You clicked me !');
  //    });
  //});
}
<input type="text" id="search">
<div id="results"></div>

As you can see in the snippet, I have a first listener on input that generates a list of buttons when you type in it. When it is empty, the buttons disappear. In my real world case, it is a search input, that when a user types in, calls a function that populates a result box below it with results from DB.

I then have an on click listener on the buttons. In the code snippet, I simply put a console('You clicked me') when you click on the buttons. In my real app, it takes the result item (each result is an user) and inserts it in a table.

The problem appears when you open, close, then re-open the results box. This is done by inputing something, clearing the input, then re-input something. When you do that and then click on one of the buttons, it fires the click event on them as many times as you opened / closed the result box, so you will see the "You clicked me" on console multiple times.

I have done some research and most of it calls for using event.stopPropagation() and / or removing the event listener(s). I did try all these possible solutions, in every way I could think of, but I couldn't make it work.

Anyways I found a way around this (the commented portion of the dynamicBtnClickListener() function), but I feel it is not optimal. It consists of getting all the buttons with querySelectorAll(), then loop through them and add the click listener to every one of them, but I do not think it is optimal nor best-practice like. This is why I come here to ask if maybe there is a better solution, possibly one that keeps the click listener on the results box (if that is the most optimal solution. Is it by the way ?).

So even though I found a solution to this problem, could someone please tell me what is the best practice and optimal way of doing this ?

Thank you very much for your help

1 Answers

Each time the you type in the text area, resBox is accessed each time and the actual element resBox gets a new event listener every time(the buttons don't have any specific listener themselves, so I make EACH BUTTON have a specific listener individually)

const searchBar = document.getElementById('search');
const resBox = document.getElementById('results');

searchBar.addEventListener('input', function handler(e) {
    if (e.target.value === '') {
    resBox.innerHTML = '';
    return false;
  }
  
    setTimeout(() => populate(e), 300);
});

function populate(e) {  
  const btnBox = document.createElement('div');
  
  for (let i = 1; i <= 3; i++) {
    const btn = document.createElement('button');
    btn.classList.add('js-click')
    btn.innerText = 'Click me';
    btn.addEventListener('click',function(ev){console.log('You clicked me !')})
    btnBox.appendChild(btn);
  }
  
  resBox.appendChild(btnBox);
}
<input type="text" id="search">
<div id="results"></div>

Now, here is an example that only has one event listener but would completely handle the situation >:D

Technically this should be faster(since one event listener compared to many), but personally I prefer this option because it "feels better" due to one function controlling the whole button layout(which would make it less "rigid")

PS: The speed difference is so insignificant, you can pick and choose(but if a whole chuck ton of buttons, yea this becomes better)

const searchBar = document.getElementById('search');
const resBox = document.getElementById('results');
const randChars=()=>{ //random characters to prove ONE event listener can work for multiple buttons in resBox
  let arr=["a","b","c","d","e"]
  let randIndex=()=>Math.floor(Math.random()*arr.length)||1
  let n=randIndex(); let returnChar=""
  for(let i=0;i<n;i++){returnChar+=arr[randIndex()]}
  return returnChar
}

searchBar.addEventListener('input', function handler(e) {
    if (e.target.value === '') {
    resBox.innerHTML = '';
    return false;
  }
  
    setTimeout(() => populate(e), 300);
});

resBox.addEventListener('click',function(ev){ //single event listener for all buttons
  let currentButton=ev.path[0]
  if(currentButton.tagName!="BUTTON"){return;} //if a button was not clicked
  console.log("Button with text\n'"+currentButton.innerText+"'\nwas clicked")
})

function populate(e) {  
  const btnBox = document.createElement('div');
  
  for (let i = 1; i <= 3; i++) {
    const btn = document.createElement('button');
    btn.classList.add('js-click')
    btn.innerText = 'Click me '+randChars();
    btnBox.appendChild(btn);
  }
  
  resBox.appendChild(btnBox);
}
<input type="text" id="search">
<div id="results"></div>

Related