Cannot read property 'value' of null at index.html:15 in color

Question

    <html>
        <head>
            <title>Website</title>
        </head>
        <style>
            #square{
                width: 50;
                height: 50;
                background-color: red;
            }
        </style>
        <body id="body">
            <script>
                var picker = document.getElementById("square")
                var color = document.getElementById("colorBox").value
                function onClick(){
                    document.getElementById("body").style.backgroundColor = color
                }
            </script>
            <input type="color" id="colorBox"/><br />
            <button onclick="onClick();">Change</button>
            <div id="square"></div>
       </body>

It produces the error which is the title. There is no issues with layout. Just the 15 lines. Look, I don't know what to put here.

select the color element in your `onClick` handler. – Daniel A. White Aug 08 '20 at 01:07 — Daniel A. White, Aug 08 '20 at 01:07
^ or move your script to the bottom of the page. – Adrian Aug 08 '20 at 01:07 — Adrian, Aug 08 '20 at 01:07

score 1 · Answer 1 · answered Aug 08 '20 at 01:08

It's because your element with ID colorBox is defined after the script. This means that when the script runs, it cannot find the element. If you move the script tag below your element definition, your code will run properly.

Note, I think another issue with your code is that you compute the value of color before your onClick function, so it will always set the background color to black when you click the button. If you move the color definition to inside of the function, it will be recomputed every time you click the button, giving what I believe is the desired result:

<html>

<head>
  <title>Website</title>
</head>
<style>
  #square {
    width: 50;
    height: 50;
    background-color: red;
  }
</style>

<body id="body">

  <input type="color" id="colorBox" /><br />
  <button onclick="onClick();">Change</button>
  <div id="square"></div>

  <script>
    var picker = document.getElementById("square")

    function onClick() {
      var color = document.getElementById("colorBox").value
      document.getElementById("body").style.backgroundColor = color
    }
  </script>
</body>

There's no reason to set a variable to a value if you are only going to use that value one time - - it's just extra code. Also, you should not scan the DOM for the same element over and over in a function that might be called many times. Get the references just once. — Scott Marcus, Aug 08 '20 at 01:32

Scott Marcus · Accepted Answer · 2020-08-08T01:26:24.467

Your script is scanning the document for your colorbox element before it has encountered that HTML. Move the script to just before the closing body to only run it after all the HTML has been parsed.

And while this will solve your most immediate problem, you'll find that there are still other problems, so see my second example for that solution.

<body id="body">
  <input type="color" id="colorBox"/><br />
  <button onclick="onClick();">Change</button>
  <div id="square"></div>

  <script>
    var picker = document.getElementById("square")
    var color = document.getElementById("colorBox").value
    function onClick(){
      document.getElementById("body").style.backgroundColor = color
    }
  </script>
</body>

Now, with that fixed, there are several other items that need attention:

Nothing is allowed to go after the closing head tag and before the opening body tag. The style element should go inside of the head.
In CSS, most of the time you must place a unit of measurement after an amount, so your height and width of 50 won't work unless you add something like px, %, em, etc.
Don't set your variables equal to properties of elements. Set variables to the elements themselves. Your code gets the value of the input before the user has selected a color, you need to set the color after they've chosen. By only getting the element reference up front, you can then easily get the current value of that element at just the time you need it.
There is no need to give body an id so that you can reference it later. A document will only ever have one body and it is accessible via document.body.
Don't use HTML event attributes (like onclick) to set up your events. Instead, do your event handling separately, in JavaScript.
Don't use self-terminating XHTML syntax. It buys you nothing and opens the door to bugs.
Lastly, get your element references just once, not inside of your event handler because every time that function runs, it will scan the document again for the same element it already found earlier.

So here's your code again, with these tips applied:

<!doctype html>
<html>
<head>
  <title>Website</title>
  <style>
    #square{
       width: 50px;   /* In CSS, you must also put a unit next to an amount */
       height: 50px;
       background-color: red;
    }
  </style>
</head>
<body>
  <input type="color" id="colorBox"><br>
  <button>Change</button>
  <div id="square"></div>

  <script>
    // Get your element references, don't get references to element properties
    var body = document.body;
    var picker = document.getElementById("square");
    var color = document.getElementById("colorBox");
    
    // Set up your event handlers in JavaScript, not HTML
    document.querySelector("button").addEventListener("click", onClick);
    
    function onClick(){
      // Set the color of the body to the current value of the input
      body.style.backgroundColor = color.value;
    }
  </script>
</body>
</html>

Re: Scott Marcus What if i want to make another button with a different function connected to it? — rblx08, Aug 08 '20 at 01:40
You simply add another HTML `button` and reference it from JavaScript (there are many ways to do that (id's, classes, by position) and then set up another event handling function and attach it as I've shown. — Scott Marcus, Aug 08 '20 at 01:56

Cannot read property 'value' of null at index.html:15 in color

2 Answers2