Javascript - setInterval() problem

[Maximation]

So I am trying to switch colors with a delay of 500ms until the user lets it stop.

 

Here is a jsfiddle page :

http://jsfiddle.net/jxrf34ww/

Here is the code : 

<button onclick="animate()" id="start">start</button><button onclick="stop()" id="stop">stop</button><div id="square"></div>

css is not important

var intervId;function animate() {    intervId = setInterval(changeColor, 500);}function stop() {    clearInterval(intervId);}function changeColor() {    var square = document.getElementById("square");    if (square.style.background == "#3498db")         square.style.background = "#2ecc71";    else         square.style.background = "#3498db";}

I was creating this after looking at a MDN article about setInterval, here is the section I am refering to :

https://developer.mozilla.org/en-US/docs/Web/API/window.setInterval#Example_2.3A_Alternating_two_colors

 

It is probably some stupid mistake that is causing the square not to switch colors, but thank you for helping me.

[Luc401]

Hmm dont know why its not working really... But i dont know why you have a function trigger another function. I would have it when you click the button it changes b0 

[Ciccioo]

    if (square.style.background == "#3498db")

that's the problem

you should never read the style attribute, as it won't return what you expect

you can see that yourself by alerting its value at every function call

what you could do instead, is to keep a state variable that keeps track of the background color in use

[Maximation]

Hmm dont know why its not working really... But i dont know why you have a function trigger another function. I would have it when you click the button it changes b0 

 

animate() sets the interval to 500ms. It will execute changeColor() with delays of 500ms.

[fizzlesticks]

What @Ciccioo said + the style for "background-color" should be "backgroundColor"

[Maximation]

that's the problem

you should never read the style attribute, as it won't return what you expect

you can see that yourself by alerting its value at every function call

what you could do instead, is to keep a state variable that keeps track of the background color in use

 

Mh, I do not really understand you. How should I determine the background value without reading the style attribute?

When I try to alert the value of document.getElementById("square").style.background it does nothing..

How should I go about keeping a state variable?

[Ciccioo]

-snip-

by "state variable" i mean just a normal variable, like intervId, that you will set accordingly every time you change the color of the div. this way, you have a reliable way to determine the current color of the div

[Aho]

Here's a fixed version of your script.

 

http://jsfiddle.net/fLf2j5Ly/

 

Using 'style.property' will return a blank string unless the style is defined inline or previously using 'style.property = value;'. Instead, here we use getComputedStyle and getPropertyValue functions to retrieve the actual CSS value, which is returned via RGB. You could convert this to hex and compare it to the hex code, but it's unnecessary.

 

I also moved your inline event handlers to the JS file, as they are bad practice.

 

To maintain compatibility with older browsers, IE8 and below specifically, you'll have to modify the script a bit to get the background color using a different function.

[Maximation]

Here's a fixed version of your script.

 

http://jsfiddle.net/fLf2j5Ly/

 

Using 'style.property' will return a blank string unless the style is defined inline or previously using 'style.property = value;'. Instead, here we use getComputedStyle and getPropertyValue functions to retrieve the actual CSS value, which is returned via RGB. You could convert this to hex and compare it to the hex code, but it's unnecessary.

 

I also moved your inline event handlers to the JS file, as they are bad practice.

 

To maintain compatibility with older browsers, IE8 and below specifically, you'll have to modify the script a bit to get the background color using a different function.

 

Good, thank you.

I will develop the habit of not creating inline event handlers.