0

I am trying to learn JavaScript from here . As told here I have following main.js file which will change the background color but nothing is happening when I click the button :-

var btn = document.querySelector('button');

function bgChange() {
  var rndCol = 'rgb(' + random(255) + ',' + random(255) + ',' + random(255) + ')';
  document.body.style.backgroundColor = rndCol;
}

btn.onclick = bgChange;

and index.html file:-

<!DOCTYPE html>
<html>
<head>
    <title>Js</title>
    <link rel="stylesheet"  href="stylesheet.css" />
    <script src="main.js"></script>
</head>
<body>

    <button>Dont click</button>
    <p id="p1" > om !!!!</p>

</body>
</html>

When I open this firefox and click on the button nothing happens please help me :(

  • 1
    The element (`button`) doesn't exist when your script runs, therefore it won't work. you either need to move the script to the footer (the "lazy" solution), or use a [document ready](https://stackoverflow.com/questions/799981/document-ready-equivalent-without-jquery) function, or [event delegation](https://davidwalsh.name/event-delegate). – random_user_name Feb 23 '18 at 14:52
  • 1
    https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/addEventListener – symcbean Feb 23 '18 at 14:52
  • @cale_b means that the button doesn't exist when your script runs. Until the browser has actually parsed the html and gone past the button element, `document.querySelector('button')` will return null. – Reinstate Monica Cellio Feb 23 '18 at 14:56

3 Answers3

3

The issue is that your script is loading before the DOM loads, so your button doesn't exist when your javascript adds the event handler. Since DOM parsing is synchronous (it happens in order, one line at a time) you can fix this by adding your script tags to the bottom of your HTML, above your </body> tag. This is fairly common practice. 

<html>
  <head>
    <title>Js</title>
    <link rel="stylesheet"  href="stylesheet.css" />
  </head>
  <body>
    <button>Dont click</button>
    <p id="p1" > om !!!!</p>
    <script src="main.js"></script>
  </body>
</html>

There is also an attribute called defer for script tags that will accomplish the same thing. It forces the script tags to load last, and you can use it like this:

<html>
  <head>
    <title>Js</title>
    <link rel="stylesheet"  href="stylesheet.css" />
    <script defer src="main.js"></script>
  </head>
  <body>
    <button>Dont click</button>
    <p id="p1" > om !!!!</p>
  </body>
</html>
Robbie Milejczak
  • 4,866
  • 1
  • 20
  • 52
1

The best approach to avoid this kind of problems is binding the event DOMContentLoaded to wait until the DOM is fully loaded and parsed.

document.addEventListener('DOMContentLoaded', function() {
  console.log("DOM is fully loaded and ready for DOM manipulation.");

  var btn = document.querySelector('button');

  function bgChange() {
    function random() {return '0';} // Just to illustrate!
    
    var rndCol = 'rgb(' + random(255) + ',' + random(255) + ',' + random(255) + ')';
    document.body.style.backgroundColor = rndCol;
  }

  btn.onclick = bgChange;
});
<button>Dont click</button>
<p id="p1"> om !!!!</p>

DOMContentLoaded event

The DOMContentLoaded event is fired when the initial HTML document has been completely loaded and parsed, without waiting for stylesheets, images, and subframes to finish loading. A very different event load should be used only to detect a fully-loaded page. It is an incredibly popular mistake to use load where DOMContentLoaded would be much more appropriate, so be cautious.

Ele
  • 31,191
  • 6
  • 31
  • 67
0

Why not just

<button id="yey">Dont click</button>

and

var btn = document.getElementById("yey");