HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Dynamically changing available select options

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
dynamicallyavailableselectoptionschanging

Problem

I have the following code working here on JSFIDDLE:


    
    

Opt1
Opt2

     
          Please select
          6 months
          9 months
          10 months
          12 months
          18 months
          24 months
          30 months
          36 months
          42 months
          48 months
    

if (window.jQuery) {
// jQuery is available.
}
else { console.log("-- no jquery --"); }

$(document).ready(function() {
    $("input.radio-button-selector").change(function(){ 
        console.log("Changed.");
        if ( $(this).val() == "opt2" ) {
            $("#term option[value='48']").hide();
            $("#term option[value='42']").hide();
        }
        else if ( $(this).val() == "opt1" ) {
            $("#term option[value='48']").show();
            $("#term option[value='42']").show();
        }
        });
    });


Questions:

  • Any way to improve the jQuery code? I saw a lot of times what surprisingly elegant things can be done in JS



  • Any way to improve the html code?



  • I only use the class "input-button-selector" for the JavaScript, is this actually needed or could I select the radio buttons in a different way?



Note that this code won't work on IE 8/9/10 (jQuery is not loading)

Solution

This is a classic example of counting on the UI to keep track of state and model.

Because you know there are only 2 options, and you know how to go from one option to another, you simply hide and add options. Nowhere in your code is explicitly declared what durations are linked to which option.

If you dont care about that, you could optimize the code doing something like this:

$(document).ready(function() {
  var monthsDifference = $("#term option[value='48'], #term option[value='42']");

    $("input.radio-button-selector").change(function(){ 
        if ( $(this).val() == "opt2" ) {
            monthsDifference.hide();
        }
        else if ( $(this).val() == "opt1" ) {
            monthsDifference.show();
        }
        });
    });


I would advise you to care though, and have the code know what the months are and build selectors for each option. Something like this (untested):

$(document).ready(function() {
  var optionMonths = {
      opt1: [6,9,10,12,18,24,30,36,42,48],
      opt2: [6,9,10,12,18,24,30,36]
  }
  //Cache all options
  var $options =  $("#term option");

    $("input.radio-button-selector").change(function(){ 
        //Hide all options first
        $options.hide();    
        //Then show relevant options
        showOptionMonths( optionMonths[ $(this).val()] );
    });
});

function showOptionMonths( months ){
    months.forEach( function( month ){
       $("#term option[value='" + month + "']").show();
    });
}


Of course, I would try to find a better name than opt1 or opt2, but I dont have enough knowledge of what you are trying to achieve to suggest something more meaningful.

Finally, you could cache the selectors for each option, I leave that as an exercise for the reader.

Code Snippets

$(document).ready(function() {
  var monthsDifference = $("#term option[value='48'], #term option[value='42']");

    $("input.radio-button-selector").change(function(){ 
        if ( $(this).val() == "opt2" ) {
            monthsDifference.hide();
        }
        else if ( $(this).val() == "opt1" ) {
            monthsDifference.show();
        }
        });
    });
$(document).ready(function() {
  var optionMonths = {
      opt1: [6,9,10,12,18,24,30,36,42,48],
      opt2: [6,9,10,12,18,24,30,36]
  }
  //Cache all options
  var $options =  $("#term option");

    $("input.radio-button-selector").change(function(){ 
        //Hide all options first
        $options.hide();    
        //Then show relevant options
        showOptionMonths( optionMonths[ $(this).val()] );
    });
});

function showOptionMonths( months ){
    months.forEach( function( month ){
       $("#term option[value='" + month + "']").show();
    });
}

Context

StackExchange Code Review Q#70524, answer score: 3

Revisions (0)

No revisions yet.