0

I'm trying to add multiple elements to a list and each element should execute the same on click function with different parameters, the problem is the variable x gets always contains the same value for all elements of the list.

How can I add elements and call the onclick event with a different parameter?

var addQuickLabelList =  function(txtList,ul) {


for (i = 0; i<txtList.length ; i++) {
        var li = document.createElement("li");
        li.setAttribute("data-icon", "false");

        var a = document.createElement("a");        
        a.innerHTML = txtList[i];
        li.appendChild(a);

        var x = "#"+txtList[i];

        a.addEventListener("click", function(){
            y = x.clone();
            alert(x);
            } , false);// add

        $(ul).append(li);
    }
};
kenfire
  • 1,305
  • 12
  • 23
Hector Franco
  • 23
  • 1
  • 7
  • `$(ul).append(li)` are you using jQuery? – j08691 Aug 28 '17 at 17:35
  • Sorry for the mistake, the line on click event should say: alert(x); nothing about cloning – Hector Franco Aug 28 '17 at 17:35
  • 1
    j08691 is right, if you're using jQuery why use native JS DOM methods like `createElement`. jQuery will do that too. – Andy Aug 28 '17 at 17:38
  • Possible duplicate of [JavaScript closure inside loops – simple practical example](https://stackoverflow.com/questions/750486/javascript-closure-inside-loops-simple-practical-example) – yuriy636 Aug 28 '17 at 17:47

1 Answers1

0

x always gets the same value because all your event handlers share the same var x variable. To scope a variable to a block, use let (or const if it won't change) instead of var.

Or you could use .forEach() on the txtList Array so that the var is scoped to the invocation of the callback.

var addQuickLabelList =  function(txtList,ul) {
    txtList.forEach(function(txtItem) {
        var li = document.createElement("li");
        li.setAttribute("data-icon", "false");

        var a = li.appendChild(document.createElement("a"));        
        a.innerHTML = txtItem;

        var x = "#"+txtItem;

        a.addEventListener("click", function(){
            console.log(x);
        } , false);

        ul.appendChild(li);
    });
};

But you also don't really even need the x variable. You already set the text as the content of the a, so you can just grab that instead. Which means you could also reuse the function, which is nicer.

function handler() {
   console.log("#" + this.textContent);
}

var addQuickLabelList =  function(txtList,ul) {
    txtList.forEach(function(txtItem) {
        var li = document.createElement("li");
        li.setAttribute("data-icon", "false");

        var a = li.appendChild(document.createElement("a"));        
        a.innerHTML = txtItem;

        var x = "#"+txtItem;

        a.addEventListener("click", handler, false);

        ul.appendChild(li);
    });
};
spanky
  • 2,768
  • 8
  • 9