5
\$\begingroup\$

I have an html button that when the user clicks the button it will change color from blue to red and back. Is there a better way to do this in jQuery?

HTML/jQuery

<html>
<head>
    <title>Button Fun</title>
    <script src = 'bower_components/jquery/dist/jquery.js'></script>
    <script type="home.js"></script>
    <link rel="stylesheet" type="text/css" href="styles.css"/>
    <script type="text/javascript">
    clicked = true;
    $(document).ready(function(){
        $("button").click(function(){
            if(clicked){
                $(this).css('background-color', 'red');
                clicked  = false;
            } else {
                $(this).css('background-color', 'blue');
                clicked  = true;
            }   
        });
    });
    </script>
</head>
<body>

    <button>Press</button>

</body>
</html>

CSS

button{
    background-color: blue;
    height:100px;
    width:150px;
}
\$\endgroup\$
  • \$\begingroup\$ For starters, you don't need an entire library for this (unless you're using jQuery elsewhere in your project) you can just use vanilla js. \$\endgroup\$ – ʰᵈˑ Jul 7 '16 at 15:34
9
\$\begingroup\$
$("button").click(function(){
    if(clicked){
        $(this).css('background-color', 'red');
        clicked  = false;
    } else {
        $(this).css('background-color', 'blue');
        clicked  = true;
    }   
});

You could do it like this instead

$("button").click(function(){
    var color = clicked ? 'red' : 'blue';
    $(this).css('background-color', color);
    clicked = !clicked;
});

We move the color picking to a single variable choice using a ternary statement and then we only have to write out the change to the CSS of the element once. then we flip the boolean.

I don't like the name of the boolean variable, it doesn't accurately describe what it is keeping track of, based on the way the code is written it should be named isButtonBlue.

If the button is blue, turn it red. If the button is not blue, turn it blue.


After looking at this a little bit longer I was thinking that you could make it one line shorter by making another line longer by moving the ternary statement into the CSS change

$("button").click(
    $(this).css('background-color', isButtonBlue ? 'red' : 'blue');
    isButtonBlue = !isButtonBlue;
});
| improve this answer | |
\$\endgroup\$
8
\$\begingroup\$

It would be better to use CSS for styling and manipulate a class with JS (using the jQuery library, in this case).

<!doctype html>
<html>
    <head>
        <meta charset="utf-8">
        <title>Button Fun</title>
        <style>
            button {
                background-color: blue;
                height: 100px;
                width: 150px;
            }

            button.pressed {
                background-color: red;
            }
        </style>
    </head>
    <body>
        <button>Press</button>
        <script src="https://ajax.googleapis.com/ajax/libs/jquery/3.0.0/jquery.min.js"></script>
        <script>
            $(() => {
                'use strict';
                $('button').click(function() {
                    $(this).toggleClass('pressed');
                });
            });
        </script>
    </body>
</html>
| improve this answer | |
\$\endgroup\$
  • 1
    \$\begingroup\$ I discussed your suggested edit to my post with other people in chat. Please feel free to join us, this is a very good answer. \$\endgroup\$ – Malachi Jul 7 '16 at 13:26
  • 1
    \$\begingroup\$ This would be my preferred solution - it does it cleanly by only adding/removing a class as opposed to direct in-line style manipulation. The latter is hard to maintain - what if in the future you want the pressed buttons to be orange? You would need to change each event handler instead of just one CSS value. It's also way harder to override with stylesheets - perhaps in a specific case you want a pressed button to be orange - that's trivial to fix with just CSS but if the style is in-line, you need !important which in turn stops playing nicely with the rest of CSS making maintenance harder \$\endgroup\$ – VLAZ Jul 9 '16 at 23:05
0
\$\begingroup\$

I recently started working on jQuery. And looking here some code reviews for better understanding. Thus i am not an expert but here is my view.

The points, i would care about doing this task are:

    1. I will use .on instead of .click because it allows binding multiple events on same element(s). lets say you want to perform task on hover enent then. Simply replace "click" with "click mouseenter".
    1. Seprate css from jquery. So use class related methods instead of .css. It will be messy and hard to maintain styles using .css.
    1. Avoid annonomus functions global variables(and counters/flags if possible) use names function.

Now for my solution i created a function changeColor($elm, classes). It takes two arguments. One is the element whose class you want to change and an array which contain classes it might have. Thus this function can be reused for similar effect on elments and class set.

Like you can perform same for a input box with ["purple yellow"] classed I placed it out of ready function(it will be available since its called in ready function ) since it make ready function clean and readable.

If you dont want this effect for all the buttons on page then use class selector or if its only one button then id(fastes approach, no DOM travering) selector.

<html>
<head>
    <title>Button Fun</title>
    <script src="https://code.jquery.com/jquery-3.0.0.min.js"></script>
    <style>
        button{            
            height:100px;
            width:150px;            
        }
        .red {
            background-color: red;
        }
        .green {
            background-color: green;
        }
        .blue {
            background-color: blue;
        }
    </style>
    <script type="text/javascript">
        // function to change class
        function changeColor($elm, classes) {                
            var className = $elm.attr('class');                
            var index = $.inArray(className, classes);

            $elm.removeClass(className);
            index++;                
            if (index === classes.length) {
                index = 0;
            }
            $elm.addClass(classes[index]);                
        }        
        // equivalent to $(document).ready
        $(function(){            
            // cache button element
            var $button = $("#dynamic-button");
            var classes = ["red", "green", "blue"];
            // attach event with $button
            $button.on("click", function (){                
                changeColor($(this), classes);                
            });
        });           
    </script>
</head>
<body>
    <button id="dynamic-button" class="blue">Press</button>
</body>
</html>

Working of changeColor.
It takes two argument. One is elment itself($(this)=>$button) and second is attay of classes(["red", "green", "black"]). Get the current class of element(lets say "red") now find the index number of this class in classes and remove it from element. Add just next class from array using index number. If index number is one less then size of array then its last class in array. Reset index to 0.

Drawbacks of this approach However only downside in this approach is if your button has multiple classes say "some-style red". Then this function will not work out of the box. You have to do some modification according to your use case. But styling can be done on id or the element too. Or you can pass only color and then set using .css.

| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.