Dynamically changing available select options

Question

I have the following code working here on JSFIDDLE:

<!DOCTYPE html>
<html>
<head>
    <meta http-equiv="X-UA-Compatible" content="IE=edge">
    <script src="//ajax.googleapis.com/ajax/libs/jquery/1.9.1/jquery.min.js"></script>
</head>

<body>

<label for="opt1"><input class="radio-button-selector" type="radio" name="account" id="opt1" value="opt1">Opt1</label>
<label for="opt2"><input class="radio-button-selector" type="radio" name="account" id="opt2" value="opt2">Opt2</label>

<div id="form_select" class="form-group">
     <select id="term" name="term" class="form-control">
          <option value="0">Please select</option>
          <option value="6">6 months</option>
          <option value="9">9 months</option>
          <option value="10">10 months</option>
          <option value="12">12 months</option>
          <option value="18">18 months</option>
          <option value="24">24 months</option>
          <option value="30">30 months</option>
          <option value="36">36 months</option>
          <option value="42">42 months</option>
          <option value="48">48 months</option>
    </select>
</div>

<script type="text/javascript">
if (window.jQuery) {
// jQuery is available.
}
else { console.log("-- no jquery --"); }
</script>

<script type="text/javascript">
$(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();
        }
        });
    });
</script>
</body>
</html>

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)

@ProgramFOX If he is not asking for us to fix it, but it works on a major browser (Chrome/FireFox/Safari), then that's okay — konijn
– konijn, Commented Nov 21, 2014 at 17:20

konijn · Accepted Answer · 2014-11-21 17:38:41Z

3

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.

answered Nov 21, 2014 at 17:38

konijn

34.4k5 gold badges71 silver badges267 bronze badges

\$\begingroup\$ 1) What is the improvement? Can you explain why is better? 2) Why not test it before posting, as I did post the fiddle, you could just fork it? 3) opt1 and opt2 names are used just for privacy, production code has relevant names. \$\endgroup\$

bg17aw
– bg17aw

2014-11-22 08:00:58 +00:00
Commented Nov 22, 2014 at 8:00
\$\begingroup\$ 4) "This is a classic example of counting on the UI to keep track of state and model." Is this a bad or a good thing? \$\endgroup\$

bg17aw
– bg17aw

2014-11-22 08:05:45 +00:00
Commented Nov 22, 2014 at 8:05
\$\begingroup\$ 5) "Nowhere in your code is explicitly declared what durations are linked to which option." Not sure what you mean and why this is bad. \$\endgroup\$

bg17aw
– bg17aw

2014-11-22 08:09:16 +00:00
Commented Nov 22, 2014 at 8:09
\$\begingroup\$ 6) "you could cache the selectors for each option" I think var monthsDifference = $("#term option[value='48'], #term option[value='42']"); and var $options = $("#term option"); in your code are examples of caching the selectors. Am I wrong? \$\endgroup\$

bg17aw
– bg17aw

2014-11-22 08:32:23 +00:00
Commented Nov 22, 2014 at 8:32
\$\begingroup\$ For the next maintainer it is bad to not have the links between the options and the months. From a software maintenance perspective, the second option is better.But, if you are comfortable by only caching 48 and 42, then you can go for that. \$\endgroup\$

konijn
– konijn

2014-11-22 16:09:54 +00:00
Commented Nov 22, 2014 at 16:09

Add a comment |

Stack Exchange Network

Dynamically changing available select options

1 Answer 1

You must log in to answer this question.

Hot Network Questions

Dynamically changing available select options

1 Answer 1

You must log in to answer this question.

Related

Hot Network Questions