Question

I am overlooking something pretty fundamental here. I want to color a specific element in a group whenever a checkbox is clicked. So I need to make these elements observable.

This is what my html looks like

<p>
    <label>
        <i>bla2</i>
        <input type="checkbox" />
    </label>
</p>

<p>
    <label>
        <i>bla3</i>
        <input type="checkbox" />
    </label>
</p>

My JS looks like this

$(document).ready(function() {

    function handleCheckbox () { 

        if ( $(this).closest(':checkbox').is(':checked') ) {
            $('this').closest('i').css('color','green');
        } else {
            $('this').closest('i').css('color','red');
        }
    }

    handleCheckbox();
    $('label').on('click', handleCheckbox() );

 });
Was it helpful?

Solution

.closest() looks at the current element and then up the hierarchy at ancestors, not at neighbors. In your case, this will point to the label object so you can look at children to find the <i> tag and <input> tag. You also have several other coding errors.

Also, your handleCheckbox() function needs the value of this set to a <label> object in order to work properly so you can't just call it direct and expect it to set all the colors appropriately. Instead, you will have to iterate over all the labels in the page and call handleCheckbox() for each one. I've done that in the code below with .each().

Here's a way to fix it:

$(document).ready(function() {
    function handleCheckbox() {
        // the this pointer here will point to the label object so you need
        // can use .find() to find children of the label object
        var newColor;
        if ($(this).find("input").is(":checked")) {
            newColor = "green";
        } else {
            newColor = "red";
        }
        $(this).find("i").css("color", newColor);
    }
    // hook up click handler and initialize the color for all labels
    $('label').on('click', handleCheckbox).each(handleCheckbox);        

 });

See working demo: http://jsfiddle.net/jfriend00/tRQ99/ and also notice that the initial color is set based on the initial checkbox state too.

Your code had these issues:

  1. .closest() goes up to ancestors. It doesn't find neighbors.
  2. When passing a callback function, you don't use the () at the end because that causes it to execute immediately and pass the return value of executing the function. You just want to pass a reference to the function which is done without the parens.
  3. You don't quote this. Treat it like a javascript variable, not a string.
  4. The this pointer will point to the label object in your callback so you need to look at child elements to find the <i> and <input> objects. You can use either .children() or .find() to find them.
  5. You initial call of handleCheckbox() was not working because it works only when this is set to a <label> object (the way it works in the event handler). So, to use the same function for initialization as the event handler, you need to iterate over all the labels and make sure this is set appropriately for that function. A simple way to do that is with .each() as I've shown in my code suggestion.

OTHER TIPS

You're passing a function in rather than using an anonymous function, so remove the ()

$('label').on('click', handleCheckbox);

Else this will execute right away on page load. Also, unquote this:

$(this).closest('i').css('color','green');

Fiddle for what you probably want here:

http://jsfiddle.net/93CeR/2

$(document).ready(function() {

    function handleCheckbox () { 

        if ( $(this).find(':checkbox').is(':checked') ) { // Use .find instead of closest because the element you want is a child
            $(this).find('i').css('color','green'); // Remove qoutes from this and use .find for same reason as above
        } else {
            $(this).find('i').css('color','red'); // Ditto
        }
    }

    $('label').each(function(){
       handleCheckbox.apply(this); // We need to call the function initially using 'label' for this.
    });
    $('label').on('click', handleCheckbox ); // You want a reference to handleCheckbox not to invoke it so remove the parenthesis

 });
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top