locked
Click event not assigning correctly in loop

    Question

  • Hi all,

    I'm trying to add an onclick event to each div that has a certain class name - I want each div to load a different file into a player for me, but for some reason they all load the last file in the array.

    function ActivateTracks() {
        allTracks = document.getElementsByClassName("track");
        var track = Windows.Storage.StorageFile;
    
        for (var k = 0; k < allTracks.length; k++) {
            track = allFiles[k];
            allTracks[k].addEventListener("click", function () { Load(track) });
        }
    }
    
    function Load(file) {
        var player = document.getElementById("player");
        player.src = URL.createObjectURL(file);
    }

    So basically - I have an array of files and an array of divs. I want the OnClick event of each div to load a different file - but once this is all complete, they all load the last file in the array. So no matter what one I click on, it'll load the last file that was in the file array - even though as far as I know I am assigning a different one to each div.

    Any help would be much appreciated.

    Thanks.

    Wednesday, December 3, 2014 11:50 PM

Answers

  • Hi, 

    There's probably some code omitted from the snippet you provided (for reading the files), but based on what I can see, I'd guess the problem is that you are assigning a reference to the same object for all click event handlers. Essentially, the variable 'track' is defined outside the loop scope and thus is the same var all the time, you are just re-assigning a value to it on each iteration loop - and then each call to Load() uses this same 'track' variable to launch the file, making all calls to launch the file set to 'track' on the last loop iteration.

    There's several approaches to fix this, but I'd personally do maybe as follows:

        // assuming that allFiles is an array containing StorageFile objects and that it's of equal length as allTracks array
    
        for (var k = 0; k < allTracks.length; k++) {
            // Using function.bind() to bind a reference to the file array and running index to the Load() function. bind() returns a new function.
            allTracks[k].addEventListener("click", Load.bind(null, allFiles, k));
        }
    }
    
    // This gets a reference to the array and an index - allows having only one array of file objects instead of that array + creating a copy of each file object inside the event handler closure 
    function Load(fileArray, index) { // the third parameter to this function will be the actual click event object
        var file = fileArray[index];
        var player = document.getElementById("player");
        player.src = URL.createObjectURL(file);
    }

    Hope this helps (didn't run the code, pardon me for potential whoops's there). This is a quite common gotcha with JavaScript scopes, some reading about the issue can found from, for instance, here http://blogs.msdn.com/b/oldnewthing/archive/2014/06/05/10531181.aspx

    Wednesday, December 10, 2014 7:36 PM

All replies

  • Hi Octodonk,

    Do you have some repro for us to test? Simply read from your code, looks good, the code told me you would play different file in the player while you click different div.

    However I think if you set a breakpoint somewhere in the Load() function, you should be able to see what is current file and be able to see if the correct file has been assigned to the player.

    --James


    We are trying to better understand customer views on social support experience, so your participation in this interview project would be greatly appreciated if you have time. Thanks for helping make community forums a great place.
    Click HERE to participate the survey.

    Thursday, December 4, 2014 7:15 AM
    Moderator
  • Hi, 

    There's probably some code omitted from the snippet you provided (for reading the files), but based on what I can see, I'd guess the problem is that you are assigning a reference to the same object for all click event handlers. Essentially, the variable 'track' is defined outside the loop scope and thus is the same var all the time, you are just re-assigning a value to it on each iteration loop - and then each call to Load() uses this same 'track' variable to launch the file, making all calls to launch the file set to 'track' on the last loop iteration.

    There's several approaches to fix this, but I'd personally do maybe as follows:

        // assuming that allFiles is an array containing StorageFile objects and that it's of equal length as allTracks array
    
        for (var k = 0; k < allTracks.length; k++) {
            // Using function.bind() to bind a reference to the file array and running index to the Load() function. bind() returns a new function.
            allTracks[k].addEventListener("click", Load.bind(null, allFiles, k));
        }
    }
    
    // This gets a reference to the array and an index - allows having only one array of file objects instead of that array + creating a copy of each file object inside the event handler closure 
    function Load(fileArray, index) { // the third parameter to this function will be the actual click event object
        var file = fileArray[index];
        var player = document.getElementById("player");
        player.src = URL.createObjectURL(file);
    }

    Hope this helps (didn't run the code, pardon me for potential whoops's there). This is a quite common gotcha with JavaScript scopes, some reading about the issue can found from, for instance, here http://blogs.msdn.com/b/oldnewthing/archive/2014/06/05/10531181.aspx

    Wednesday, December 10, 2014 7:36 PM