0
votes

I have a simple JavaScript function that should change the class and therefore the color of 4 buttons when clicked. They do not all change when the "Click me" button is pressed the first time though - only the colours for button1 and button3 change to red. Clicking the "Click me" button again then changes the color of button2 and after another click button4 is updated.

<style>
    .button {
        background-color: green;
    }

    .updatedButton {
        background-color: red;
        color: white;
    }
</style>

<script>
    function changeColor() {
        var elems = document.getElementsByClassName("button");
        for (var i=0; i<elems.length; i++) {
            console.log(JSON.stringify(elems[i].getAttribute('id')));
            elems[i].classList.add('updatedButton');
            elems[i].classList.remove('button');
        }
    }
</script>

<button id="button1" class="button">a</button>
<button id="button2" class="button">b</button>
<button id="button3" class="button">c</button>
<button id="button4" class="button">d</button>
<button onclick="changeColor()">Click me</button>

I logged to the console the ID of the elements that were inside the loop and this is what I got showing that only certain buttons are being updated.

First click: "button1" (15:16:52:545) "button3" (15:16:52:547)

Second click: "button2" (15:16:55:981)

Third click: "button4" (15:16:58:841)

I tried reloading the page but the behaviour is consistent. Could anyone explain why this is happening instead of every button changing color on the first button press please?

4
Why the JSON.stringify(...) call?Andreas
<button onclick="changeColor">Click me</button> if you put parens after changeColor it will run the function without waiting for onclick to triggerNikko Khresna

4 Answers

2
votes

getElementsByClassName returns live collection that is changed immediatelly when you remove the class. Use querySelectorAll instead.

function changeColor() {
  var elems = document.querySelectorAll(".button");
  for (var i = 0; i < elems.length; i++) {
    console.log(JSON.stringify(elems[i].getAttribute('id')));
    elems[i].classList.add('updatedButton');
    elems[i].classList.remove('button');
  }
}
.button {
  background-color: green;
}

.updatedButton {
  background-color: red;
  color: white;
}
<button id="button1" class="button">a</button>
<button id="button2" class="button">b</button>
<button id="button3" class="button">c</button>
<button id="button4" class="button">d</button>
<button onclick="changeColor()">Click me</button>
1
votes

getElementsByClassName() returns a live HTMLCollection, meaning that as soon as you remove the button class, it's no longer in the collection.

To prevent that, create a shallow copy in an array initially:

var elems = [...document.getElementsByClassName("button")];

Depending on what your build environment supports, you can also use Array.from:

var elems = Array.from(document.getElementsByClassName("button"));
0
votes

Try to include all the buttons inside one div and assign a class for it then select all buttons inside this class and change the color for all the selected buttons!

0
votes

Like others have said, you need to use querySelctorAll instead. Check this jsfiddle out.

http://jsfiddle.net/ekcLbwur/11/

    function changeColor() {
        let elems = document.querySelectorAll(".button");
        for (let i=0; i<elems.length; i++) {
            elems[i].classList.add('updatedButton');
            elems[i].classList.remove('button');
        }
    }
window.onload = function() {
    document.getElementById("button5").onclick = changeColor;
};

Also, for reference, it is better to use let instead of var. Furthermore, it is better to alter things like onClick in the JS than HTML.