2
\$\begingroup\$

I have about 6 of these, but I'm only posting two. Essentially I have a bunch of options with a "yes" or "no" radio button. If "no" is selected, I want a form to popup.

$('#Option1').live('change', function() {
    if ($('input[name=Option1]:checked').val() == "False") {
        $('#formDiv').show();
    } else {
        $('#formDiv').hide();
    }
});
$('#Option2').live('change', function() {
    if ($('input[name=Option2]:checked').val() == "False") {
        $('#formDiv').show();
    } else {
        $('#formDiv').hide();
    }
});

How can I make this more efficient, or even consolidate it? Or is having a function for each pair of radio buttons necessary?

\$\endgroup\$
  • 1
    \$\begingroup\$ This belongs on codereview, but when you post it there, please include your HTML. Only by seeing the repeated parts of the HTML can people offer the best way to do the code. \$\endgroup\$ – jfriend00 Jan 17 '12 at 0:35
  • \$\begingroup\$ Also, you should be switching to $.on() and/or $.delegate(), as $.live() is deprecated. \$\endgroup\$ – Jared Farrish Jan 17 '12 at 0:40
3
\$\begingroup\$

This should work:

$('#Option1, #Option2').live('change', function() {
    if ($('input[name=' + $(this).attr('id') + ']:checked').val() == 'False') {
        $('#formDiv').show();
    else {
        $('#formDiv').hide();
    }

    });

Or, if whatever has the id option1 is the same page element as the input with the name option1, you could do:

$('#Option1, #Option2').live('change', function() {
    if ($(this).val() == 'False')
        $('#formDiv').show();
    else {
        $('#formDiv').hide();
    }

    });
\$\endgroup\$
  • \$\begingroup\$ .show and .hide could be replaced to a .toggle($(this).val()=='False') statement. \$\endgroup\$ – biziclop Jan 17 '12 at 14:29
0
\$\begingroup\$

I'd recommend setting up a listener on all your radio buttons then using 'this' to reference the radio button that is selected. If you are writing six identical functions, you're not adhering to DRY.

Without the HTML to back this up, it's a little tricky to see exactly what's going on.

\$\endgroup\$
0
\$\begingroup\$

You could bind the selection change to a class instead of the id, this way you will have a generic function valid for all the select elements. Something like :

$('.mySelect').live('change', function() {  
    var selectedOptionValue = $(this).val();
    if ($('input[name='+selectedOptionValue+']:checked').val() == "False") {
        $('#formDiv').show();
    }
    else {
        $('#formDiv').hide();
    }


});
\$\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.