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

Displaying dependent options when a selection is made in a dropdown

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

Problem

I was wondering if someone could please point me in the right direction with this bit of JavaScript I have been working on.

I have made this form that currently has onblur JavaScript validation and additionally I am to make it dynamic so that when a user selects an option from a dropdown option form a div is shown with corresponding options. But at the moment that code I have written for this is looking pretty repetitive and suffering from 'code smell'.

Is there a better way to do this? I am thinking a for loop that iterates through the dropdown options but am not 100% sure how to put this in place?

```
adventureForm = document.querySelector('#adventure');
charterDrop = document.querySelector('.charter-options');
mountainDrop = document.querySelector('.mountain-lake-options');
trampDrop = document.querySelector('.tramp-options');
photoDrop = document.querySelector('.photo-options');
dropDowns = document.querySelector('#dropdowns');

// check adventure dropdown choice
if (field.id === "adventure" && adventureForm.selectedIndex === 0) {
dropDowns.classList.add('hidden');

} else if (field.id === "adventure" && adventureForm.selectedIndex === 1) {
dropDowns.classList.remove('hidden');
charterDrop.classList.remove('hidden');
mountainDrop.classList.add('hidden');
trampDrop.classList.add('hidden');
photoDrop.classList.add('hidden');
} else if (field.id === "adventure" && adventureForm.selectedIndex === 2) {
dropDowns.classList.remove('hidden');
charterDrop.classList.add('hidden');
mountainDrop.classList.remove('hidden');
trampDrop.classList.add('hidden');
photoDrop.classList.add('hidden');
} else if (field.id === "adventure" && adventureForm.selectedIndex === 3) {
dropDowns.classList.remove('hidden');
charterDrop.classList.add('hidden');
m

Solution

There are definitely improvements. Let us start with some basics.

Begin your file with

'use strict';


This avoids a class of nasty bugs that easily creep into JavaScript programs.

Do not use global variables. Parameterization is generally the best option but in this case we can avoid polluting the global namespace by simply wrapping our code in an immediately invoked function expression (IIFE).

(function () {
    var elements;
    var i = 0;
    var isError = false;
    var errorMessage;
    var correctMessage = '✓';
    var errorSpan;
    ...
}());


Continuing, there is indeed a lot of repetition as you have correctly recognized. Fortunately, JavaScript makes it easy to remove this kind of duplication while at the same time making our code more declarative. Let's start with some simple refactoring

function hide(element) {
    element.classList.add('hidden');
}

function show(element) {
    element.classList.remove('hidden');
}


Now let us extract the code under // check adventure dropdown choice
gets us to this so far

function checkAdventureDropdown(option) {
    if (option.id !== 'adventure') {
        return;
    }

    if (adventureForm.selectedIndex === 0) {
        dropDowns.classList.add('hidden');
    } else if (adventureForm.selectedIndex === 1) {
        dropDowns.classList.remove('hidden');
        charterDrop.classList.remove('hidden');
        mountainDrop.classList.add('hidden');
        trampDrop.classList.add('hidden');
        photoDrop.classList.add('hidden');
    } else if (adventureForm.selectedIndex === 2) {
        dropDowns.classList.remove('hidden');
        charterDrop.classList.add('hidden');
        mountainDrop.classList.remove('hidden');
        trampDrop.classList.add('hidden');
        photoDrop.classList.add('hidden');
    } else if (adventureForm.selectedIndex === 3) {
        dropDowns.classList.remove('hidden');
        charterDrop.classList.add('hidden');
        mountainDrop.classList.add('hidden');
        trampDrop.classList.remove('hidden');
        photoDrop.classList.add('hidden');
    } else if (adventureForm.selectedIndex === 4) {
        dropDowns.classList.remove('hidden');
        charterDrop.classList.add('hidden');
        mountainDrop.classList.add('hidden');
        trampDrop.classList.add('hidden');
        photoDrop.classList.remove('hidden');
    }
}


Note that we can factor out the common test into an early return.
There is a lot more we can do here. We could simplify with a switch statement, but I think a cleaner approach would be to use an object.
Let us convert our imperative conditional code into a mapping that declares which elements should be visible and which elements need to be hidden for each specific option.

var states = {
    0: { hide: [dropDowns], show: [] },
    1: {
        hide: [trampDrop, mountainDrop, photoDrop],
        show: [dropDowns, charterDrop]
    },
    2: {
        show: [mountainDrop],
        hide: [charterDrop, trampDrop, photoDrop]
    },
    3: {
        show: [trampDrop],
        hide: [charterDrop, mountainDrop, photoDrop]
    },
    4: {
        show: [dropDowns, photoDrop],
        hide: [charterDrop, mountainDrop, trampDrop]
    }
}


Not only is this more readable, as we can see clearly which states include and exclude which components. Now checkAdventureDropdown becomes quite simple, with very few places for bugs to hide.

function checkAdventureDropdown(option, states) {
    if (option.id !== 'adventure') {
        return;
    }
    var state = states[adventureForm.selectedIndex];
    state.show.forEach(show);
    state.hide.forEach(hide);
}


Here is what we end up with

'use strict';
(function () {
    var elements;
    var i = 0;
    var isError = false;
    var errorMessage;
    var correctMessage = '✓';
    var errorSpan;

    var adventureForm;
    var charterDrop;
    var mountainDrop;
    var trampDrop;
    var photoDrop;
    var dropDowns;

    var states = {
        0: { hide: [dropDowns], show: [] },
        1: {
            hide: [trampDrop, mountainDrop, photoDrop],
            show: [dropDowns, charterDrop]
        },
        2: {
            show: [mountainDrop],
            hide: [charterDrop, trampDrop, photoDrop]
        },
        3: {
            show: [trampDrop],
            hide: [charterDrop, mountainDrop, photoDrop]
        },
        4: {
            show: [dropDowns, photoDrop],
            hide: [charterDrop, mountainDrop, trampDrop]
        }
    };
    if (option.id !== 'adventure') {
        return;
    }
    function checkAdventureDropdown() {
        var state = states[adventureForm.selectedIndex];
        state.show.forEach(show);
        state.hide.forEach(hide);
    }
    checkAdventureDropdown();
}());


There is definitely room for improvement, but I would argue that this is a step in the right direction.

Looking over the linked html, each possible value of adventureForm.selectedIndex corresponds to a meaning

Code Snippets

'use strict';
(function () {
    var elements;
    var i = 0;
    var isError = false;
    var errorMessage;
    var correctMessage = '✓';
    var errorSpan;
    ...
}());
function hide(element) {
    element.classList.add('hidden');
}

function show(element) {
    element.classList.remove('hidden');
}
function checkAdventureDropdown(option) {
    if (option.id !== 'adventure') {
        return;
    }

    if (adventureForm.selectedIndex === 0) {
        dropDowns.classList.add('hidden');
    } else if (adventureForm.selectedIndex === 1) {
        dropDowns.classList.remove('hidden');
        charterDrop.classList.remove('hidden');
        mountainDrop.classList.add('hidden');
        trampDrop.classList.add('hidden');
        photoDrop.classList.add('hidden');
    } else if (adventureForm.selectedIndex === 2) {
        dropDowns.classList.remove('hidden');
        charterDrop.classList.add('hidden');
        mountainDrop.classList.remove('hidden');
        trampDrop.classList.add('hidden');
        photoDrop.classList.add('hidden');
    } else if (adventureForm.selectedIndex === 3) {
        dropDowns.classList.remove('hidden');
        charterDrop.classList.add('hidden');
        mountainDrop.classList.add('hidden');
        trampDrop.classList.remove('hidden');
        photoDrop.classList.add('hidden');
    } else if (adventureForm.selectedIndex === 4) {
        dropDowns.classList.remove('hidden');
        charterDrop.classList.add('hidden');
        mountainDrop.classList.add('hidden');
        trampDrop.classList.add('hidden');
        photoDrop.classList.remove('hidden');
    }
}
var states = {
    0: { hide: [dropDowns], show: [] },
    1: {
        hide: [trampDrop, mountainDrop, photoDrop],
        show: [dropDowns, charterDrop]
    },
    2: {
        show: [mountainDrop],
        hide: [charterDrop, trampDrop, photoDrop]
    },
    3: {
        show: [trampDrop],
        hide: [charterDrop, mountainDrop, photoDrop]
    },
    4: {
        show: [dropDowns, photoDrop],
        hide: [charterDrop, mountainDrop, trampDrop]
    }
}

Context

StackExchange Code Review Q#142960, answer score: 2

Revisions (0)

No revisions yet.