Passing image array values and changing image source onclick

Question

OK, so finally the penny dropped (loud clunk!) on the click issue I was having here Append dynamic div just once and a JSFiddle issue. The code now shows user a choice of pics once per node clicked. Phew.

However, now my img.src=e.target.src line is having trouble accessing the other images in the array. Only the last image in the array will add to the table. I think this is because the allImages.onclick event should be inside the loop?

I have tried that and then img is showing up as undefined. I'm guessing that is because the loop (and therefore the function) is running before var img is declared? I think it is an issue with the order of things.

All help appreciated.

var makeChart = function () {
    var table = document.createElement('table'),
    taskName = document.getElementById('taskname').value,
    header = document.createElement('th'),
    numDays = document.getElementById('days').value, //columns
    howOften = document.getElementById('times').value, //rows
    row,
    r,
    col,
    c;

    var myImages = new Array();
    myImages[0] = "http://www.olsug.org/wiki/images/9/95/Tux-small.png";
    myImages[1] = "http://a2.twimg.com/profile_images/1139237954/just-logo_normal.png";
    for (var i = 0; i < myImages.length; i++) {
        var allImages = new Image();
        allImages.src = myImages[i];

        var my_div = document.createElement("div");
        my_div.id = "showPics";
        document.body.appendChild(my_div);
        var newList = document.createElement("ul");
        newList.appendChild(allImages);
        my_div = document.getElementById("showPics");
        my_div.appendChild(newList);
        my_div.style.display = 'none';
    }
    header.innerHTML = taskName;
    table.appendChild(header);

    header.innerHTML = taskName;
    table.appendChild(header);

    function addImage(col) {
        var img = new Image();
        img.src = "http://cdn.sstatic.net/stackoverflow/img/tag-adobe.png";
        col.appendChild(img);
        img.onclick = function () {
            my_div.style.display = 'block';

            allImages.onclick = function (e) { // I THINK THIS IS THE PROBLEM
                img.src = e.target.src;
                my_div.style.display = 'none';
                img.onclick=null;
            };
        }

    }

    for (r = 0; r < howOften; r++) {
        row = table.insertRow(-1);
        for (c = 0; c < numDays; c++) {
            col = row.insertCell(-1);
            addImage(col);
        }
    }

    document.getElementById('holdTable').appendChild(table);
    document.getElementById('createChart').onclick=null;
}

score 1 · Answer 1 · edited Jan 05 '13 at 15:21

Your addImage() function makes a direct reference to the allImages variable. One problem is that since you were using (and reusing) that variable in a for loop earlier in the code it is only going to retain the last value that was assigned to it. So no matter how many times you call addImage() it's always adding the onclick function to the last image that allImages pointed to.

I'd also suggest renaming the allImages variable. That's a very misleading name because it in fact only ever represents a single image.

Hope that helps!

score 1 · Accepted Answer · answered Jan 05 '13 at 15:10

Well, the problem seems to stem from different parts. First of all,

for (var i = 0; i < myImages.length; i++) {
    var allImages = new Image();
    allImages.src = myImages[i];

    var my_div = document.createElement("div");
    my_div.id = "showPics";
    document.body.appendChild(my_div);
    var newList = document.createElement("ul");
    newList.appendChild(allImages);
    my_div = document.getElementById("showPics");
    my_div.appendChild(newList);
    my_div.style.display = 'none';
}

This loop creates a new div for EACH image in myImages, then appends a ul to that div, and finally appends the Image for the current image to the ul.

The question of what document.getElementById('showPics') returns, since there are as many divs with the id showPics appended to body as myImages.length, has a mystical magical answer which should never be spoken, or even thought, of again.

Why not do the sensible thing and create one singular happy div outside the loop? Append a single ul child to it, outside the loop. Then proceed to append as many li as you want in the loop.

var my_div = document.createElement('div');
my_div.id = 'showPics';
var newList = document.createElement('ul');
my_div.appendChild(newList);

for var i = 0; i < myImages.length; i++) {
    ...
    var li = document.createElement('li');
    li.appendChild(allImages);
    newList.appendChild(li);
    ...
}

my_div.style.display = 'none';

Now, my_div is the one and only div containing the images. So, the click event handlers can toggle its visibility safely.

Second,

function addImage(col) {
    var img = new Image();
    img.src = "http://cdn.sstatic.net/stackoverflow/img/tag-adobe.png";
    col.appendChild(img);
    img.onclick = function () {
        my_div.style.display = 'block';

        allImages.onclick = function (e) { // I THINK THIS IS THE PROBLEM
            img.src = e.target.src;
            my_div.style.display = 'none';
            img.onclick=null;
        };
    }
}

allImages references the same Image object now that you are out of the loop, which happens to be the last image in myImages. So, only the last image in myImages will register the handler to a click event. To solve this problem, we make a new variable.

var sel = null; //This comes before my_div

Now, we add the click handler to allImages inside the loop so that every image in myImages gets a piece of the pie, as they say.

for var i = 0; i < myImages.length; i++) {
    var allImages = new Image();
    allImages.src = myImages[i];

    allImages.onclick = function (e) {
      if(sel !== null) {
        sel.src = e.target.src;
        my_div.style.display = 'none';
        sel.onclick=null;
        sel = null;
      }
    };
    ...
}

And finally, adjust addImage so that sel can be set when the image is clicked.

function addImage(col) {
    ...
    img.onclick = function () {
        my_div.style.display = 'block';
        sel = img;
    }
    ...
}

That's all there is to it! Example.

Note that, if you comment out sel.onclick = null, you can change a particular cell's image as many times you like.

That's great Rikonator. Thanks for your help. It was driving me crazy. It is good to try these things for learning purposes but when you're stumped it just becomes frustrating and turns you off the subject. I can see I was near yet quite far from the working solution. — Inkers, Jan 05 '13 at 20:05

Passing image array values and changing image source onclick

2 Answers2

Linked