[Okta Webinar] Learn how to a build a cloud-first strategyRegister Now

x
  • Status: Solved
  • Priority: Medium
  • Security: Public
  • Views: 582
  • Last Modified:

Click function doesn't work first time

This code to toggle a button color does not work on the first click and only starts working after the second one. Why?

<!DOCTYPE html>
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
    <title></title>
    <style>
        #Button1 {
            background-color: rgb(0, 170, 88);
        }
    </style>
</head>

<body>
    <input id="Button1" type="button" value="Toggle Color" onclick="toggleColor()" />
        <script language="javascript" type="text/javascript">
        function toggleColor() {
            if (document.getElementById("Button1").style.backgroundColor == "rgb(0, 170, 88)") {
                document.getElementById("Button1").style.backgroundColor = "rgb(238, 52, 36)";
            }
            else {
                document.getElementById("Button1").style.backgroundColor = "rgb(0, 170, 88)";
            }
        }

    </script>
</body>
</html>
0
BobHavertyComh
Asked:
BobHavertyComh
1 Solution
 
Paul JacksonCommented:
Your JavaScript function needs to be defined before it appears in the html, move your JavaScript in between the Head tags where it belongs and it will work fine.
0
 
Rainer JeschorCommented:
Hi,
nothing to do with the location of the script.
Simply on the first click, the Javascript returns an empty string as the color is not set on the button directly. Then it sets the value whereas in the subsequent calls there is a value where the compare works.

Please try here:
http://jsfiddle.net/EE_RainerJ/kp630L57/

Solution:
either set the style directly on the button OR additionally add the check for "" (empty)

HTH
Rainer
0
 
Jim RiddlesPrepress/OMS SpecialistCommented:
As Rainer stated, the value of the background color is not actually set in JavaScript at the beginning of the script, therefore you need to check for a null value, as well.  I have altered your code somewhat to make it a little cleaner and less cluttered.  See below:

<input id="Button1" type="button" value="Toggle Color" onclick="toggleColor(this)" />
<script language="javascript" type="text/javascript">
function toggleColor(obj) {
    if (!obj.style.backgroundColor || obj.style.backgroundColor == "rgb(0, 170, 88)") {
        obj.style.backgroundColor = "rgb(238, 52, 36)";
    } else {
        obj.style.backgroundColor = "rgb(0, 170, 88)";
    }
}
</script>

Open in new window


Note that on line 1 I am passing the object along with the function call.  This allows me to reference the object as a variable, obj, rather than using "document.getElementById" every time.  On line 3, I add the obj reference in the parens for the function.  On line 4, I begin the if statement by checking to see if the background is false or null while checking for the default color, as well.  That is the only true change you need to make to your existing code.  Whereas before, on the first click, your if statement was false for the first condition, and automatically assigned the background color to the same value you were using in the else clause.

Alternatively, you also could have simply checked to see if it the value was not your default value, first and change it to the second color in the else clause.  Although that would have worked in this instance, as you were only checking for two colors, that is not the correct method.

Let me know if you have any questions on this.  Thanks!
0
 
Jim RiddlesPrepress/OMS SpecialistCommented:
Upon further reflection, I wanted to get a clearer understanding of this issue.  What I came upon is that if an element is styled using CSS stylesheets, then there is a different method to detect the style.

<input id="Button1" type="button" value="Toggle Color" onclick="toggleColor(this)" />
<script language="javascript" type="text/javascript">
function toggleColor(obj) {
    if (getComputedStyle(obj).backgroundColor == "rgb(0, 170, 88)") {
        obj.style.backgroundColor = "rgb(238, 52, 36)";
    } else {
        obj.style.backgroundColor = "rgb(0, 170, 88)";
    }
}
</script>

Open in new window


Note my change on line 4.  What I have done is used "getComputedStyle(element)" to get the current computed style of the input button using the obj variable.  This will correctly determine the current background color of the button each time the function is called.

I believe this is the best method to use, but my previous post will work, as well.  Cheers!
0
 
BobHavertyComhAuthor Commented:
Right, inline styling assures it is set from the beginning. Thanks.
0

Featured Post

VIDEO: THE CONCERTO CLOUD FOR HEALTHCARE

Modern healthcare requires a modern cloud. View this brief video to understand how the Concerto Cloud for Healthcare can help your organization.

Tackle projects and never again get stuck behind a technical roadblock.
Join Now