0

I'm just starting in on javascript and working on a problem that our instructor gave us. We have an html site with four buttons, each of the buttons has a color, and when you hit the button it changes the background/text color. Sample HTML and javascript below.

HTML

<div id="wrapper">
  <ul id="switcher">
    <li id="grayButton"></li>
    <li id="whiteButton"></li>
    <li id="blueButton"></li>
    <li id="yellowButton"></li>
  </ul>
</div>

CSS

document.getElementById("yellowButton").onclick = turnYellow;

function turnYellow (){
   document.getElementById("wrapper").style.backgroundColor = "yellow";
   document.getElementById("wrapper").style.color = "orange";
    }

I got that to work fine, but I was trying to refactor so that my function was more generic:

document.getElementById("grayButton").onclick = changeColor("gray", "white");

function changeColor(backColor, frontColor) {
   document.getElementById("wrapper").style.backgroundColor = backColor;
   document.getElementById("wrapper").style.color = frontColor;  
}

and I can't figure out why that doesn't work. Any thoughts?

4 Answers 4

4

You need to call your function in an anonymous function, adding the parameters () caused the function to be invoked immediately:

document.getElementById("grayButton").onclick = function() {
    changeColor("gray", "white");
}
Sign up to request clarification or add additional context in comments.

Comments

1

"element.onlick" event requires a function object (callable/callback) not a return value. Thus it should be:

document.getElementById("grayButton").onclick = function(){
    changeColor("gray", "white");
}

Also, HTML5 data attributes seems like a great idea:

<html>
<body>
<div id="wrapper"> 
  <ul id="switcher"> 
    <li data-bg-color="red"  data-color="white" onclick="changeColor(this)">
       red/white
    </li> 
    <li data-bg-color="gray" data-color="black" onclick="changeColor(this)">
       gray/black
    </li>
  </ul> 
</div>

<script>   
  function changeColor(el) {   
      var wrapper =  document.getElementById("wrapper");  
      wrapper.style.color = el.getAttribute('data-color'); 
      wrapper.style.backgroundColor = el.getAttribute('data-bg-color'); 
  }
</script>
</body>
</html>

Comments

0

This is my solution My example

HTML

 <body onload="start()">
    <div id="wrapper">
      <ul>
        <li data-params="yellow,orange">Hit me</li>
        <li data-params="green,darkgreen">Hit me</li>
        <li data-params="aqua,blue">Hit me</li>
        <li data-params="chocolate,brow">Hit me</li>
      </ul>
    </div>
  </body>

JavaScript

function start(){
  //Get necesary elements
  var list = document.getElementsByTagName("li");
  //Adding event
  for(var i in list){
    list[i].addEventListener("click", changeStyle);
  }
}

function changeStyle(){
  var params  = this.dataset.params.split(","),
      wrapper = document.getElementById("wrapper");

  wrapper.style.backgroundColor = params[0];
  wrapper.style.color =  params[1];
}

Comments

0

You simply called the function changeColor and assigned its return value which is undefined to the event handler.

I am pretty sure it even worked and changed the text color and background color for that element but not like what you expected i.e. synchronously vs asynchronously.

You just need to leave a reference to the event handler like this changeColor without the parentheses like what you did in the first example and the event handler would take care of everything for you when the event occurs/fires and it detects it.

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.