Adding jQuery onClick handler using forEach

Question

I'm having a hard time figuring out what kind of issue I'm running into with this piece of code. This code takes an array of links that are gathered via Ajax. Each time a user searches a new term, linksArr is populated with new links, and the link buttons will open the new set of links.

populateLinks = function (linksArr){
    // linksArr contains links from Ajax search
    linksArr.forEach(function(link,index){
        // Add on click handler to each link button
        $('.linkButton' + [index]).on("click", function(){
            window.open(link, "_blank");
        })
    });
};

The first set of links works perfectly fine. The problem comes after any subsequent array of links passed into the function. When a new array of links is passed into the function, once you click a link button, window.open() will reference and open the first array of links that was passed to linksArr, and then immediately after, it will open up the current links that were most recently passed into the function.

I'm quite confused what the problem could be. I believe it could be an issue with closure due to forEach function being used but, after a few hours I wasn't able to figure it out.

This looks to be a seemingly a simple problem and maybe I've overlooked something. I would love to understand what exactly is happening in regards to the links being opened by window.open, and why the previous links that were passed earlier (they aren't even in linksArr when new links populate it) are being opened up before the current, most recently passed in links in linksArr.

Why aren't you just using `` tags with target in them? The reason it is happening is indexing is array based and if you pass more than one array in the indexing will start from zero each time. Provide a [mcve] — charlietfl, Nov 25 '17 at 18:34
`$("#linkContainer").on("click",".linkClass",function() { window.open(this.href,"_blank"); });` — mplungjan, Nov 25 '17 at 18:39
Don't use `linkButton0` .. `linkButtonN` as a class. Classes are meant to be, well, *classes*. Use something like `linkButton` as a class. If you must, use `linkButton0` .. `linkButtonN` as an ID, not a class. — Anthony Mills, Nov 25 '17 at 18:42

score 1 · Answer 1 · answered Nov 25 '17 at 18:39

1

on doesn't replace the click handler. It adds a click handler.

You might want to say .off("click").on("click", function () { ... }) instead.

answered Nov 25 '17 at 18:39

Anthony Mills

8,676
4
32
51

Adding jQuery onClick handler using forEach

1 Answers1