Question

Im have a jquery code that running in loop over my images and load them one by one, All works fine beside the fact that it inserting to the 'a' tag the last image in results! Here is my code (Please take a look in "var a"... it should insert the variable 'thissrc' differently every loop:

$.get(url, function (data) {
    var countImages = data.length;
    console.log("count: " + countImages);
    var $ul = $('#thumbs').empty();
    var counter = 0;
    for (var i = 0; i < countImages; ++i) {
        try {
            var description = data[i].desc[0];
        } catch (e) {
            description = '';
        }
        if (description == undefined) description = '';
        var thissrc = data[i].src;
        console.log("ME: " + thissrc);
        $('<img width="' + screen.width / 2.4 + '" alt="' + data[i].alt + '" title="' + description + '"/>').load(function () {
            ++counter;
            var $this = $(this);
            var $li = $('<span/>', {
                className: 'pic'
            });
            var $a = $('<a/>', {
                href: '#',
                    'id': 'rel',
                    'data-inline': true,
                    'data-transition': 'slide',
                className: 'show-page-loading-msg',
                    'data-referrer': 'photo_container',
                    'data-imgsrc': thissrc
            });
            $ul.append($li.append($a.append($this)));
            if (counter == countImages) {
                $thumbscontainer.append($ul.show());
            }
        }).attr('src', data[i].src);

    }
}, 'json');

Thanks in Advance!

Eran.

Was it helpful?

Solution

You should be able to set data-imgsrc by reading back the image's src, ie :

'data-imgsrc': this.src

This will be reliable assuming no other code has had the opportunity to change the src in the short while after the img was created and before its initial src successfully loads.

If this is not a safe assumption then the solution is somewhat more elaborate but not at all uncommon.

OTHER TIPS

A common mistake. The problem boils down to:

for (var i = 0; i < countImages; ++i) {
    var thissrc = data[i].src;
    setTimeout(function() {
        // Will only ever alert the last value of thissrc
        alert(thissrc);
    }, 100);
}

In JavaScript, a for construct (or any regular block statement) does not create a new lexical scope. That is, each iteration uses the same variable thissrc. The above snippet is equivalent to:

var thissrc;
for (var i = 0; i < countImages; ++i) {
    thissrc = data[i].src;
    setTimeout(function() {
        alert(thissrc);
    }, 100);
}

In fact, every variable declaration (using var) inside a function is scoped to that function.

MDN explains the problem really well and provides a solution using an extra function. In this case, it could look like:

$.get(url, function (data) {
    // ...
    for (var i = 0; i < countImages; ++i) {
        // ...
        var thissrc = data[i].src;
        // ...
        // Make a new callback for $.load using the *current* value of thissrc
        $.load(url, makeCallback(thissrc));
    }
    // ...

    function makeCallback(thissrc) {
        // Yes, this function returns another function
        return function() {
             // Do your callback stuff here
        };
    }
}

Note that because makeCallback is defined inside $.get's callback function, it has access to all those local variables such as counter and description. Those variables are in the scope of makeCallback.

JavaScript 1.7 makes this a bit easier with the let keyword:

let allows you to declare variables, limiting its scope to the block, statement, or expression on which it is used. This is unlike the var keyword, which defines a variable globally, or locally to an entire function regardless of block scope.

That means that you could fix your problem with just one change:

let thissrc = data[i].src;

Unfortunately, as this is a fairly recent addition, it's not supported in old browsers.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top