-1

i want to know why i cant use li element in function. i.e. i have tried this command li.classList.toggle("done") but it shows Uncaught TypeError: Cannot read property 'toggle' of undefined"

var li = document.getElementsByTagName("li");
for(var i=0; i<li.length; i++)
{
 li[i].addEventListener("click", function()
 {
  li.classList.toggle("done");
 });
}
.done {
  text-decoration: line-through;
}
<!DOCTYPE html>
<html>
<head>
 <title>Javascript + DOM</title>
 <link rel="stylesheet" type="text/css" href="style.css">
</head>
<body>
 <h1>Shopping List</h1>
 <p id="first">Get it done today</p>
 <input id="userinput" type="text" placeholder="enter items">
 <button id="enter">Enter</button>
 <ul>
  <li class="bold red" random="23">Notebook</li>
  <li>Jello</li>
  <li>Spinach</li>
  <li>Rice</li>
  <li>Birthday Cake</li>
  <li>Candles</li>
 </ul>
 <script type="text/javascript" src="script.js"></script>
</body>
</html>
pravin deva
  • 69
  • 2
  • 8

2 Answers2

1

That's becuase li referes to the HTMLCollection of <li> elements, not an individual <li> element itself. You need to:

  1. use li[i].classList and
  2. use let in the for loop, due to JS closure

So you have something like this, assuming that you only support browsers that supports ES6:

const li = document.getElementsByTagName("li");
for (let i = 0; i < li.length; i++) {
  const el = li[i];
  el.addEventListener("click", () => {
    el.classList.toggle("done");
  });
}

Otherwise, you will need to use the forEach function instead. Since getElementsByTagName does not return an iterator, you are better off using querySelectorAll, which there is a forEach method available:

var li = document.querySelectorAll('li');
li.forEach(function(el) {
  el.addEventListener("click", function () {
    el.classList.toggle("done");
  });
});

See proof-of-concept example:

const li = document.getElementsByTagName("li");
for (let i = 0; i < li.length; i++) {
  li[i].addEventListener("click", function() {
    li[i].classList.toggle("done");
  });
}
.done {
  text-decoration: line-through;
}
<h1>Shopping List</h1>
<p id="first">Get it done today</p>
<input id="userinput" type="text" placeholder="enter items">
<button id="enter">Enter</button>
<ul>
  <li class="bold red" random="23">Notebook</li>
  <li>Jello</li>
  <li>Spinach</li>
  <li>Rice</li>
  <li>Birthday Cake</li>
  <li>Candles</li>
</ul>
Terry
  • 63,248
  • 15
  • 96
  • 118
0

The issue is that inside of the anonymous function you are out of scope from from your original for loop because the code in the function runs on a click even that is unrelated to the loop that created it.

You might observe that li.classList should have been li[i].classList but this won't help.

There are a couple of ways to resolve this but the most reliable way I believe is to use the event callback data and use the target property.

const li = document.getElementsByTagName('li');

for(let i=0; i<li.length; i++)
{
 li[i].addEventListener("click", function(event) {

  event.target.classList.toggle('done');

 });
}
.done {
  text-decoration: line-through;
}
<!DOCTYPE html>
<html>
<head>
 <title>Javascript + DOM</title>
 <link rel="stylesheet" type="text/css" href="style.css">
</head>
<body>
 <h1>Shopping List</h1>
 <p id="first">Get it done today</p>
 <input id="userinput" type="text" placeholder="enter items">
 <button id="enter">Enter</button>
 <ul>
  <li class="bold red" random="23">Notebook</li>
  <li>Jello</li>
  <li>Spinach</li>
  <li>Rice</li>
  <li><span>Birthday Cake</span></li>
  <li><div>Candles</div></li>
 </ul>
 <script type="text/javascript" src="script.js"></script>
</body>
</html>

I wrapped added a couple of child elements to some list items to demonstrate that target is the li and not the item clicked inside.

Marc
  • 5,109
  • 2
  • 32
  • 41