JavaScript Event Handling - Select not changing

Viewed 55

I am attempting to use JavaScript to change the image that is displayed based on the option selected from a dropdown menu.

Using chrome monitorevents() I am able to tell that the change event is being triggered, however the variable direction is not being changed.

If I change the assigned value of direction manually, as you can see commented out, the correct image will display.

I'm sure it's something minor I'm overlooking, but I've been noodling at this pretty much all day.

I would appreciate anyone's input.


    <div class="dropdown">
        <label for="choose-lighting" id="windowDirection">Which direction does your window face?</label>

        <select name="lighting" id="choose-lighting">
            <option value="empty">--- Direction ---</option>
            <option value="north">North</option>
            <option value="south">South</option>
            <option value="east">East</option>
            <option value="west">West</option>
        </select>
    </div>

    <div id="displayBox">
        <img src="" id="onDisplay">
    </div>

    <script>
        // var direction = document.querySelector("select");
        // var onDisplay = document.getElementById("onDisplay")
        var select = document.getElementById("choose-lighting");
        var direction = select.options[select.selectedIndex].text;
        console.log(direction);
        // var direction = "south";

        function chooseLighting() {

            // switch (direction) {
            //     case "north":
            //         document.getElementById("onDisplay").src = "images/lightguide_north.jpg";
            //         break;
            //     case "south":
            //         document.getElementById("onDisplay").src = "images/lightguide_south.jpg";
            //         break;
            //     case "east":
            //         document.getElementById("onDisplay").src = "images/lightguide_east.jpg";
            //         break;
            //     case "west":
            //         document.getElementById("onDisplay").src = "images/lightguide_west.jpg";
            //         break;
            //     default:
            //         document.getElementById("onDisplay").src = "images/lightguide_genericweb.jpg";
            //         break;
            // }

            if(direction == "North") {
                document.getElementById("onDisplay").src = "images/lightguide_north.jpg";
            }

            else if(direction == "South") {
                document.getElementById("onDisplay").src = "images/lightguide_south.jpg";
            }

            else if(direction == "East") {
                document.getElementById("onDisplay").src = "images/lightguide_east.jpg";
            }

            else if(direction == "West") {
                document.getElementById("onDisplay").src = "images/lightguide_west.jpg";
            }

            else {
                document.getElementById("onDisplay").src = "images/lightguide_genericweb.jpg";
            }

        }

        if (window.addEventListener) {
            window.addEventListener("load", chooseLighting, false);
        }

        else if (window.attachEvent) {
            window.attachEvent("onload", chooseLighting);
        }

        if (document.getElementById("choose-lighting").addEventListener) {
            document.getElementById("choose-lighting").addEventListener("change", chooseLighting, false);
            console.log(direction);
        }

        else if (document.getElementById("choose-lighting").attachEvent) {
            document.getElementById("choose-lighting").attachEvent("onchange", chooseLighting);
            console.log(direction);
        }

    </script>
1 Answers

Your binding (the setting of a to be equal to b) of direction only occurs once: at the very start. You should instead access the option every time the event fires: inside the event-handler.

You should also use .value on the select, as opposed to options[index].text. This will give you the empty, north, south values that you entered in the options' value attributes instead of the string values, which may not be optimal for use in code. By using the value attributes, you can substitute them directly into your image paths.

With both of these optimizations, you could simplify your code down to the following:

const select = document.querySelector('#choose-lighting');
// const onDisplay = document.querySelector('#on-display');

const chooseLighting = () => {
    const direction = select.value;
    
    let path;
    if (direction == 'empty')
        path = 'images/lightguide_genericweb.jpg';
    else
        path = `images/lightguide_${direction}.jpg`;

    // onDisplay.src = path;
    console.log('Set the image src to', path);
}

select.addEventListener('change', chooseLighting, false);
window.addEventListener('DOMContentLoaded', chooseLighting, false);
<label for="choose-lighting">Which direction does your window face?</label>

<select name="lighting" id="choose-lighting">
    <option value="empty">--- Direction ---</option>
    <option value="north">North</option>
    <option value="south">South</option>
    <option value="east">East</option>
    <option value="west">West</option>
</select>

You could even make the function a one-liner if you really wanted to, but that's probably not the most readable.

const chooseLighting = () => onDisplay.src = `images/lightguide_${select.value == 'empty' ? 'genericweb' : select.value}.jpg`;

Edit: Following your question in the comment, here is the same thing implemented using more... traditional JavaScript.

var select = document.querySelector('#choose-lighting');

function chooseLighting() {
    var direction = select.value;
    
    if (direction == 'empty') {
        var path = 'images/lightguide_genericweb.jpg';
    } else {
        var path = `images/lightguide_${direction}.jpg`;
    }

    console.log('Set the image src to', path);
}

select.addEventListener('change', chooseLighting, false);
window.addEventListener('DOMContentLoaded', chooseLighting, false);
<label for="choose-lighting">Which direction does your window face?</label>

<select name="lighting" id="choose-lighting">
    <option value="empty">--- Direction ---</option>
    <option value="north">North</option>
    <option value="south">South</option>
    <option value="east">East</option>
    <option value="west">West</option>
</select>

I've even thrown in an intentional example of var being weird. You can see that, because var has function scope, I am able to perform path's declaration inside the if statement. In any other language, that would make path only valid until the end of that if or else's block. But not var: this oddness is part of why let and const have replaced var. The two new keywords behave as they would in any other language.

Related