patternjavascriptMinor
Varying a second drop-down's options based on a first selection
Viewed 0 times
selectionvaryingoptionsdropfirstdownsecondbased
Problem
I have some JavaScript code that allows users to select a plan which triggers the second select box to populate with options. The code works, however, it seems to be very inefficient. Any advice on improving the efficiency?
JavaScript:
```
var term = 0;
var price = 0;
var additional = 0;
var fix = 0;
var plan = document.getElementById('billing-plan');
plan.addEventListener('change', enableBilling, false);
var select = document.getElementById('billing-price');
select.addEventListener('change', updatePrice, false);
var basic = {
"Option" : ["$200/month for 1-yr", "$250/month"],
"Value" : [300, 350]
}
var prime = {
"Option" : ["$300/month for 1-yr", "$350/month"],
"Value" : [400, 450]
}
var gold = {
"Option" : ["$400/month for 1-yr", "$450/month"],
"Value" : [500, 550]
}
function populateBilling(planName) {
//RESET BILLING PERIOD OPTIONS AND TOTALS
select.options.length = 1;
document.getElementById('payment-total').innerText = 0 + additional;
document.getElementById('payment-rebill').innerText = 0 + additional;
//FILL BILLING PERIOD WITH PLAN OPTIONS AND VALUES
if (planName == "basic") {
for (i = 0; i < basic.Option.length; i++){
var temp = document.createElement('option');
temp.value = basic.Value[i];
temp.text = basic.Option[i];
select.appendChild(temp);
}
} else if (planName == "prime") {
for (i = 0; i < basic.Option.length; i++){
var temp = document.createElement('option');
temp.value = prime.Value[i];
temp.text = prime.Option[i];
select.appendChild(temp);
}
} else {
for (i = 0; i < basic.Option.length; i++){
var temp = document.createElement('option');
temp.value = gold.Value[i];
temp.text = gold.Option[i];
select.appendChild(temp);
}
}
}
function enableBilling(e) {
document.getElement
JavaScript:
```
var term = 0;
var price = 0;
var additional = 0;
var fix = 0;
var plan = document.getElementById('billing-plan');
plan.addEventListener('change', enableBilling, false);
var select = document.getElementById('billing-price');
select.addEventListener('change', updatePrice, false);
var basic = {
"Option" : ["$200/month for 1-yr", "$250/month"],
"Value" : [300, 350]
}
var prime = {
"Option" : ["$300/month for 1-yr", "$350/month"],
"Value" : [400, 450]
}
var gold = {
"Option" : ["$400/month for 1-yr", "$450/month"],
"Value" : [500, 550]
}
function populateBilling(planName) {
//RESET BILLING PERIOD OPTIONS AND TOTALS
select.options.length = 1;
document.getElementById('payment-total').innerText = 0 + additional;
document.getElementById('payment-rebill').innerText = 0 + additional;
//FILL BILLING PERIOD WITH PLAN OPTIONS AND VALUES
if (planName == "basic") {
for (i = 0; i < basic.Option.length; i++){
var temp = document.createElement('option');
temp.value = basic.Value[i];
temp.text = basic.Option[i];
select.appendChild(temp);
}
} else if (planName == "prime") {
for (i = 0; i < basic.Option.length; i++){
var temp = document.createElement('option');
temp.value = prime.Value[i];
temp.text = prime.Option[i];
select.appendChild(temp);
}
} else {
for (i = 0; i < basic.Option.length; i++){
var temp = document.createElement('option');
temp.value = gold.Value[i];
temp.text = gold.Option[i];
select.appendChild(temp);
}
}
}
function enableBilling(e) {
document.getElement
Solution
Same advice as @jfriend00, with an addition: if the user selects the "Choose Plan" placeholder, disable the billing period selector.
Parsing the selection text to get a JavaScript identifier is sketchy. I'd make the value of each option explicit.
etc.
function populateBilling(planName) {
//RESET BILLING PERIOD OPTIONS
select.options.length = 1;
document.getElementById('payment-total').innerText = 0 + additional;
document.getElementById('payment-rebill').innerText = 0 + additional;
//FILL BILLING PERIOD WITH PLAN OPTIONS AND VALUES
var selectedPlan = plans[planName];
if (selectedPlan) {
for (var i = 0; i < selectedPlan.Option.length; i++) {
var temp = document.createElement('option');
temp.value = selectedPlan.Value[i];
temp.text = selectedPlan.Option[i];
select.appendChild(temp);
}
} else {
document.getElementById('billing-price').disabled = true; // ← This
}
}Parsing the selection text to get a JavaScript identifier is sketchy. I'd make the value of each option explicit.
Basicetc.
function enableBilling(e) {
document.getElementById('billing-price').disabled=false;
var planName = plan.options[plan.selectedIndex].value; // ← This
populateBilling(planName);
}Code Snippets
function populateBilling(planName) {
//RESET BILLING PERIOD OPTIONS
select.options.length = 1;
document.getElementById('payment-total').innerText = 0 + additional;
document.getElementById('payment-rebill').innerText = 0 + additional;
//FILL BILLING PERIOD WITH PLAN OPTIONS AND VALUES
var selectedPlan = plans[planName];
if (selectedPlan) {
for (var i = 0; i < selectedPlan.Option.length; i++) {
var temp = document.createElement('option');
temp.value = selectedPlan.Value[i];
temp.text = selectedPlan.Option[i];
select.appendChild(temp);
}
} else {
document.getElementById('billing-price').disabled = true; // ← This
}
}<option value="basic">Basic</option>function enableBilling(e) {
document.getElementById('billing-price').disabled=false;
var planName = plan.options[plan.selectedIndex].value; // ← This
populateBilling(planName);
}Context
StackExchange Code Review Q#41814, answer score: 5
Revisions (0)
No revisions yet.