1

I have been working on previewing multiple images before they are uploaded and tried using @ratan-paul response to the "Preview an image before it is uploaded" quite useful. However when I tried to tweak the code a bit I got a rather unexpected result. What I did was to create a <span> tag for each <image> to add some styling options. Here's the jQuery code I ended up with:

$(function () {
    var span_id = "";
    function readURL(input) {
        for(var i =0; i< input.files.length; i++){
            var span = $('<span>');
            span_id = "span"+i;
            span.attr('id',span_id);
            span.appendTo('#img-previews');
            if (input.files[i]) {
                var reader = new FileReader();
                reader.onload = function (e) {
                    var img = $('<img id="dynamic">');
                    img.attr('src', e.target.result);
                    img.attr('height','150px');
                    img.attr('width','200px');
                    img.appendTo("#"+span_id);
                };
                reader.readAsDataURL(input.files[i]);
            }
        }
    }

    $("#imgUpload").change(function(){
        readURL(this);
    });
});

Now the strange thing is that when I run the code above I create a <span> tag for each image but all the images get crammed into the last <span> created. On the other hand if i debug the script I get the wanted result where each image is inserted into its own <span> tag. I'm kinda baffled by this so if anyone has any ideas as to why this is happening or how to avoid it would appreciate the assistance.

Here is a screen grab of when I run the script without debugging: Screen grab of unexpected behavior

And here is a screen grab of the result when stepped through with the js debugger Result when stepping through a JS debugger

natral
  • 775
  • 1
  • 12
  • 32

1 Answers1

1

Short story:

Change your loop to

for(let i =0; i< input.files.length; i++){

let keyword in the for loop

Note, let in the for loop may not work as one would expect for IE11 and Edge. You could move the creation of your span into the reader.onload event to adjust for those browsers (your generation of id's will have to change though).

Long story:

using var i =0 instantiates i over the entire loop, whereas let i =0 binds it to the current iteration.

What's happening is that the for loop fires off the async call reader.onload which is added to the JS event loop for future resolution, and the for loop proceeds to its next iteration. Important bit: At the next iteration, the previous reader.onload had not resolved yet.

Sometime later, the reader.onload async calls return after the for loop is completed (where i is at the max value for the loop), and appends the resultant imgs to #span_(max i).

Update: OK, so let may not be supported everywhere, caniuse puts it at 80% as of July 2017.

For consistent results, move the creation of the span element into the onload event:

function readURL(input) {
    for(var i =0; i< input.files.length; i++){
        if (input.files[i]) {
            var reader = new FileReader();
            reader.onload = function (e) {
                var span = $('<span>');
                var uniqueId;
                do {  //simple loop to generate a unique id
                  uniqueId = 'img-' + (Math.random()+1).toString(36).substring(2, 7);
                } while ($('#' + uniqueId).length > 0); //redo loop if id not unique
                span.attr('id', uniqueId);
                span.appendTo('#img-previews');

                var img = $('<img>');
                img.attr('src', e.target.result);
                img.attr('height','150px');
                img.attr('width','200px');
                span.append(img);
            };
            reader.readAsDataURL(input.files[i]);
        }
    }
}

Note that the creation of these span elements may end up being out of order; if you'd want them in the order as they were added, have a look at using Promise.all (or the jQuery version).

Also note, I haven't tested the above so some massaging may be needed.

duncan smith
  • 91
  • 1
  • 4
  • I tried switching my for loop with the let statement @duncan-smith but results did not change. In my IDE (phpstorm 2017.1.4) 'let' is indicated as being an error saying "let definitions are not supported in the current by the current JavaScript version". I'm working with the yii2 framework and the current jquery version installed is 2.2.4. The browsers i'm currently testing on are mozilla firefox 54.0 (32bit), firefox developer edition 55.0b6 (64-bit), and chrome 59.0.3071.115 (Official Build) (64-bit). any idea as to what is the problem? – natral Jul 07 '17 at 14:36
  • @natral, I've updated my answer. check out if it works for you. – duncan smith Jul 09 '17 at 21:39
  • Great that helps a lot @duncan. Thanks! – natral Jul 10 '17 at 13:13