2

I'm writing a routine in Javascript that iterates through all audio elements in a HTML page. All elements are marked with ID in the form of 'track_[nr]' and each one has a customized button which toggles play/pause marked with ID 'control_[nr]'.

Now I want to automate an onclick event on each control element by iterating through all elements and assigning a onclick function.

I came up with following code, but the "for" loop doesn't behave like I'd expect.

"document.writeln(j)" always prints 5 (i have currently 5 audio elements), no matter on which control element i click. I would expect it to write "0" when you click on "control_0", "1" when clicking on "control_1", etc.

Any help is much appreciated!

<script type='text/javascript'>
    var audio = new Array();
    var ctrl = new Array();
    var i = 0;

    do {
        audio[i] = document.getElementById('track_'+i), ctrl[i] = document.getElementById('control_'+i);
        i++;
    } while(audio[i-1]);
    tracks=i-1;

    for (var j = 0; j < tracks; j++) {
        ctrl[j].onclick = function () {
            document.writeln(j);
        }
    }
</script>

Regards

vitaminx
  • 27
  • 1
  • 4

4 Answers4

1

First, you have 'control_' not 'ctrl_' in the JS! However, your main problem is that the onclick function is a closure which means that it doesn't use the value of j in its definition, it uses the actual variable j which is changed by the for loop. You need to create a function that takes j as a parameter and returns the handler. This works because the value of j is passed into the function.

var audio = new Array();
    var ctrl = new Array();
    var i = 0;

    do {
      audio[i] = document.getElementById('track_' + i), ctrl[i] = document.getElementById('ctrl_' + i);
      i++;
    } while (audio[i - 1]);
    tracks = i - 1;

    function makeHandler(j) {
      return function() {
        alert(j);
      };
    }
    for (var j = 0; j < tracks; j++) {
      ctrl[j].onclick = makeHandler(j);
    }
button {
  display: block;
}
<button id="ctrl_0">Button 0</button>
<button id="ctrl_1">Button 1</button>
<button id="ctrl_2">Button 2</button>
<button id="ctrl_3">Button 3</button>
<button id="ctrl_4">Button 4</button>
<button id="ctrl_5">Button 5</button>

<div id="track_0">placeholder 0</div>
<div id="track_1">placeholder 1</div>
<div id="track_2">placeholder 2</div>
<div id="track_3">placeholder 3</div>
<div id="track_4">placeholder 4</div>
<div id="track_5">placeholder 5</div>
vitaminx
  • 27
  • 1
  • 4
Harold Ship
  • 989
  • 1
  • 8
  • 14
  • I've just added this handler to my code and it works perfectly, thanks! :) About the "control_" vs. "ctrl_": That's a typo, the element ID's are actually "control_" in the HTML part of the code. I'm going to change that in the OP. – vitaminx Mar 23 '15 at 12:27
  • @vitaminx that's great! – Harold Ship Mar 23 '15 at 12:33
0
ctrl[j].onclick = function () {
    document.writeln(j);
}

what this piece of code does is, it assigns a function that says write the value of j, to the onclick event listener of the element

once the for loop is finished it would have assigned the function to corresponding elements and the value of j will be equal to the number of elements in the page.

When the actual function is executed. It prints the value of "j".

To acheive the desired result, you will need to use closures. take a look at this answer

Community
  • 1
  • 1
Anbarasan
  • 1,197
  • 13
  • 18
0

Another approach: You could also store the trackId in a data attribute on the DOM element itself.

for (var j = 0; j < tracks; j++) {
    ctrl[j].setAttribute("data-trackId", j);
    ctrl[j].onclick = function(event) {
        console.log(event.toElement.getAttribute("data-trackId"));
    }
}
vitaminx
  • 27
  • 1
  • 4
nuala
  • 2,681
  • 4
  • 30
  • 50
0

Change your for-loop to:

 for (var j = 0; j < tracks; j++) {
    ctrl[j].onclick = function (index) {
        return function(){ alert(index); }
    }(j);
 }

You create a new function and assign it to each element. Otherwise all values would be 5 because of closures.

vitaminx
  • 27
  • 1
  • 4
maxeh
  • 1,373
  • 1
  • 15
  • 24