click event listener is working only for 1st list element and other sibling elements it's not

Question

I'm new to JS and trying to achieve same action of adding new class using Event Listener to HTML elements but it's only working for 1st element and none of other 4 is working.

What mistake am I doing here ?

DOM Selectors

let rating_1 = document.getElementsByClassName("user-rating__rating-number-1")[0];
let rating_2 = document.getElementsByClassName("user-rating__rating-number-2")[0];
let rating_3 = document.getElementsByClassName("user-rating__rating-number-3")[0];
let rating_4 = document.getElementsByClassName("user-rating__rating-number-4")[0];
let rating_5 = document.getElementsByClassName("user-rating__rating-number-5")[0];

Working

rating_1.addEventListener("click", () => {

    rating_1.classList.add("user-rating__rating-number--orange");

});

Not Working

rating_2.addEventListener("click", () => {
    //console.log(rating_2.innerText);

    //alert("clicked 2");

    rating_2.classList.add("user-rating__rating-number--orange");

});

EDIT :

Culprit of this strange behavior is

let ratingArray = [rating_1, rating_2, rating_3, rating_4, rating_5];

When I do not use this array at all, I can click on each of the elements and assign new class to it; which is what I want to achieve.

I've commented out removeClassFromElements to avoid any confusion / distraction from main problem
But my question still remains unanswered why array of DOM selectors is not working as expected ? Why only 1st element of this array is able to accept new class name and not the others ?

I'm new to JavaScript coming from Java + Eclipse / IntelliJ world; it's so hard to debug issue with JS + VSCode ( any tips appreciated )

davidhuerta · Answer

If you just want to change the background-color from white to orange of these circles, what you can do is to assign a common class name (I've added the common class test but you can change to whatever you want) in your html code, and then with a forEach loop add the class name. Something like this:

document.querySelectorAll('.test').forEach(item => {
  item.addEventListener('click', event => {
    item.classList.add("user-rating__rating-number--orange");
  })
})

.user-rating__rating-number--orange {
  background-color: orange;
}
.user-rating__rating-number > li {
  border: 1px solid red;
  border-radius: 50%;
  padding: 1rem;
  width: 4rem;
  height: 4rem;
  text-align: center;
  cursor: pointer;
}
.user-rating__rating-number {
  display: flex;
  justify-content: space-between;
  list-style: none;
}

But this might not be good solution, because you might want for your user to select only one number. For example if a user selects 4 as initial feedback, but then he wants to change his rating to 2, both numbers will have an orange background.

To prevent this you can have this in your JavaScript code, so the user can only select one number as rating.

document.querySelectorAll('.test').forEach(item => {
  item.addEventListener('click', event => {
    if (!item.classList.contains("user-rating__rating-number--orange")) {
      const allItems = document.querySelectorAll('.test');

      allItems.forEach(el => {
        if (el.classList.contains("user-rating__rating-number--orange")) {
          el.classList.remove("user-rating__rating-number--orange");
        }
      })
      item.classList.add("user-rating__rating-number--orange");
    }    
  })
})

.user-rating__rating-number--orange {
  background-color: red;
}
.user-rating__rating-number > li {
  border: 1px solid red;
  border-radius: 50%;
  padding: 1rem;
  width: 4rem;
  height: 4rem;
  text-align: center;
  cursor: pointer;
}
.user-rating__rating-number {
  display: flex;
  justify-content: space-between;
  list-style: none;
}

Dorad · Answer

You implemented removeClassFromElements in wrong way. you wanted to exclude current element from removal, but you send the element and not its index, therefore the splicing mechanism you used didn't work. instead, filter the ratingArray comparing the excluded element itself:

const removeClassFromElements = (elementToExclude) => {
    ratingArray.filter(elem => elem != elementToExclude).forEach(element => element.classList.remove("user-rating__rating-number--orange"));
}

original code:

const removeClassFromElements = (elementToExclude) => {
    let newArray = ratingArray.slice();
    newArray.splice(elementToExclude, 1);

    newArray.forEach(element => element.classList.remove("user-rating__rating-number--orange"));
}

click event listener is working only for 1st list element and other sibling elements it's not

2 Answers