How to avoid using onclick multiple times

Question

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)" class="${isSelected}">${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)" class="${isSelected}">${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 class="wrapper">
      <div class="select-btn">
        <span>Select Country</span>
        <i class="uil uil-angle-down"></i>
      </div>
      <div class="content">
        <div class="search">
          <i class="uil uil-search"></i>
          <input spellcheck="false" type="text" placeholder="Search">
        </div>
        <ul class="options"></ul>
      </div>
    </div>

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

ncn-ends · Accepted Answer · 2022-12-25 15:09:21Z

2

Using a technique called event delegation, you can apply the event listener only to the parent element instead of each child, and perform a check to make sure that the child was 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 are some articles with more in-depth explanations:

edited Dec 25, 2022 at 15:09

answered Dec 25, 2022 at 8:42

ncn-ends

362 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

Andy Over a year ago

Might be worth adding that this technique is called event delegation.

Sarkar · Accepted Answer · 2022-12-25 08:13:55Z

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 class="${isSelected}">${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)" class="${isSelected}">${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");
  }
});

Sir I'm little confused where to add your code, can you please update my code with your code.
@GupaDutta I've updated the answer tho please make sure it works since I don't tools to double check right now

Collectives™ on Stack Overflow

How to avoid using onclick multiple times

2 Answers 2

1 Comment

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

1 Comment

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related