How to avoid using onclick multiple times-CodePudding

I'm searching for a vanilla JS country code selector from few days and now I have found a perfect code for my project but the problem I'm facing right now is that the code is using onclick() in every option to update the clicked data to select menu, which looks ugly and reduced the speed of code execution.

Instead of using onclick on every li option can I use the 'click' event using addEventListener to update the data. if I can do this can you please explain me how can I do this. actually I'm new in JS so that I need expert guidance.

Please have a look on my JS Snippet. addCountry FUNCTION and Search EVENT

const wrapper = document.querySelector(".wrapper"),
selectBtn = wrapper.querySelector(".select-btn"),
searchInp = wrapper.querySelector("input"),
options = wrapper.querySelector(".options");

let countries = [
                    "<span class='flag-icon flag-icon-afg'></span> Afghanistan ( 93)",
                    "<span class='flag-icon flag-icon-bel'></span> Belgium ( 32)",
                    "<span class='flag-icon flag-icon-chn'></span> China ( 86)",
                ];

function addCountry(selectedCountry) {
    options.innerHTML = "";
    countries.forEach(country => {
        let isSelected = country == selectedCountry ? "selected" : "";
        
// I DONT WANT TO SHOW THIS onclick="updateName(this)" IN EVERY LI OPTION //        
        let li = `<li onclick="updateName(this)" >${country}</li>`;
// I DONT WANT TO SHOW THIS onclick="updateName(this)" IN EVERY LI OPTION // 

        options.insertAdjacentHTML("beforeend", li);
    });
}
addCountry();

function updateName(selectedLi) {
    searchInp.value = "";
    addCountry(selectedLi.innerHTML);
    wrapper.classList.remove("active");
    filterData = /(<span\b[^<>]*><\/span>\s*)\w (?:\s \w )*\s*\((\ [\d-] )\)/g;
    selectBtn.firstElementChild.innerHTML = selectedLi.innerHTML.replace(filterData, `$1$2`);;
}

searchInp.addEventListener("keyup", () => {
    let arr = [];
    let searchWord = searchInp.value.toLowerCase();
    arr = countries.filter(data => {
        return data.toLowerCase().includes(searchWord);
    }).map(data => {
        let isSelected = data == selectBtn.firstElementChild.innerHTML ? "selected" : "";
        
// I DONT WANT TO SHOW THIS onclick="updateName(this)" IN EVERY LI OPTION //         
        return `<li onclick="updateName(this)" >${data}</li>`;
// I DONT WANT TO SHOW THIS onclick="updateName(this)" IN EVERY LI OPTION //         
        
    }).join("");
    options.innerHTML = arr ? arr : `<p style="margin-top: 10px;">Oops! Country not found</p>`;
});

selectBtn.addEventListener("click", () => wrapper.classList.toggle("active"));

document.addEventListener('click', (event)=> {
    if (!wrapper.contains(event.target)) {
        wrapper.classList.remove("active");
    }else{
        wrapper.classList.add("active");
    }
  });

/* Import Google Font - Poppins */
@import url('https://fonts.googleapis.com/css2?family=Poppins:wght@400;500;600;700&display=swap');
*{
  margin: 0;
  padding: 0;
  box-sizing: border-box;
  font-family: 'Poppins', sans-serif;
}
body{
  background: #4285f4;
}
::selection{
  color: #fff;
  background: #4285f4;
}
.wrapper{
  width: 370px;
  margin: 85px auto 0;
}
.select-btn, li{
  display: flex;
  align-items: center;
  cursor: pointer;
}
.select-btn{
  height: 65px;
  padding: 0 20px;
  font-size: 22px;
  background: #fff;
  border-radius: 7px;
  justify-content: space-between;
  box-shadow: 0 10px 25px rgba(0,0,0,0.1);
}
.select-btn i{
  font-size: 31px;
  transition: transform 0.3s linear;
}
.wrapper.active .select-btn i{
  transform: rotate(-180deg);
}
.content{
  display: none;
  padding: 20px;
  margin-top: 15px;
  background: #fff;
  border-radius: 7px;
  box-shadow: 0 10px 25px rgba(0,0,0,0.1);
}
.wrapper.active .content{
  display: block;
}
.content .search{
  position: relative;
}
.search i{
  top: 50%;
  left: 15px;
  color: #999;
  font-size: 20px;
  pointer-events: none;
  transform: translateY(-50%);
  position: absolute;
}
.search input{
  height: 50px;
  width: 100%;
  outline: none;
  font-size: 17px;
  border-radius: 5px;
  padding: 0 20px 0 43px;
  border: 1px solid #B3B3B3;
}
.search input:focus{
  padding-left: 42px;
  border: 2px solid #4285f4;
}
.search input::placeholder{
  color: #bfbfbf;
}
.content .options{
  margin-top: 10px;
  max-height: 250px;
  overflow-y: auto;
  padding-right: 7px;
}
.options::-webkit-scrollbar{
  width: 7px;
}
.options::-webkit-scrollbar-track{
  background: #f1f1f1;
  border-radius: 25px;
}
.options::-webkit-scrollbar-thumb{
  background: #ccc;
  border-radius: 25px;
}
.options::-webkit-scrollbar-thumb:hover{
  background: #b3b3b3;
}
.options li{
  height: 50px;
  padding: 0 13px;
  font-size: 21px;
}
.options li:hover, li.selected{
  border-radius: 5px;
  background: #f2f2f2;
}

