Loop changing background color with switch in javascript

Question

So I've been trying to make this switch, it's simple in my head. stating a variable "num" equaling 1 to begin with, setting an "if" or "while" statement for when "num" is equal to 2, the background color of a certain div will change from white to yellow and back in .5 second intervals. My problem I THINK is that I can't seem to change the global variable. I've changed it to 2, and the loop worked, but when it's 1 and I try to use the button to change to 2, nothing happnes! Here's the JS I have so far:

var num = 1;
document.getElementById("numberDisplay").innerHTML = num;
if (num == 2) {
  var flash = setInterval(timer, 1000);
  function timer() {
    document.getElementById("colorChanger").style.backgroundColor = "yellow";
    setTimeout(white, 500);
    function white() {
      document.getElementById("colorChanger").style.backgroundColor = "white";
    }
  }
} else {
  document.getElementById("colorChanger").style.backgroundColor = "white";
  document.getElementById("numberDisplay").innerHTML = num;
}
document.getElementById("buttonOne").onclick=function(){num = 1;}
document.getElementById("buttonTwo").onclick=function(){num = 2;}

edit: 1/30/2018 12:25pm Really appreciate the help guys! A friend of mine actually gave me some advice, that was using window.onload and event timers! Here's what I came up with:

   var num = 1;

   window.onload = function(){
    var button1 = document.getElementById("buttonOne")
    var button2 = document.getElementById("buttonTwo")
    button1.addEventListener("click",function(){
        num = 2;
    })
    button2.addEventListener("click",function(){
        num = 1;
    })
    setInterval(checkNum, 1000);
   }

   function checkNum() {
    if (num == 2) {
            document.getElementById("buttonOne").style.backgroundColor = "#ccccb3";
            document.getElementById("buttonTwo").style.backgroundColor = "white";
            document.getElementById("lightChanger").style.backgroundColor = "yellow";
            setTimeout(grey, 500);
            function grey() {
            document.getElementById("lightChanger").style.backgroundColor = "grey";
                }
    } else {
        document.getElementById("lightChanger").style.backgroundColor = "grey";
        document.getElementById("buttonOne").style.backgroundColor = "white";
        document.getElementById("buttonTwo").style.backgroundColor = "#ccccb3";
    }
}

@Sidney regarding #2, isn't it called with `setInterval`? @TylerLancaster - you should also call `clearInterval` sometime — Adelin, Jan 30 '18 at 05:48
If you fix your indentation things will be much clearer both for you and for any who would help you. — tremby, Jan 30 '18 at 05:48
I think you need to have that `if` condition inside the `timer` function — Aswin Ramesh, Jan 30 '18 at 05:51
@Adelin good point. I looked at the code at least three times but missed it every time. That's what I get for not using Ctrl + F. Deleted my previous comment. — Sidney, Jan 30 '18 at 06:07

Aswin Ramesh · Answer 1 · 2018-01-30T06:27:20.633

Instead of setInterval, you can use the timer function directly inside click handler, and your timer function like,

var colorChanger = document.getElementById("colorChanger");
function white() {
  colorChanger.style.backgroundColor = "white";
}

function timer(n) {
  num = n;
  if (num == 2) {
    colorChanger.style.backgroundColor = "yellow";
    setTimeout(white, 500);
    return;
  }
  colorChanger.style.backgroundColor = "white";
}

try this,

var num = 1;
var colorChanger = document.getElementById("colorChanger");


function white() {
  colorChanger.style.backgroundColor = "white";
}

function timer(n) {
  num = n;
  if (num == 2) {
    colorChanger.style.backgroundColor = "yellow";
    setTimeout(white, 500);
    return;
  }
  colorChanger.style.backgroundColor = "white";
}

document.getElementById("buttonOne").onclick = function() {
  timer(1);
}
document.getElementById("buttonTwo").onclick = function() {
  timer(2);
}

<div id="colorChanger">switch</div>


<button id='buttonOne'>1</button>
<button id='buttonTwo'>2</button>

As an optimization, you can create a function that takes color and sets it to element. This would reduce code a bit — Rajesh, Jan 30 '18 at 06:02

score 0 · Answer 2 · edited Jun 20 '20 at 09:12

but when it's 1 and I try to use the button to change to 2, nothing happnes!

Because, on click, you are changing a variable. This will not trigger anything unless it is observable. You will have to wrap related code a function and call it instead.

function timer(n) {
  function updateBackground(color) {
    document.getElementById("colorChanger").style.backgroundColor = "yellow";
  }
  var colors = ['white', 'yellow'];
  updateBackground(colors[n - 1]);
  if (n === 2) {
    setTimeout(function() {
      updateBackground('white')
    }, 500);
  }
}

document.getElementById("buttonOne").onclick = function() {
  timer(1);
}
document.getElementById("buttonTwo").onclick = function() {
  timer(2);
}

<div id="colorChanger">switch</div>


<button id='buttonOne'>1</button>
<button id='buttonTwo'>2</button>

Few improvements,

Any variable that is not in a function becomes a part of global scope. You should avoid it.
I would suggest avoiding .onclick. If I fetch the element and replace it with my function, it will break your code. Always prefer using .addEventListener. This is opinionated though.
You should use classes instead of setting individual style. It keeps code clean and maintainable.

function timer(n) {
  if (n === 2) {
    document.getElementById("colorChanger").classList.add("yellow");
    setTimeout(function() {
      document.getElementById("colorChanger").classList.remove("yellow");
    }, 500);
  }
}

document.getElementById("buttonOne").onclick = function() {
  timer(1);
}
document.getElementById("buttonTwo").onclick = function() {
  timer(2);
}

.white {
  background: white;
}

.yellow {
  background: yellow;
  /* This is for demo purpose only */
  font-weight: bold;
}

<div id="colorChanger" class='white'>switch</div>


<button id='buttonOne'>1</button>
<button id='buttonTwo'>2</button>

References:

score 0 · Accepted Answer · answered Jan 30 '18 at 07:27

I changed something:

(function(){
 var changeColor = function(num) {
  document.getElementById("numberDisplay").innerHTML = num;
  if (num === 2) {
     var flash = setInterval(timer, 1000);
     function timer() {
       document.getElementById("colorChanger").style.backgroundColor = "yellow";
       setTimeout(white, 500);
       function white() {
          document.getElementById("colorChanger").style.backgroundColor = "white";
       }
     } 
    } else {
     document.getElementById("colorChanger").style.backgroundColor = "white";
     document.getElementById("numberDisplay").innerHTML = num;
  }
 }
 
 document.getElementById("buttonOne").addEventListener('click', function(evt){
  let num = 1;
  changeColor(num);
 });
 document.getElementById("buttonTwo").addEventListener('click', function(evt){
  let num = 2;
  changeColor(num);
 });
})();

<div id="numberDisplay"></div>
<div id="colorChanger">Color</div>
<button id="buttonOne">1</button>
<button id="buttonTwo">2</button>

Loop changing background color with switch in javascript

3 Answers3

References: