-1

I'm new to Javascript and I'm trying to call a function inside another function inside a for loop in Javascript. I read all the answers about closures and scope in JS loops but still can't manage to make it work. Basically what I'm trying to do is attach an event listener to every element of an array.

Here's my code:

HTML:

    <div class="todo-item">
       To Do item #1
       <button class="btn btn-success btn_task" data-name="data1">Done</button>
    </div>

    <div class="todo-item">
       To Do item #2
       <button class="btn btn-success btn_task" data-name="data2">Done</button>
    </div>

    <div class="todo-item">
       To Do item #3
       <button class="btn btn-success btn_task" data-name="data3">Done</button>
    </div>

JS function that updates HTML through AJAX:

    function showTodo(str, label) {

        var xmlhttp = new XMLHttpRequest();
        xmlhttp.onreadystatechange = function() {

            if (this.readyState == 4 && this.status == 200) {
                document.getElementById("todo").innerHTML = this.responseText;
            }

        };
        xmlhttp.open("GET", "../datasource.php?lang=" + label + "&data=" + str, true);
        xmlhttp.send();

}

The part that doesn't work:

var todoElements = document.getElementsByClassName("btn_task");

for (var i = 0; i < todoElements.length; i++) {

        (function(index) {
                todoElements[index].addEventListener("click", function(){
                var attribute = todoElements[index].getAttribute('data-name');
                showTodo(attribute, "Spanish");
            })
        })(i);

    }
tomschmidt
  • 382
  • 1
  • 2
  • 12

2 Answers2

1

There is no reason to use a IIFE here. To bind to all the buttons, you just need a simple for loop.

Inside the event handler, you can use this to get the element you clicked on.

var todoElements = document.getElementsByClassName("btn_task");

for (var i = 0; i < todoElements.length; i++) {
    todoElements[i].addEventListener('click', function(){
        showTodo(this.getAttribute('data-name'), "Spanish");
    });
}
gen_Eric
  • 223,194
  • 41
  • 299
  • 337
0

try this instead (ES6):

function getAttr(e){
    const name = this.dataset.name;
    showTodo(name, "Spanish");
}

const buttons = document.querySelectorAll('.btn_task');

buttons.forEach(button => button.addEventListener('click', getAttr));

What we do here is get all the buttons, assign each button a click handler called getAttr. getAttr is passed the button elment that was clicked, it looks at the dataset (all the properies of the element that have the "data-') and selects the data-name property. It passes this to your function.

Ben Davison
  • 713
  • 7
  • 15
  • Chances are that the OP does not know what ES6 even is. Besides, browser support for ES6 native syntax is not wide-spread and consistent enough to make general recommendations for it. – Tomalak Jan 17 '17 at 15:40
  • I was looking to avoid ES6 as many of my users are on older browsers. Thanks though. – tomschmidt Jan 17 '17 at 15:42
  • @Tomalak what is not widespread about the [support](http://kangax.github.io/compat-table/es6/) unless you are on IE11 ofcourse. – Ben Davison Jan 17 '17 at 15:49
  • @BenDavison Yes, it's always not an issue unless you care about browsers that don't support it. That's self-evident. The point is, developer vanity is not a good reason to break it for IE11 users. Nothing substantial can be gained from using arrow functions here, so the sensible thing to do is not to use them. `querySelectorAll` and `Array#forEach` are in fact supported, even though I would default to jQuery for this anyway. – Tomalak Jan 17 '17 at 16:02