javascript set onclick in dynamically generated element

Question

I have an object that I want to loop over to create elements. Each element should have an onclick that changes the text of another element (It's for a dynamically generated drop-down).

    const orders = {
        INDEX: 'index',
        REVERSE_INDEX: 'reverse index',
        RANDOM: 'random',
        SELECTION: 'selection order',
        REVERSE_SELECTION: 'reverse selection order',
        CURRENT_ORDER: 'current start order',
        REVERSE_CURRENT_ORDER: 'reverse current start order',
        ALPHABETICAL: 'alphabetical',
        REVERSE_ALPHABETICAL: 'reverse alphabetical'
    };

    var ddc = document.getElementById("orderDropDownContent");

    for (var key in orders) {
        var ddMenuItem = document.createElement("p");
        ddMenuItem.className = "dropdown-item";
        ddMenuItem.onclick = function() { 
            document.getElementById("orderDropDown").innerText = orders[key];
        };
        ddMenuItem.innerText = orders[key];
        ddc.appendChild(ddMenuItem);
    }

The html is just

<div class="dropdown">
    <div id="orderDropDown" value="index" class="dropbtn">index</div>
    <div id="orderDropDownContent" class="dropdown-content">
    </div>
</div>

The inner text works for each element, and each gets an onclick, but the problem is that the onclick function doesn't use the value of orders[key] from when it is created, it uses the expression orders[key] which equates to the last value in the object 'reverse alphabetical'. How do I create the function with the string value at the time when it was created.

I tried adding a line

        var theText = '' + orders[key];

But that didn't work.

I feel like this has probably been asked before, but I can't find a duplicate.

Ahh, got it, thanks. I deeply don't get scope in JS. – stib May 25 '20 at 04:07 — stib, May 25 '20 at 04:07

score 2 · Accepted Answer · answered May 25 '20 at 04:13

Var is scoped to the nearest function, so when you go through your for loop and add the event listeners, by the time the function is actually called, which is later on, key will always be the last value from when you looped through it. Couple ways to fix this, easiest is to use let which is block scoped instead of var. Best practice is to always use const where possible, and let everywhere else, and avoid using var.

JavaScript closure inside loops – simple practical example

Ah. 'Closure' was what I should have been looking for. – stib May 25 '20 at 04:29 — stib, May 25 '20 at 04:29

smartdroid · Answer 2 · 2020-05-25T04:36:20.707

The issue is by the time your element creation loop is over, the variable key points to the last item in orders, i.e. REVERSE_ALPHABETICAL.

So when you are calling on click functions they always get the last item via key.

One way to resolve this will be to store the current value of key in the menu item element when you create each using setAttribute.


ddMenuItem.setAttribute(“orderIndex”,key);

Then in the on click function, use this.getAttribute(“orderIndex”) to get the correct index of orders.

This will work on old versions of browsers that do not support let.

javascript set onclick in dynamically generated element

2 Answers2