Manipulating Items in a <ul> list

I’ve got a list of books in an unordered list, two of the items within which have the class="current" applied to them. The class is being changed to ‘finished’ using the JS below, at which point, both items should be scored out.

var listItems = document.getElementsByClassName('current');

for(var i = 0; i < listItems.length; i++) {
  listItems[i].className = 'finished';

For some reason, only the first item on the list is being crossed out.

I’ve commented out the loop and checked listItems.length in the console, and it reports 2 items as a result of getElementsByClassName(), so that’s right, but for some reason the loop only completes once - any suggestions?

Here’s a Codepen

The listItems are a collection, which updates when the DOM changes.
When you’ve changed the class of one DOM element, the length of listItems reduces accordingly.

The solution here is to run through the list backwards.

Also, setting the className endangers any other class that might in the future be on that element.
You can use the classList methods to protect you from future bugs there.

1 Like

Yes, this works well

for(var i = listItems.length - 1; i >= 0; i--) {

So if changing to this


and it was wanted to have the background color removed,

  background-color: inherit;
<ul class="list">
  <li class="current finished">The Wind Up Bird Chronicle</li>
  <li class="current finished">A Visit From the Goon Squad</li>
  <li id="next">The Flamethrowers</li>
  <li>To Kill a Mockingbird</li>
  <li>Kafka on the Shore</li>

And you can remove the old one with:

1 Like

Looking at some of the results I’d got by testing assorted values in the console, that’s what I suspected was going on, though I’d not managed to pin down exactly what the mechanism for that was.

Obvious when you think about it.

Unfortunately, I’m obliged to use getElementsByClassName on this occasion, but I will investigate that one.

It does indeed. Thanks both :slight_smile:

There’s no problem with getElementsByClassName, the issue is with setting the className value.

When you have elements with several classes on it, you run in to trouble with className. They don’t have to be classes that you’ve put on there either. Some libraries add their own helpful classes such as first and last, and odd and even.

<ul class="list">
  <li class="current first odd">The Wind Up Bird Chronicle</li>
  <li class="current even">A Visit From the Goon Squad</li>
  <li class="odd" id="next">The Flamethrowers</li>
  <li class="even">To Kill a Mockingbird</li>
  <li class="last odd">Kafka on the Shore</li>

Using className will destroy all of those classes, replacing them all only with what you assign to className. That can cause big problems later on down the line.

A good solution to that is to remove only the class that you intend, and to add only what you need.

There are some class handling functions that give you addClass() and removeClass() functions, but if you can, the classList collection provides a better way to do that.

In your case, it would be to remove the current class attribute, and add the finished one instead.


You could even use a well-named function to express that more clearly:

function replaceClass(el, oldClass, newClass) {
    if (el.classList.contains(oldClass) {
for (var i = listItems.length - 1; i <= 0; i -= 1) {
    replaceClass(listItems[i], 'current', 'finished');
1 Like

Thanks Paul, that’s a very clear explanation. It’s a few steps on from where I am currently, but makes for a perfectly readable solution.

A little further on in same the exercise, there’s a task to add a class to all of the <li> tags in the list - you can see in the loop how it was addressed. Whilst it works, it’s nowhere near as re-useable as your own solution.

var chillButton = document.getElementById('chill');

var coolButton = function() {
  var allListItems = document.getElementsByTagName( 'li' );
  for(var j = 0; j < allListItems.length; j++) {
  allListItems[j].className += ' cool';

chillButton.addEventListener('click', coolButton);

This topic was automatically closed 91 days after the last reply. New replies are no longer allowed.