8

I have a list like so:

 <ul>
   <li class="item">Item 1</li>
   <li class="item">Item 2</li>
   <li class="item">Item 3</li>
 </ul>

And I need to add the class active to each item individually and remove it on the previous item on click.

Something similar to this question Vanilla JS remove class from previous selection however I need to use For loop instead of ForEach due to old browser compatibility.

I've been trying to adapt that answer to my example:

const items = document.querySelectorAll(".item");

for (let i = 0; i < items.length; i++) {
  const item = items[i];
  item.addEventListener("click", addActiveClass);

  for (let i = 0; i < items.length; i++) {
    const item = items[i];
    item.addEventListener("click", removeClass);
  }
}

function removeClass(e) {
  e.target.classList.remove("active");
}

function addActiveClass(e) {
  e.target.classList.add("active");
}

But it's still not working as expected :(

ggorlen
  • 44,755
  • 7
  • 76
  • 106
Nesta
  • 117
  • 10
  • initially all the items will be active or inactive? – Shivanshu Gupta Jun 24 '20 at 15:09
  • 1
    You are adding both a click listener for removing a class and adding a class to the same elements. When a click happens its going to fire off both of those listeners effectively adding the class and then removing right after. Not only that you have a for loop inside your for loop this is going to add multiple event listeners to the same elements over and over. The answer you linked to the inner forEach was just to remove the class not add a click listener that then removes the class – Patrick Evans Jun 24 '20 at 15:09
  • @ShivanshuGupta inactive – Nesta Jun 24 '20 at 15:15

7 Answers7

7

I assume you need to remove the active class from the rest of the items and then update the correct item. However, I'm not sure that classList is available in the older browsers you seek to support. You should look into the polyfill for older bowsers.

const items = document.querySelectorAll(".item");

for (let i = 0; i < items.length; i++) {
  const item = items[i];
  item.addEventListener("click", changeActiveClass);
}

function changeActiveClass(e)
{
  for (let i = 0; i < items.length; i++) {
    const item = items[i];
    item.classList.remove('active');
  }
  e.target.classList.add('active');
}
Matt Ellen
  • 11,268
  • 4
  • 68
  • 90
7

Your current approach adds multiple click handlers to the element which fire sequentially in order, removing and adding the class multiple times and with unpredictable results due to the nested loop.

Existing answers use an O(n) solution that involves looping over all of the elements in the list, but there's no need for this. Simply keep a reference to the last selected element and remove the .active class from that element if it's defined:

const list = [...document.querySelectorAll("ul li")];
let selectedEl;

for (const el of list) {
  el.addEventListener("click", e => {
    selectedEl && selectedEl.classList.remove("active");
    selectedEl = e.target;
    e.target.classList.add("active");
  });
}
.active {
  background: yellow;
}
<ul>
  <li class="item">Item 1</li>
  <li class="item">Item 2</li>
  <li class="item">Item 3</li>
</ul>

An alternate approach is to set a tabindex attribute to each element, then add listeners for focus and blur (or focusin/focusout if you want bubbling) which may more accurately capture the selection/deselection semantics you're trying to achieve. Moving focus to anywhere will deselect the list item, unlike the click event approach shown above.

for (const el of [...document.querySelectorAll("ul li")]) {
  el.addEventListener("focus", e => e.target.classList.add("active"));
  el.addEventListener("blur", e => e.target.classList.remove("active"));
}
.active {
  background: yellow;
  outline: none;
}
<ul>
  <li class="item" tabindex="-1">Item 1</li>
  <li class="item" tabindex="-1">Item 2</li>
  <li class="item" tabindex="-1">Item 3</li>
</ul>

If you're looking to support legacy browsers, you could try (untested):

var list = document.getElementsByTagName("li");
var selectedEl;

for (var i = 0; i < list.length; i++) {
  list[i].addEventListener("click", function (e) {
    if (selectedEl) {
      selectedEl.className = selectedEl.className
        .split(/\s+/)
        .filter(function (e) { e !== "active"; })
        .join(" ");
    }

    selectedEl = e.target;
    e.target.className += " active";
  });
}
.active {
  background: yellow;
}
<ul>
  <li class="item">Item 1</li>
  <li class="item">Item 2</li>
  <li class="item">Item 3</li>
</ul>

or

var list = document.getElementsByTagName("li");

