Toggle multiple css classes on div with javascript

Question

I have a problem to toggle between 4 color classes. I trying to change color everytime this function is used.

function changeBackground() {
    var all = getSelected();

    var blue = document.getElementsByClassName("blue");
    for (var i = 0; i < all.length; i++) {
        all[i].classList.add("green");
        all[i].classList.remove("blue");
    }

    var red = document.getElementsByClassName("red");
    for (var i = 0; i < red.length; i++) {
        all[i].classList.add("blue");
        all[i].classList.remove("red");
    }

    var yellow = document.getElementsByClassName("yellow");
    for (var i = 0; i < yellow.length; i++) {
        all[i].classList.add("red");
        all[i].classList.remove("yellow");
    }

    for (var i = 0; i < all.length; i++) {
        all[i].classList.add("yellow");
        all[i].classList.remove("green");

    }
}

getSelected returns document.getElementsByClassName("selected"); and make sure only divs who are selected do change background.

Html looks like this: <div id="box1" class="box center green size200"></div>

Works well untill it comes to blue->green and the classes won't be removed. How do i solve this?

jsfiddle.net/7t86z9p6/3 click on the box and then press R on keyboard to run the changeBackground function.. — J. Jesper
– J. Jesper, Commented Dec 8, 2017 at 15:21

Mauricio Florez · Accepted Answer · 2017-12-08 15:53:18Z

1

Please check this https://jsfiddle.net/maflorezp/1u3xjxaq/1/

You have some errors walking the elements and you need validate class before change

function changeBackground() {
    var all = getSelected();

    for (var i = 0; i < all.length; i++) {
        var color = all[i].classList;
        if(color.contains("blue")){
          all[i].classList.add("green");
          all[i].classList.remove("blue");
        } else if(color.contains("red")){
          all[i].classList.add("blue");
          all[i].classList.remove("red");
        } else if(color.contains("yellow")){
          all[i].classList.add("red");
          all[i].classList.remove("yellow");
        } else if(color.contains("green")){
          all[i].classList.add("yellow");
          all[i].classList.remove("green");
        }        
    }
}

edited Dec 8, 2017 at 15:53

answered Dec 8, 2017 at 15:38

Mauricio Florez

1,1328 silver badges15 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

Liora Haydont · Accepted Answer · 2017-12-08 16:06:02Z

I see a few issues with your code:

1- You loop on all of the boxes for each color. You should replace

for (var i = 0; i < blue.length; i++) {
    all[i].classList.add("green");
    all[i].classList.remove("blue");
}

by

for (var i = 0; i < blue.length; i++) {
    blue[i].classList.add("green");
    blue[i].classList.remove("blue");
}

2- You should select all your divs before making any modification, to make sure you only select the one that were that color before starting the function.

var blue = document.getElementsByClassName("blue");
var red = document.getElementsByClassName("red");
var yellow = document.getElementsByClassName("yellow");
var green = document.getElementsByClassName("green");

3- You currently use getSelected to get all the selected divs but then you run the code on every element of the document.

I think instead of using 4 loops, you should only create one and check the class for each elemnts of all, it would resolve a lot of you issues. Something like:

function changeBackground() {
    var all = getSelected();

    for (var i = 0; i < all.length; i++) {
        var colorBlue = all[i].classList.contains("blue")
        var colorRed = all[i].classList.contains("red")
        var colorGreen = all[i].classList.contains("greed")
        var colorYellow = all[i].classList.contains("yellow")
        if(colorBlue){
           all[i].classList.add("green");
           all[i].classList.remove("blue"); 
        }
        //check other colors here the same way
    }
}

Liora you need one variable to store the color the first time, because otherwise the color will change in every check

Collectives™ on Stack Overflow

Toggle multiple css classes on div with javascript

2 Answers 2

Comments

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related