Traffic lights onclick button function

Answers (5)

Reputation: 1

var redlight = document.getElementById("redlight");
var yellowlight = document.getElementById("yellowlight");
var greenlight = document.getElementById("greenlight");

function start() {
  if (redlight.classList.contains("on")) {
    redlight.classList.remove("on"); //switch off red
    yellowlight.classList.add("on"); //switch on yellow
  } else if (yellowlight.classList.contains("on")) {
    yellowlight.classList.remove("on"); //switch off yellow
    greenlight.classList.add("on"); //switch on green   
  } else if (greenlight.classList.contains("on")) {
    greenlight.classList.remove("on"); //switch off green
    redlight.classList.add("on"); //switch on red
  }
}

.traffic-light {
  background: black;
  width: 75px;
  height: 150px;
  margin: auto;
}
.light {
  width: 40px;
  height: 40px;
  border-radius: 40px;
  margin: auto;
}
.light.red {
  background: #520202;
}
.light.red.on {
  background: #ff0000;
}
.light.yellow {
  background: #3F4A00;
}
.light.yellow.on {
  background: #ffff00;
}
.light.green {
  background: #044A00;
}
.light.green.on {
  background: #008000;
}
button {
  width: 75px;
  height: 30px;
  margin: auto;
}

<div class="traffic-light">
  <div id="redlight" class="light red on"></div>
  <div id="yellowlight" class="light yellow"></div>
  <div id="greenlight" class="light green"></div>
  <button onclick="start()">Motion- Start!</button>
</div>

Upvotes: 0

Yann Chabot

Reputation: 4869

Alot of mistakes actually, there you go I will break it down, but first

TL;DR

Here is working code:

<html>
 <body>
  <div style="background:black;width:75px;height:150px;margin:auto;">
   <div id ="redlight" style="background:#ff0000;width:40px;height:40px;border-        radius:40px;margin:auto;"></div>
   <div id = "yellowlight" style="background:#3F4A00;width:40px;height:40px;border-radius:40px    ;margin:auto"></div>
   <div id = "greenlight" style="background:#044A00;width:40px;height:40px;border-radius:40px;margin:auto"></div>
   <button onclick="start()" style="width:75px;height:30px;margin:auto">Motion-    Start!</button>
 </div>
<script>
  var redlight = document.getElementById('redlight');
  var yellowlight = document.getElementById('yellowlight');
  var greenlight = document.getElementById('greenlight');
  var colors = ["rgb(255, 0, 0)",'rgb(82, 2, 2)','rgb(255, 255, 0)','rgb(63, 74, 0)','rgb(0, 128, 0)','rgb(4, 74, 0)'];
  function start() {
    if  (redlight.style.backgroundColor == colors[0])
    {
        redlight.style.backgroundColor = colors[1]; //switch off red
        yellowlight.style.backgroundColor = colors[2]; //switch on yellow
    }
    else if (yellowlight.style.backgroundColor == colors[2]) {
        yellowlight.style.backgroundColor = colors[3]; //switch off yellow
        greenlight.style.backgroundColor = colors[4]; //switch on green
    }
    else if (greenlight.style.backgroundColor == colors[4]) {
        greenlight.style.backgroundColor = colors[5]; //switch off green
        redlight.style.backgroundColor = colors[0]; //switch on red
    }
 }
 </script>
</body>

First of all, GetElementById does not work, you have to write it that way getElementById

Secondly, You need to have curly braces ({}) surrounding your function start

Thirdly, whatch out, style.background returns the color in rgb. So, I changed the value in you colors array to put them in rgb.

Forthly, watch out at your variables name, you made alot of mistakes in them. The greenlight is named greenliight once. Also, you named colors array color once.

Compare my code and your code and do the fix. Have a good day

Upvotes: 1

Hatchet

Reputation: 5428

OK. Let's go through this line-by-line:

<html>
    <head>
        <body>

Why are you putting a <body> tag inside your <head> tag? Proper HTML structure is more like this:

<html>
    <head>
        <title></title>
    </head>
    <body>
    </body>
</html>

<div style="background:black;width:75px;height:150px;margin:auto;">

Inline styles are hard to read and difficult to maintain. Look into learning CSS.

<div id="redlight" style="background:#ff0000;width:40px;height:40px;border-        radius:40px;margin:auto;"></div>
<div id="yellowlight" style="background:#3F4A00;width:40px;height:40px;border-radius:40px ;margin:auto"></div>
<div id="greenlight" style="background:#044A00;width:40px;height:40px;border-radius:40px;margin:auto"></div>
<button onclick="start()" style="width:75px;height:30px;margin:auto">Motion- Start!</button>