for (var i = 0; i < list.length; i++) {
  list[i].addEventListener("focusin", function (e) {
    e.target.className += " active";
  });
  list[i].addEventListener("focusout", function (e) {
    e.target.className = e.target.className
      .split(/\s+/)
      .filter(function (e) { e !== "active"; })
      .join(" ");
  });
}
.active {
  background: yellow;
  outline: none;
}
<ul>
  <li class="item" tabindex="-1">Item 1</li>
  <li class="item" tabindex="-1">Item 2</li>
  <li class="item" tabindex="-1">Item 3</li>
</ul>
ggorlen
  • 44,755
  • 7
  • 76
  • 106
4

You'll need to change the for-loop a little:

for (let i = 0; i < items.length; i++) {
  const clickedItem = items[i];
 
  // Add an event listener to each item
  clickedItem.addEventListener("click", function() {

    // Remove class from all items
    for (let j = 0; j < items.length; j++) {
      const item = items[j];
      item.classList.remove("active");
    }

    // Add class to the clicked item
    clickedItem.classList.add("active");

  });
}

Note that you could also use an arrow function, but if you're going for backwards compatibility, it's best to use function.

4

This looks like a fine candidate for event delegation. Code could be rewritten to something like

document.addEventListener("click", addActive);

function addActive(evt) {
  if (evt.target.classList.contains("item")) {
    // remove .active from all li.active
    const allItems = document.querySelectorAll(".active");
    for (let i=0; i<allItems.length; i += 1) {
      allItems[i].classList.toggle("active");
    }
    // add .active to the current (clicked) item
    evt.target.classList.toggle("active");
  }
}
li.item {
  cursor: pointer;
}
.active {
  color: red;
}
<ul>
   <li class="item">Item 1</li>
   <li class="item">Item 2</li>
   <li class="item">Item 3</li>
 </ul>
KooiInc
  • 119,216
  • 31
  • 141
  • 177
3

you could try something like this:

const items = document.querySelectorAll(".item");

for (let i = 0; i < items.length; i++) {
    const item = items[i];
    item.addEventListener("click", toggleActive);
}

function cleanActiveClasses() {
    for (let i = 0; i < items.length; i++) {
        const item = items[i];
        item.className = item.className.replace(/(?:^|\s)active(?!\S)/g , '')
    }
}

function toggleActive(e) {
    cleanActiveClasses();
    e.target.className += " active";
}
.active {
  color: red;
}
<ul>
   <li class="item">Item 1</li>
   <li class="item">Item 2</li>
   <li class="item">Item 3</li>
 </ul>

Added some css just to give you a visual feedback.

In a nutshell: just one listener on the DOM element, that cleans every element before assigning the new class.

Hope it helps.

EDIT: Since you requested this for backwards compatibility, old IE versions don't support classList. Edited a new version using className and a regex to remove the active class.

3

This will add / remove classes as needed. Additionally, it is always good to check and make sure the element you are looking for .item is actually on the page.

const items = document.querySelectorAll(".item");

for (let i = 0; i < items.length; i++) {
  const item = items[i];

  item.addEventListener("click", () => {
    for (let i = 0; i < items.length; i++) {
      const item = items[i];
      item.classList.remove("active");
    }

    item.classList.add("active");
  });
}
.item:hover {
  cursor: pointer;
}
.active {
  color: blue;
}
<div>
  <ul>
     <li class="item">Item 1</li>
     <li class="item">Item 2</li>
     <li class="item">Item 3</li>
   </ul>
 </div>
nihiser
  • 604
  • 1
  • 15
  • 28
  • 1
    @ggorlen I was unaware of qSA returning 0, good to know! Will update to remove active from other elements. – nihiser Jun 24 '20 at 15:45
2

You just need to do this in one handler on click event.

const items = document.querySelectorAll(".item");

for (let i = 0; i < items.length; i++) {
  const item = items[i];
  item.addEventListener("click", (e) => toggleActiveClass(e, items));
}

function toggleActiveClass(e) {
  for (let i = 0; i < items.length; i++) {
    const item = items[i];
    item.classList.remove("active");
  }
  e.target.classList.add("active");
}
.item.active {
  color: red;
}
<ul>
   <li class="item">Item 1</li>
   <li class="item">Item 2</li>
   <li class="item">Item 3</li>
 </ul>