var state = 0;

function selectLI(element) {
  if (state == 0) {
    element.innerHTML = element.innerHTML   "<span class='liTick'>&#10004;</span>";
    state = 1;
  } else {
    var ele = document.getElementsByClassName('checklistLI');
    for (var i = 0; i < ele.length; i  ) {
      var els = ele[i].getElementsByClassName(".liTick");
      els.item(0).style.display = "none";
    }
  }
}

<ul >
  <li  onclick="selectLI(this)">item 1</li>
  <li  onclick="selectLI(this)">item 2</li>
</ul>

What the code is supposed to do is remove the ticks generated by the code above. But it doesn't work and keep giving me error!

CodePudding user response：

There are two issues making it so your code doesn't hide the tick:

1. You've included a . in the class name when calling getElementsByClassName, but the class name doesn't actually have a dot in it.

2. You're looping through all the li elements, but only some of them have a liTick inside them; you're not allowing for the possibility you didn't find one.

The minimal fix is:

var els = ele[i].getElementsByClassName("liTick");
// No `.` here −−−−−−−−−−−−−−−−−−−−−−−−−−^
if (els[0]) { // <== Make sure there is one
  els[0].style.display = "none";
}

Or in really up-to-date environments, you could use optional chaining:

var els = ele[i].getElementsByClassName("liTick");
els[0]?.style.display = "none";

But, I wouldn't do that, because it's just hiding the tick. The be consistent, if you're adding a tick, you should remove (not just hide) it.

Separately, you have only one state flag, but there are multiple elements. I'm going to assume they should be independently "tick"-able. For that, we want to base our decision on whether the element already has a tick in it.

Also, avoid using innerHTML (and in particular avoid .innerHTML = .innerHTML "x"). Instead, just insert what you need.

Here's an example:

function selectLI(element) {
    const tick = element.querySelector(".liTick");
    if (tick) {
        // Remove the tick
        tick.remove(); // In modern environments
        // Or: tick.parentNode.removeChild(tick); // In older environments
    } else {
        // Add the tick
        element.insertAdjacentHTML(
            "beforeend",
            "<span class='liTick'>&#10004;</span>"
        );
    }
}

<ul >
  <li  onclick="selectLI(this)">item 1</li>
  <li  onclick="selectLI(this)">item 2</li>
</ul>

Or if you wanted only one item to be allowed to have a tick:

function selectLI(element) {
    const tick = element.querySelector(".liTick");
    if (tick) {
        // Remove the tick
        tick.remove(); // In modern environments
        // Or: tick.parentNode.removeChild(tick); // In older environments
    } else {
        // Remove any other tick
        const other = document.querySelector(".checklistLI .liTick");
        if (other) {
            other.remove(); // Or, again, the longer form in old environments
        }
        // Add the tick to this element
        element.insertAdjacentHTML(
            "beforeend",
            "<span class='liTick'>&#10004;</span>"
        );
    }
}

<ul >
  <li  onclick="selectLI(this)">item 1</li>
  <li  onclick="selectLI(this)">item 2</li>
</ul>

---

Note that this is not accessible to people using assistive technologies. Consider using a <input type="checkbox"> instead.

CodePudding user response：

As @Andy Holmes suggested in a comment:

This seems a lot more complicated for what it's actually doing. Why don't you just put the ticks there by default, and when clicking on the relevant li you just toggle a hidden class (which has display: none;) on the tick rather than this seemingly excessive code?

HTML

<ul >
  <li  onclick="selectLI(this)">item 1 <span class='liTick hidden'>&#10004;</span></li>
  <li  onclick="selectLI(this)">item 2 <span class='liTick hidden'>&#10004;</span></li>
  <li  onclick="selectLI(this)">item 3 <span class='liTick hidden'>&#10004;</span></li>
</ul>

CSS

.hidden {
  display: none;
}

JS

function selectLI(element) {
  document.querySelectorAll(".checklistLI").forEach(function (el) {
    el.getElementsByClassName("liTick").item(0).classList.add("hidden");
  });

  element.getElementsByClassName("liTick").item(0).classList.remove("hidden");
}

Demo

checklist

CodePudding user response：

You can use this solution, it's working!

<ul >
    <li  onclick="selectLI(this, 1)">item 1</li>
    <li  onclick="selectLI(this, 2)">item 2</li>
</ul>

<script>
    var state1 = 0;
    var state2 = 0;

    function selectLI(element, flag) {
        if (flag === 1) {
            if (state1 === 0) {
                element.innerHTML = element.innerHTML   "<span class='liTick'>&#10004;</span>";
                state1 = 1;
            } else {
                element.innerHTML = 'item 1';
                state1 = 0;
            }
        } else {
            if (state2 === 0) {
                element.innerHTML = element.innerHTML   "<span class='liTick'>&#10004;</span>";
                state2 = 1;
            } else {
                element.innerHTML = 'item 2';
                state2 = 0;
            }
        }
    }
</script>