CODEWITHSUNDEEP
javascripthtmlclickaddeventlistener
user13101974
user13101974

Reputation:

Why click event listener just once working

I want the function to be executed when any li element is clicked. How can I do that?

var listItem = document.getElementById("listItem");

listItem.addEventListener("click", clickTodo)

function clickTodo() {
  listItem.style.color = "red"
}
ul li {
cursor: pointer;
}
<ul>
  <li id="listItem">One</li>
  <li id="listItem">Two</li>
  <li id="listItem">Other</li>
</ul>

Upvotes: 0

Views: 131

Answers (4)

mplungjan
mplungjan

Reputation: 177830

IDs need to be unique and eventListener on one element works on one element only

It is recommended to delegate from nearest static container

Added benefit, you can add new TODOs and the click still works

I also suggest you use CSS and toggle the class

var list = document.getElementById("list");

list.addEventListener("click", clickTodo)

function clickTodo(e) {
  const tgt = e.target;
  if (tgt.classList.contains("listItem")) tgt.classList.toggle("red");
}
ul li {
  cursor: pointer;
}

.red {
  color: red;
}
<ul id="list">
  <li class="listItem">One</li>
  <li class="listItem">Two</li>
  <li class="listItem">Other</li>
</ul>

Upvotes: 1

Florent Arlandis
Florent Arlandis

Reputation: 925

And id is supposed to be unique and used on a single element. If you want to select multiple elements you should use a class instead. But in that case you can just select elements directly:

document.querySelectorAll('li').forEach(li => 
    li.addEventListener('click', clickTodo));

function clickTodo() {
  this.style.color = "red"
}

Upvotes: 0

Majed Badawi
Majed Badawi

Reputation: 28414

listItem should be a class instead of an id to group all your li elements. Then, add the event listener to each item using a forEach as follows:

const listItems = document.getElementsByClassName("listItem");

[...listItems].forEach(listItem => 
  listItem.addEventListener("click", clickTodo) 
);

function clickTodo() {
  this.style.color = "red"
}
ul li {
cursor: pointer;
}
<ul>
  <li class="listItem">One</li>
  <li class="listItem">Two</li>
  <li class="listItem">Other</li>
</ul>

Upvotes: 0

Unmitigated
Unmitigated

Reputation: 89224

You shouldn't have multiple elements with the same id. You should use a class instead.

You can then select all the elements using document.querySelectorAll and add event listeners to each of them.

document.querySelectorAll('ul > li.listItem').forEach(li => 
    li.addEventListener('click', clickTodo));

function clickTodo() {
  this.style.color = "red"
}
ul li {
cursor: pointer;
}
<ul>
  <li class="listItem">One</li>
  <li class="listItem">Two</li>
  <li class="listItem">Other</li>
</ul>

Upvotes: 0

Related Questions