Its this good for best practices ? add item and delete

Hello Campers,

I know this been repetitive in the past, but sometimes we want to do or find the right ways of approaching a problem, i just want a feedback regrading best practices, in this type, of scenarios. Ex: i have an input field that adds items and show a delete button, everythings works ok, my only doubt is that if im doing it right by calling/invoking the removeItems() function the right way at the end of the code.

Note: Sometimes developers will want to have an onclick=“removeItems()” on the plain HTML But im not doing it that way i think will be best to have it clean and simple HTML markup, thats why the my js code was written like that.

function removeItems(){
  var removeBtn = document.getElementsByClassName('remove');

  for( var i = 0; i < removeBtn.length; i++ ){
    var elRemoveBtn = removeBtn[i];
    elRemoveBtn.addEventListener('click', function(){
      var item = this.parentNode;
      var parent = item.parentNode;
      return parent.removeChild(item);
    });
  }
};
removeItems(); // invoking the removeItems function. (is this ok ?)

and then calling it on the addItems function

function addItem(){

  var input = document.getElementById("add");
  var myValue = input.value;
  var container = document.getElementById('faves');
  //container.insertAdjacentHTML('afterbegin', `<li>${myValue}</li>`);
  container.innerHTML += '<li><span>'+myValue+'</span><button class="remove" id="removeBtn">Remove</button></li>';
  input.value = "";

  removeItems();  // calling the remove items function.

    };
document.getElementById('btnAdd').addEventListener('click', addItem);

I just want a feedback for best practices and if there is another way of better code it.
codepen link for better undersanding.

Thanks !

In my opinion, you do not need to add an event listener to each button with class=“remove”. One option is to add a single event listener to the ul element with id=“faves” and then detect if the clicked element is a button with class=“removed”.

function addItem() {
  const myValue = input.value;
  const itemElem = document.createElement('li');
  itemElem.innerHTML = `
    <span class="item-text"></span>
    <button class="remove">Remove</button>
  `;
  itemElem.querySelector('.item-text').textContent = myValue;
  container.appendChild(itemElem);
  input.value = '';
}

function removeItem({ target }) {
  if (target && target.nodeName === "BUTTON" && target.classList.contains("remove")) {
    const item = target.parentNode;
    item.parentNode.removeChild(item);
  }
}

const input = document.getElementById("add");
const container = document.getElementById("faves");
document.getElementById("btnAdd").addEventListener("click", addItem);
container.addEventListener("click", removeItem);

This avoids the issue of having to remove an event listener each time an item is removed. You typically want to remove event listeners that are no longer useful (i.e. deleted).

1 Like

It’s not ok to do that :slight_smile:
You keep assigning new eventListeners to existing buttons that already have event listeners - this is bad

Thank you @camperextraordinaire i will surely do as you mention, that was my only concern maybe i was doing something wrong or good for best practices, thank you so much i will have that in mind as well. This is really helpful, thanks for the feedback. Surely that’s the best way to approach this thanks!.

Thanks! @snigo for your beedback, i will surely have that in mind!.. thannks to many event listeners are not ok if you can have only one. Thanks!.

Hi @camperextraordinaire Oh wow thanks! for letting me know that its good to know that and work with textContent for better script secure page. its good to work with that for me. Also Randell i know i been using ES5 sixtax and not ES6 but i still want to take a bit of practice will good old vanilla js, althought i know i will have to update on const, let, and also template strings to the new back-tick ES6. Thanks for the update Randell. Working hard to be like you :slight_smile:

OK you have this line of code:
itemElem.innerHTML += <span class="item-text"></span><button class="remove">Remove</button>;

when you take away the + and leave the = does the same thing why is that ?

ok so this can be done with += and = and wont have any impact in any then ?.

Ok perfect now i get it… thanks @camperextraordinaire for all your help :slight_smile: