changing the classname of the specific Li that is clicked on-CodePudding

This is probably super basic javascript but I can't seem to figure out how to change the class name of the specific li that I am clicking on.

HTML:

var ul = document.querySelector("ul");
var li = ul.document.querySelectorAll("li");

function loopThroughLi() {
  for (i = 0; i < li.length; i  ) {
    li[i].addEventListener("click", function() {
      li[i].classList.toggle("FinishTask");
    })

  }
}

loopThroughLi();

.FinishTask {
  text-decoration: line-through;
}

<!DOCTYPE html>
<html>
<head>
  <link rel="stylesheet" type="text/css" href="style.css">
</head>
<body>
  <ul>
    <li>Notebook</li>
    <li>Carrot</li>
    <li>Spinach</li>
    <li>Rice</li>
  </ul>
  <script type="text/javascript" src="script.js"></script>
</body>
</html>

CodePudding user response：

The issue is that li[i] is undefined by the time the handler runs because i has been incremented all the way up to li.length by then.

You can just save a reference to li[i] to fix the issue.

const ul = document.querySelector("ul");
const li = document.querySelectorAll("li");

function loopThroughLi() {
    for (let i = 0; i < li.length; i  ) {
        const elem = li[i];
        elem.addEventListener("click", function() {
            elem.classList.toggle("FinishTask");
        });
    }
}

loopThroughLi();

.FinishTask{
    text-decoration: line-through;
}

<!DOCTYPE html>
<html>
<head>
    <link rel="stylesheet" type="text/css" href="style.css">
</head>
<body>
    <ul>
        <li>Notebook</li>
        <li>Carrot</li>
        <li>Spinach</li>
        <li>Rice</li>
    </ul>
    <script type="text/javascript" src="script.js"></script>
</body>
</html>

CodePudding user response：

the variable i keeps incrementing until it is past the number of elements in li, so by the time the click event fires, li[i] is undefined. there are a couple ways to handle this. you can use this, instead of li[i] to toggle the class. or you could use let to define the i variable which will change its scope. or you could save li[i] in a separate variable outside of the click event. the first option seems the most simple and performant.

var ul = document.querySelector("ul");
var li = ul.querySelectorAll("li");

function loopThroughLi() {
  for (i = 0; i < li.length; i  ) {
    li[i].addEventListener("click", function() {
      this.classList.toggle("FinishTask");
    })

  }
}

loopThroughLi();

.FinishTask {
  text-decoration: line-through;
}

<!DOCTYPE html>
<html>
<head>
  <link rel="stylesheet" type="text/css" href="style.css">
</head>
<body>
  <ul>
    <li>Notebook</li>
    <li>Carrot</li>
    <li>Spinach</li>
    <li>Rice</li>
  </ul>
  <script type="text/javascript" src="script.js"></script>
</body>
</html>

CodePudding user response：

I did change to use a class for the task list and one for each task item since I find element CSS and script to be fragile to maintain.
Here I integrate through the list of items and set a click event handler.

I also illustrate not only how to set a class by a toggle but also how to un-toggle the class based on a data attribute value - I did the same for the data attribute and illustrate how to use that in the CSS as well, setting up the values on the loop where we added the event handlers to each.

I also used a forEach on every element in the task list as a simpler set of code perhaps.

const taskList = document.querySelector(".task-list");
const taskItems = taskList.querySelectorAll(".task-item");

function setupItems(items) {
  items.forEach(function(item, index, items) {
    //set not done as we initialize
    item.dataset.isdone = "false";
    console.log(index, item);
    item.addEventListener("click", function(element, index, array) {
      const alreadyDone = this.dataset.isdone == "true";
      this.classList.toggle("finish-task", !alreadyDone);
      this.dataset.isdone = alreadyDone ? "false" : "true";
    }, item);
  });
}

setupItems(taskItems);

.finish-task {
  text-decoration: line-through;
}

.task-item[data-isdone="true"] {
  background-color: #ddddff;
}

.task-item[data-isdone="false"] {
  background-color: #ddffdd;
}

<body>
  <ul >
    <li >Notebook</li>
    <li >Carrot</li>
    <li >Spinach</li>
    <li >Rice</li>
  </ul>
</body>