tags:

views:

129

answers:

5

Hello,

Can this code be shorten in some way? I've tried several ways to compact it, but I can't get it to work:

    //Customer info
$('input#state-field-a, input#state-field-b').hide();
$('select#country-a').change(function(){      
  if ($(this).val() === "United States" || $(this).val() === "Canada" ||$(this).val() === "null")
  {
    $('select#state-a').show();
    $('input#state-field-a, input#state-field-b').hide();
     } else {
    $('select#state-a').hide();
    $('input#state-field-a').show();
  } 
});
//Shipping nfo
$('select#country-b').change(function(){      
  if ($(this).val() === "United States" || $(this).val() === "Canada" ||$(this).val() === "null")
  {
    $('select#state-b').show();
    $('input#state-field-b').hide();
     } else {
    $('select#state-b').hide();
    $('input#state-field-b').show();
  } 
});

Thanks in advance.

UPDATE: I forgot to give some context to this.

I have two areas in the same page, one for Billing/Customer Info and other for Shipping Info, when the user selects an option from the select menu, other options show/hide within the same section. Both functions should work 'independent' than each other since they belong to different sections.

For example, if I select Canada from the Customer Info select menu, it can't change/alter anything in the Shipping Info section.

Not sure if that makes sense.

Thanks again for any help on this.

+4  A: 

Anywhere you select an id, start with the id. Everything before an id selector is irrelevant.

Ex:

$('#state-field-a, #state-field-b').hide();

Reduce all code duplication. This can be made into a function:

    if ($(this).val() === "United States" || $(this).val() === "Canada" ||$(this).val() === "null")
  {
    $('select#state-a').show();
    $('input#state-field-a, input#state-field-b').hide();
     } else {
    $('select#state-a').hide();
    $('input#state-field-a, select#state-b').show();
  } 

Replace your if statement with this more modular statement:

var acceptedCountries = ["United States", "Canada", "null"];
if( $.inArray($(this).val(), acceptedCountries) > 0 )
Stefan Kendall
`$.inArray()` would return `0` if it was `"United States"` :)
Nick Craver
+4  A: 

You can use a few shortcut functions to narrow all of your code down to this:

$('#state-field-a, #state-field-b').hide();
$('#country-a, #country-b').change(function(){
  var m = $.inArray($(this).val(), ["United States","Canada","null"]) != -1;

  $('#' + this.id.replace('country', 'state') + '-a').toggle(m);
  if(this.id === 'country-a') $('#state-field-b').toggle(m);

  $('#' + this.id.replace('country', 'state-field') + '-a').toggle(!m);
});

We're doing a few things different here:

  • Not using tag selectors on IDs
  • Using .change() once, since they both have the same effect
  • Using $.inArray() to narrow down the if or clauses (IE doesn't have .indexOf()...)
  • Use .toggle(bool) instead of repeated .show()/.hide() code
  • The extra if is to account for the difference in your two handlers
Nick Craver
@bschaeffer - I'm not using `.toggle()`, I'm using `.toggle(bool)`, which is `.toggle(showIfImTrueHideOtherwise)` :) It's equivalent to `$(selector)[bool ? "show" : "hide"]();`
Nick Craver
I deleted my comment because I didn't realize `.toggle(bool)` was an option... commented too quickly. Sorry.
bschaeffer
+1  A: 

First, in your html, change select id's to classes like so:

<select id="a" class="country">...</select>
<select id="b" class="country">...</select>

Then you can shorten it this way (edited using .toggle(bool) from Nick's answer):

var inputs = $('#state-field-a, #state-field-b'),
    selects = $('select.country'),
    countries = {'United States', 'Canada'};

inputs.hide();

selects.change(function() { 
    var value = $(this).val(),
        toggle = ($.inArray(value, countries) != -1 || value === "null");

    $(this).toggle(toggle);
    inputs.toggle(toggle);
});
bschaeffer
A: 

See http://jsfiddle.net/nK2T3/

You need to use a selector context variable to limit the scope of your selection to the appropriate section. For the sake of argument, I assumed your inputs were segregated into fieldsets:

var countrySelects = '#country-a, #country-b',
    stateSelects = '#state-a, #state-b',
    stateInputs = '#state-field-a, #state-field-b';    

$(stateInputs).hide();

$('fieldset').each(function () {
    var $fieldset = $(this);

    $(countrySelects, $fieldset).change(function () {
        var $countrySelect = $(this),
            $stateSelect = $(stateSelects, $fieldset),
            $stateInput = $(stateInputs, $fieldset);

        if ($countrySelect.val() === "United States" || $countrySelect.val() === "Canada" || $countrySelect.val() === "null") {
            $stateSelect.show();
            $stateInput.hide();
        } else {
            $stateSelect.hide();
            $stateInput.show();
        }
    })
});​
draeton
A: 

This is not a shortening of your code, and in fact it maybe a little but longer than your code :)

You could do without too many id's and referencing specific elements by making it generic. As far as I can tell, both the billing and shipping sections work exactly alike with maybe a difference or two. If so, you can have an identical structure for both such as:

<div id='billing' class='address'>
    <select class='country'> .. </select>
    <select class='state'> .. </select>
    <input type='text' class='state-field'>
</div>

<div id='shipping' class='address'>
    <select class='country'> .. </select>
    <select class='state'> .. </select>
    <input type='text' class='state-field'>
</div>

By grouping each under a parent and only using the id for - #billing, #shipping, everything underneath can follow exactly the same structure. Here is the callback handler for when a country is changed (thanks for @Nick Craver for the toggle trick):

function countryChanged() {
    var countries = $(this);
    var selectedCountry = countries.val();
    var container = $(this).closest('.address');

    var shouldShowStates = function(country) {
        return ['United States', 'Canada', 'null'].indexOf(country) != -1;
    }

    var toggleFields = function(showList) {
        $('.state', container).toggle(showList);
        $('.state-field', container).toggle(!showList);
    }

    var showStates = shouldShowStates(selectedCountry);
    toggleFields(showStates);
}


$('.country').change(countryChanged);
Anurag