2

I have an un-ordered HTML list of images. Each Line (li) element has its own id:

<ul id="Demo3">
    <li id="li1"><img src="images/FT/HopkinsPrairie.jpg" /></li>
    <li id="li2"><img src="images/FT/PineForest.jpg" /></li>
    <li id="li3"><img src="images/FT/2011GroupThumb.jpg" /></li>
    <li id="li4"><img src="images/FT/JuniperSpringsSwimming.jpg" /></li>
    <li id="li5"><img src="images/FT/HappyHourThumb.jpg" /></li>
    <li id="li6"><img src="images/FT/HappyHourThumb.jpg" /></li>
</ul>

When I click each image I would like a larger version of the image to appear, sort of like this:

<li id="li2"><a href="images/FT/PineForestBig.jpg" target="_blank"><img src="images/FT/PineForest.jpg" /></a></li>

Unfortunately the CSS class associated with #Demo3 does a lot of manipulation to the class associated with all #li elements, so I have to add the anchor in programmatically.

I try doing it like this:

i = 1;
while ($("#li" + i).length > 0) {  // If the li id exists
    myLink = "images/FT/Page" + i + ".jpg";  // create a link 
    var element = document.getElementById("li" + i);  // get the element
    element.onclick = function () {          // create an onclick for this element
        var win = window.open(myLink, '_blank');
        win.focus();
    }
    i++;
}

This does not work! For whatever reason, each image gets the same onclick as the last one in the list.

Does anyone know what I'm doing wrong or how to solve this so that each image contained within each li element gets its own onclick?

Any other suggestions?

rnrneverdies
  • 15,243
  • 9
  • 65
  • 95
Steve Silberberg
  • 277
  • 3
  • 18
  • does changing your while argument to `while ($("#Demo3 li").length > 0)` change anything? To me it looks like you are just looping through `#li1` – Shan Robertson Dec 17 '14 at 01:19
  • @Amadan no, this jquery question have another approach. – rnrneverdies Dec 17 '14 at 01:20
  • 1
    This question is closed against the wrong duplicate as explained [in this meta post](http://meta.stackoverflow.com/questions/280498/false-duplicate) – rene Dec 17 '14 at 09:51

2 Answers2

2

Another Approach, Use Wildcard selectors:

$("[id^=li").each(function(index, value) {
    value.click(function () {
       var id = parseInt(value.id.substring(2)); // strip "li", convert to int.
       var win = window.open("images/FT/Page" + id + ".jpg", '_blank');
       win.focus();
    });
});

or simply

$("[id^=li").click(function (event) {
    var id = parseInt(this.id.substring(2)); // strip "li", convert to int.
    var win = window.open("images/FT/Page" + id + ".jpg", '_blank');
    win.focus();
});
Community
  • 1
  • 1
rnrneverdies
  • 15,243
  • 9
  • 65
  • 95
0

This does not work! For whatever reason, each image gets the same onclick as the last one in the list.

The direct solution for you is this:

while ($("#li" + i).length > 0) {  // If the li id exists
    (function(i) { // <---- THIS LINE
        var myLink = "images/FT/Page" + i + ".jpg";  // create a link 
        var element = document.getElementById("li" + i);  // get the element
        element.onclick = function () {          // create an onclick for this element
            var win = window.open(myLink, '_blank');
            win.focus();
        }
        i++;
    })(i); // <---- and THIS LINE
}

The reason is, myLink was being captured in your onclick closure. Your loop went a number of times, each time changing myLink and creating a closure on myLink. When the onclick triggered, the myLink value is the last value it had at the end of the loop - not at the time the closure was created.

The approach above, using an IIFE, "fixes" the value of i, and creates a separate variable myLink in each iteration for the onclick handler to close over.

The alternative approach by OneWay does this by directly using another closure as the callback of each. I'd use his solution in your code, and this answer to help you understand why your original behaved as it did.

Amadan
  • 191,408
  • 23
  • 240
  • 301