-2

I am learning JS and am trying to simply change the background color of a div when clicking on a button and wonder why my code isn't working.

Maybe somebody can take a quick look at the code below:

let btnLeft = document.getElementsByClassName("left");
let btnRight = document.getElementsByClassName("right");
let ad = document.getElementById("ad");

btnLeft.addEventListener("click", changeTheBg());
btnRight.addEventListener("click", changeTheBg2());


function changeTheBg(){
   
ad.style.backgroundColor = "green";

};

function changeTheBg2(){
    
    ad.style.backgroundColor = "pink";
    
    };
#ad {
width: 400px;
max-width: 500px;
height:200px;
background-color: red;
border-radius: 20px;
}
<html>
<head>
    <link rel="stylesheet" type="text/css" href="/style.css">
</head>
<body>

<div id="ad"></div>

<div id="controls">
    <button class="left">Left
    </button>
    <button class="right">Right
    </button>
</div>
<script src="script.js"></script>
</body>
</html>
Roland
  • 1,908
  • 4
  • 21
  • 34

2 Answers2

1

You have several problems:

  1. .addEventListener() takes a function "reference" as the second argument, you've passed a function "invocation" because you've added parenthesis after the function name:

    btnLeft.addEventListener("click", changeTheBg());
    

    As a result, your changeTheBg function is being invoked immediately and the return value from that function (nothing in this case) winds up being the reference for the callback.

    Simply remove the parenthesis:

    btnLeft.addEventListener("click", changeTheBg);
    
  2. You've misspelled getElementById(), it's not getElementsById().
  3. .getElementsByClassName() returns a collection of elements, not a single one, so trying to call .addEventListener() on the collection will fail. Instead use .querySelector(), which returns the first element that matches the CSS selector passed to it, as in:

    let btnLeft = document.querySelector(".left");
    let btnRight = document.querySelector(".right");
    
  4. pinks is not a valid color.

Here's the working code:

let btnLeft = document.querySelector(".left");
let btnRight = document.querySelector(".right");
let ad = document.getElementById("ad");

btnLeft.addEventListener("click", changeTheBg);
btnRight.addEventListener("click", changeTheBg2);

function changeTheBg(){
  ad.style.backgroundColor = "green";
};

function changeTheBg2(){
  ad.style.backgroundColor = "pink";
};
#ad {
  width: 400px;
  max-width: 500px;
  height:200px;
  background-color: red;
  border-radius: 20px;
}
<div id="ad"></div>

<div id="controls">
    <button class="left">Left
    </button>
    <button class="right">Right
    </button>
</div>
Scott Marcus
  • 64,069
  • 6
  • 49
  • 71
  • That only covers -one- two of the many problems with the code. It was already mentioned in a duplicate before you reopened the question. – Quentin Dec 22 '19 at 19:21
  • @Quentin I addressed the first error I saw and posted, then was in the process of updating the answer when I spotted others. All errors have been identified now. – Scott Marcus Dec 22 '19 at 19:31
  • Thanks Everyone. It's my first day trying to get with js again, sorry for the uncareful errors! – Roland Dec 22 '19 at 19:37
1
  1. getElementsByClassName returns a live NodeList, use querySelector(".classname") instead (see Scott's comment and link for why not to use the NodeList!)
  2. getElementsById is a typo and should be getElementById (singular), as you only select a single element via an id
  3. you have to bind to the handler function name reference, not execute it via ()
  4. "pinks" is another typo and not a valid colour name, it should be "pink"

let btnLeft = document.querySelector(".left");
let btnRight = document.querySelector(".right");
let ad = document.getElementById("ad");

btnLeft.addEventListener("click", changeTheBg);
btnRight.addEventListener("click", changeTheBg2);


function changeTheBg() {
  ad.style.backgroundColor = "green";
};

function changeTheBg2() {
  ad.style.backgroundColor = "pink";
};
#ad {
  width: 400px;
  max-width: 500px;
  height: 200px;
  background-color: red;
  border-radius: 20px;
}
<div id="ad"></div>
<div id="controls">
  <button class="left">Left
    </button>
  <button class="right">Right
    </button>
</div>
Constantin Groß
  • 10,719
  • 4
  • 24
  • 50
  • `getElementsByClassName returns a NodeList, from which you have to select e.g. the first item via index 0` <--- [**No, no, no!!**](https://stackoverflow.com/questions/54952088/how-to-modify-style-to-html-elements-styled-externally-with-css-using-js/54952474#54952474) – Scott Marcus Dec 22 '19 at 19:35