Default image is not set

Question

I am using window.FileReader to offer an instant preview of the images that the user chooses to upload.

I have the following issues:

A) Code in line 63-77 seems to be ignored although when I print the value of "i" in line 25 I can see that is increasing (I am suspecting that it must have something to do with the nature of window.Filereader but not sure).

if (i == 0) //if this is the first picture add it as primary too 
{
    var primaryimage = ' <img id=' + filename + '  height="220" width="220" src=' + this.result + ' /> ';

    $('#primary-pic').find('.custom-input-file').hide();
    $('#primary-pic').find('p').hide();
    $('#primary-pic').append(primaryimage);
    $('.custom-input-file').show();
}

B) If you add more than one image and press "Set as default" for some reason this doesn't seem to work for the last image but no idea why .

Thanks

To begin with, your code is heavily nested and it seems that you miss some variable scope, such as i in your onloadend handler, which is the final value i gets by the time the function is invoked. In addition, you add the $(".setDefault:button") handler multiple times. I would use event delegation if I were you, so that the handler could be added only once to a surrounding element. — MasterAM
– MasterAM, Commented Jul 29, 2013 at 0:39

MasterAM · Accepted Answer · 2013-07-29 02:10:49Z

1

There were numerous thing wrong with your code.

Among them:

Deep nesting of one very long function, which was hard to read and debug.
Variable scope: you assumed that i is somehow 'frozen' when you create a handler. In fact, i read within the handler was the same i as in the main function:

In this example, i progresses and the value encountered by the function will be 4.

for (var i = 0; i < 4; i++) {
    var file = input.files[i];
    var reader = new FileReader();
    reader.onloadend = (function (file) {
        return function () {
            if (i == 0) { //actually, i will be 4
            }
        }
    })(file)
}

Some HTML errors.
The use of inline event handlers. I do not recommend using those.
The business logic and UI are tightly coupled. I would suggest creating an object that holds the image metadata and state, and let it construct the UI according to its state. It's a more robust architecture.

I have spent some time on fixing some of the issues. You can find my version here. I hope that you find it useful and learn from it.

answered Jul 29, 2013 at 2:10

MasterAM

16.5k6 gold badges47 silver badges68 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

Athanatos Over a year ago

Thanks for the detailed explanation and your time spent on this. It makes sense now.

Samurai · Accepted Answer · 2013-07-29 00:47:44Z

0

1) Those lines will never reach because if(i == 0) then input.files.length == 1 returns true and so whenever it goes to the else section (2 lines above) i is actually bigger than 0.

2) I think the problem is you're creating multiple elements which share the same id (img and button). Id must be unique.

answered Jul 29, 2013 at 0:47

Samurai

3,7295 gold badges29 silver badges41 bronze badges

Comments

jdog · Accepted Answer · 2013-07-29 00:56:57Z

Hey there I believe I see your problem. You are using the onloadend event from the FileReader object in a loop. This is an asynchronous function, that is JavaScript will not wait for this function to execute before moving onto the next iteration of the loop. As such some of your logic depends on the onloadend function having been executed already for the image in question however in your current setup this is unreliable, the i var might be pointing to the next iteration of the loop already however the onloadend function may still be executing for the previous loop and using the wrong i value. This will mess up the rest of your logic as well. As for issue b) I didn't fully dig into this one however I noticed you declared the click event for this inside of the loop, so some of the images had multiple instances of this event attached to them. My advice, wait for ALL your images to load, then inject any extra HTML, then setup any events you require (i.e. the click event). I'll bet issue b) will sort itself out if you do this :)

Collectives™ on Stack Overflow

Default image is not set

3 Answers 3

1 Comment

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

1 Comment

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related