Again-- inline styles. CSS classes were made for this kind of thing.

Inline click handler. Not the best idea, but for now, we'll say it's OK. We have bigger problems to worry about!

Now, the <script> tag:

var redlight = document.GetElementById(redlight);

JavaScript is case-sensitive. You're looking for getElementById. Also, you're trying to get the element with the id of redlight, so you need to pass in a string, not a variable: getElementById("redlight")

var yellowlight = document.GetElementById[yellowlight];
var greenlight = document.GetElementById[greenlight];

Similar case here, but one more thing: you call functions with parentheses, not brackets.

var colors = ['#ff0000', '#520202', '#ffff00', '#3F4A00', '#008000', '#044A00']

Missing semicolon at the end of the line.

function start()

A function needs to be surrounded in curly braces: {}

if redlight.style.background == colors[0] {

If statement conditional needs to be parenthesized.

Also, if you were using CSS classes, this would be much easier.

    redlight.style.background = colors[1] //switch off red
    yellowlight.style.background = colors[2] //switch on yellow
} else if (yellowlight.style.background == "yellow") {
    yellowlight.style.background = colors[3] //switch off yellow
    greenliight.style.background = color[4] //switch on green   
} else if (greenlight.style.background == "green") {
    greenlight.style.background = colors[5] //switch off green
    redlight.style.background = colors[0] //switch on red
}

The rest of it: please, please, Please use classes. They will make your life so much easier. Use semicolons, too. They're important.

Just... be careful when you code.

Here is a majorly revised (and working!) edition of your code:

var redlight = document.getElementById("redlight");
var yellowlight = document.getElementById("yellowlight");
var greenlight = document.getElementById("greenlight");

function start() {
  if (redlight.classList.contains("on")) {
    redlight.classList.remove("on"); //switch off red
    yellowlight.classList.add("on"); //switch on yellow
  } else if (yellowlight.classList.contains("on")) {
    yellowlight.classList.remove("on"); //switch off yellow
    greenlight.classList.add("on"); //switch on green   
  } else if (greenlight.classList.contains("on")) {
    greenlight.classList.remove("on"); //switch off green
    redlight.classList.add("on"); //switch on red
  }
}

.traffic-light {
  background: black;
  width: 75px;
  height: 150px;
  margin: auto;
}
.light {
  width: 40px;
  height: 40px;
  border-radius: 40px;
  margin: auto;
}
.light.red {
  background: #520202;
}
.light.red.on {
  background: #ff0000;
}
.light.yellow {
  background: #3F4A00;
}
.light.yellow.on {
  background: #ffff00;
}
.light.green {
  background: #044A00;
}
.light.green.on {
  background: #008000;
}
button {
  width: 75px;
  height: 30px;
  margin: auto;
}

<div class="traffic-light">
  <div id="redlight" class="light red on"></div>
  <div id="yellowlight" class="light yellow"></div>
  <div id="greenlight" class="light green"></div>
  <button onclick="start()">Motion- Start!</button>
</div>

On a positive note, keep up with the commenting! Even little comments in your code are better than none.

You may want to consider using something other than notepad for your coding-- even a primitive code editor should support bracket matching.

Upvotes: 3

Bryan M

Reputation: 11

When you call GetElementById, be sure to use parentheses instead of square brackets. Also, the 'G' should be lowercased, and you need quotes around the element name. Those three lines should look like this:

document.getElementById("redlight");

Upvotes: 0

Coralie B

Reputation: 603

Did you try to define your function between brackets ?

function start()
{
if  (redlight.style.background==colors[0])
{
    redlight.style.background= colors[1]//switch off red
    yellowlight.style.background=colors[2]//switch on yellow
} else if (yellowlight.style.background=="yellow") {
    yellowlight.style.background=colors[3]//switch off yellow
    greenlight.style.background=color[4]//switch on green  
} else if (greenlight.style.background=="green") {
    greenlight.style.background= colors[5]//switch off green
    redlight.style.background=colors[0]//switch on red
} 
}

also, be careful to your uppercases. The syntax is for instance:

document.getElementById("id").style.background = "red";

And to finish, you wrote "greenliight" and not "greenlight".

Upvotes: 1

Traffic lights onclick button function

Answers (5)

Related Questions