If statement not working with color code

Question

The code that I have says: whenever the 'a' key is pressed, the background changes. This doesn't happen, and I think it is because of the color code inside of the if statement.

var colors = ['#ce0e0e', '#079b0c', '#3e3fd6']; //red, green, blue

function changeBackground(){
   document.write("use 'a' key to change background");
   var colorAtRandom = colors[Math.floor(Math.random() * colors.length)];
   document.body.style.backgroundColor = colorAtRandom;
   document.getElementById('button').className = 'hidden'
}

window.addEventListener('keydown', checkKey, false);

function checkKey(key){
   if (key.keyCode == 65){
      if (colorAtRandom != '#ce0e0e'){
         changeBackground();
      } else {
         alert('background is red');
      }
   }
}

.hidden {
    display:none;
}
.show {
    display:block;
}

<input id=button type=button value='change backgound color' onclick='changeBackground()'>

Note by the editor: The original code had the script wrapped in a <script> tag inside <head> with no load event listener. I couldn't reproduce that for the snippet. To see the original code please refer to the revisions.

why would you do that edit @ibrahim mahir It makes copy/paste for testing way more troublesome. — JanWillem Huising, Aug 24 '18 at 22:07
You shouldn't use `document.write()` in code that runs after the page has loaded. See https://stackoverflow.com/questions/802854/why-is-document-write-considered-a-bad-practice — Barmar, Aug 24 '18 at 22:12
@JanWillemHuising The fact that the code is wrapped in the head section doesn't affect anything (the edited code and the original are equivalent). I just put the note there for answerers that may suggest to OP that it is a bad idea to put code there without a load event listener. — ibrahim mahrir, Aug 24 '18 at 22:20

score 2 · Answer 1 · edited Aug 24 '18 at 23:02

first don't use document.write, secund make your variable colorAtRandom global.

var colors = ['#ce0e0e', '#079b0c', '#3e3fd6']; //red, green, blue
var colorAtRandom;
    function changeBackground(){
  //    document.write("use 'a' key to change background");
      colorAtRandom = colors[Math.floor(Math.random() * colors.length)];
      document.body.style.backgroundColor = colorAtRandom;
      document.getElementById('button').className = 'hidden'
    }

    window.addEventListener('keydown', checkKey,false);
    function checkKey(key){
    console.log(key.keyCode);
      if (key.keyCode == 65){
        if (colorAtRandom != '#ce0e0e'){
          changeBackground();
        } else {
          console.log('background is red');
        }
      }
    }

  .hidden {
    display:none;
  }
  .show {
    display:block;
  }

<input id="button" type="button" value='change backgound color' onclick='changeBackground()'>

JanWillem Huising · Accepted Answer · 2018-08-24T22:40:13.530

The problems lies within the scoping. the variable colorAtRandom cant be found within the function checkKey(). make this global and it will work.

The second problem whas that the event listener was not working. i changed it so it will be added wenn the button is clikced. Because document.write creates a new document you will lose the eventlistener you created at loading. therefore it is not advised.

this should fix it.

<!DOCTYPE html>
<html>
<head>
    <meta charset="utf-8"/>
    <title>Tester</title>
    <script type="text/javascript">
    var colors = ['#ce0e0e', '#079b0c', '#3e3fd6']; //red, green, blue
 
 var colorAtRandom = colors[Math.floor(Math.random() * colors.length)]; // this variable has to be global.
 
    function changeBackground(){
      //document.write("use 'a' key to change background");  using document.write is not advised. 
      colorAtRandom = colors[Math.floor(Math.random() * colors.length)]; // chose new random color. made it global to get rid of scope
      document.body.style.backgroundColor = colorAtRandom;
      document.getElementById('button').className = 'hidden';
   window.addEventListener('keydown', checkKey, false); // add the event listener when button is pressed. 
    }
    function checkKey(key){
 console.log("test");
      if (key.keyCode == 65){
        if (colorAtRandom != '#ce0e0e'){
          changeBackground();
        } else {
          alert('background is red');
    changeBackground();  // change background after alert so it doesnt get stuck.
        }
      }
    } 
</script>
<style>
  .hidden {
    display:none;
  }
  .show {
    display:block;
  }
</style>
</head>
<body>
<input id=button type=button value='change backgound color' onclick='changeBackground()'>
</body>
</html>

If statement not working with color code

2 Answers2