<!DOCTYPE html>
<html lang="en" dir="ltr">
  <head>
    <meta charset="utf-8">  
    <title>Custom Select Menu</title>
    <link rel="stylesheet" href="style.css">
    <meta name="viewport" content="width=device-width, initial-scale=1.0">
    <link rel="stylesheet" href="https://amitdutta.co.in/flag/css/flag-icon.css">
  </head>
  <body>
    <div >
      <div >
        <span>Select Country</span>
        <i ></i>
      </div>
      <div >
        <div >
          <i ></i>
          <input spellcheck="false" type="text" placeholder="Search">
        </div>
        <ul ></ul>
      </div>
    </div>

    <script src="script.js"></script>
        
  </body>
</html>

CodePudding user response：

Instead of applying the event listener on each item, you can apply the event listener only to the parent and perform a check to make sure that the children were targeted in the click event.

parent.addEventListener("click", (e) => {
  if (!e.target.classlist.includes("child")) return;
  action();
});

You need some sort of check to make sure only elements you want to be clicked on trigger the action. Otherwise if there is spacing between the children, or the parent has other children that aren't these list items you have, it may create bugs.

In your case it would be like so:

options.addEventListener("click", (e) => {
  if (!e.target.classList.contains("country_btn")) return;
  updateName(e.target);
});

In your case I added a class name to each list item to be used for this check. If you prefer, you can check that the element tag is a li. You can also change the class name to be something else instead.

Here is a CodeSandbox demo of your code improved.

Here is an article with a more in-depth explanation.

CodePudding user response：

you can get all the li's using js then add click listener to them using a for loop, this way ou don't see the onClick event on the li elemnt an the code works as expected

const LiElements = Array.from(document.querySelectorAll(".options li"))

LiElements.forEach(option => {
    option.addEventListener("click", updateName)
})

here is an exapmle in your code

const wrapper = document.querySelector(".wrapper"),
  selectBtn = wrapper.querySelector(".select-btn"),
  searchInp = wrapper.querySelector("input"),
  options = wrapper.querySelector(".options");

  let countries = [
    "<span class='flag-icon flag-icon-afg'></span> Afghanistan ( 93)",
    "<span class='flag-icon flag-icon-bel'></span> Belgium ( 32)",
    "<span class='flag-icon flag-icon-chn'></span> China ( 86)",
  ];


function addCountry(selectedCountry) {
  options.innerHTML = "";
  countries.forEach((country) => {
    let isSelected = country == selectedCountry ? "selected" : "";

    let li = `<li >${country}</li>`;

    options.insertAdjacentHTML("beforeend", li);
  });
  AssignListeners();
}

addCountry();

function AssignListeners(){
    const LiElements = Array.from(document.querySelectorAll(".options li"))

    LiElements.forEach(option => {
        option.addEventListener("click", updateName)
    })
}

function updateName(selectedLi) {
  searchInp.value = "";
  addCountry(selectedLi.innerHTML);
  wrapper.classList.remove("active");
  filterData = /(<span\b[^<>]*><\/span>\s*)\w (?:\s \w )*\s*\((\ [\d-] )\)/g;
  selectBtn.firstElementChild.innerHTML = selectedLi.innerHTML.replace(
    filterData,
    `$1$2`
  );
}

searchInp.addEventListener("keyup", () => {
  let arr = [];
  let searchWord = searchInp.value.toLowerCase();
  arr = countries
    .filter((data) => {
      return data.toLowerCase().includes(searchWord);
    })
    .map((data) => {
      let isSelected =
        data == selectBtn.firstElementChild.innerHTML ? "selected" : "";

      // I DONT WANT TO SHOW THIS onclick="updateName(this)" IN EVERY LI OPTION //
      return `<li onclick="updateName(this)" >${data}</li>`;
      // I DONT WANT TO SHOW THIS onclick="updateName(this)" IN EVERY LI OPTION //
    })
    .join("");
  options.innerHTML = arr
    ? arr
    : `<p style="margin-top: 10px;">Oops! Country not found</p>`;
});

selectBtn.addEventListener("click", () => wrapper.classList.toggle("active"));

document.addEventListener("click", (event) => {
  if (!wrapper.contains(event.target)) {
    wrapper.classList.remove("active");
  } else {
    wrapper.classList.add("active");
  }
});