0

Given the following HTML:

<fieldset>
    <legend>
        <input type="checkbox" name="amazon_activated" /> 
    </legend>
    <table>
        <tr>
            <td>
                Amazon Data
            </td>
        </tr>
    </table>
</fieldset>

next code allows hide/show the data container table related to the checkbox:

$("input[name='amazon_activated']").click(function(){
    $(this).parent("legend").next("table").toggle( $(this).is(":checked") );
});

and next code it should init the hide/show state after page's load:

if ( ! $("input[name='amazon_activated']").is(":checked")){
    $(this).parent("legend").next("table").hide();
}

Well, it is failing.

I already know why: this refers to the page element, not the checkbox element.


So I wonder:

Is it the best policy immediately choose an id/class:

$("#id")

for the important elements in order to facilitate their control through jquery, over form-based selectors?:

$("input[name='amazon_activated']")

5 Answers 5

1

Cache the element.

var element = this;

When this is in the right context.


You usually code Javascript for a given markup, not the other way around. ID's and classes have meaning other than being used as selectors.

1
  • No, I've addressed the issue. Don't add unnecessary classes or ID's. Instead, once you find the element you want, save it in a variable and use it instead of the out-of-context this. May 29, 2012 at 18:02
1

I think you can cache your elements at the top of the outer function. It will be convenient to use them in other places. Also, caching elements will also enhance the performance. You can do something like this:

var input = $("input[type='checkbox']"),
    table = $("fieldset table");
1
if ( ! $("input[name='amazon_activated']").is(":checked")){
    $(this).parent("legend").next("table").hide();
}

within above code this belong to page element scope, where

$("input[name='amazon_activated']").click(function(){
    $(this).parent("legend").next("table").toggle( $(this).is(":checked") );
});

in above code this belongs to input[name='amazon_activated']'s callback scope which point to input[name='amazon_activated'].

So to make active first code you should try

if ( ! $("input[name='amazon_activated']").is(":checked")){
    $("input[name='amazon_activated']").parent("legend").next("table").hide();
}

Its better to keep reference of input in a variable and use that

var checkboxElement = $("input[name='amazon_activated']"); // you can also use id

then you this like

if ( ! checkboxElement.is(":checked")){
    checkboxElement.parent("legend").next("table").hide();
}

checkboxElement.click(function(){
    $(this).parent("legend").next("table").toggle( $(this).is(":checked") );
});
1

I think the reason every answer is telling you to cache your objects is that it will be tremendously faster. To answer the specific question, disregarding the rest, i.e.:

Is it the best policy immediately choose an id/class for the important elements in order to facilitate their control through jquery, over form-based selectors?

First, I would say "attribute selectors" over "form-based selectors" as I don't believe jQuery distinguishes between, say $('input[name="amazon_activated"]') and $('a[href="#"]') as far as how it searches.

That said, the general rule of thumb is:

id selectors are faster than class selectors are faster than attribute selectors.

So, if all you care about is jQuery speed, that's key. However, adding ids and classes, only for the sake of targeting via jQuery could slow down your page load times more than the corresponding speed-up in selector performance.

In summary to this overly-long answer:

  • Cache the result of a jQuery selector when possible
  • Use ids and classes when possible, but
  • Don't add unnecessary ids and classes unless testing proves them necessary
0

Thanks to all for your answers. All have been very helpful (I give +1 to all).

I want to add my final solutions that takes in account your tips.

Indeed there are several checkbox elements involved, so I have cached them and use an associative array to iterate over it (avoiding add more id/class).

var checkboxElements = {
    "amazon_activated" : $("input[name='amazon_activated']"),
    "clickbank_activated" : $("input[name='clickbank_activated']"),
    "ebay_activated" : $("input[name='ebay_activated']")
}

$.each(checkboxElements, function(i, el){
    $(el).parent("legend").next("table").toggle( $(el).is(":checked") );
    $(el).click(function(){
        $(el).parent("legend").next("table").toggle( $(el).is(":checked") );
    });
});

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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