[gadgetgirl]

Hi - I'm building a page which uses javascript to display an overlay when an image is clicked on. I'm nearly there - but the overlay is not displayed the first time the image is clicked on. Subsequent clicks show the overlay but not the first click. I've tried putting the call to the js function in the head section of the page and just before the end body tag but it makes no difference. thanks for your suggestions. (the site is a work in progress so go easy on me, but any suggestions to improve the look, feel etc would be appreciated). Here's the link.

[Skateside]

Advice 1: use thumbnails. This won't solve the immediate problem, but it'll help those images load a lot faster. A thumbnail is an image that's been shrunk down in something like Photoshop, shrinking it down in HTML means that you still have to lead that huge image.

Main advice: that way you add those calls to showOverlay isn't quite right. What you're doing is returning an execution of that function rather than executing it when you want to. Instead of writing this:

<a href='#' onClick='return showOverlay();'><img src="images/snap6.jpg" name="lhPhotos" width="120px" height="90px"></a>

just write this:

<a href='#' onClick='return showOverlay'><img src="images/snap6.jpg" name="lhPhotos" width="120px" height="90px"></a>

Even better advice: use jQuery to delegate your events. Since you're using it anyway, by far the best advice I can give you is to remove the inline onClick altogether. Have a bit of a Google of "event delegation" and you'll see how awesome it is. Basically, you assign your click event to the parent element and check to see which part of it was clicked. The little snippit of code below will do the same thing that you're trying to do:

$(function () {
    // When the container is clicked ...
    $('#photoWrap').click(function (e) {
        // ... work out what was clicked.
        var jQtarget = $(e.target);
        // If an A tag or something inside an A tag was clicked ...
        if (jQtarget.is('a') || jQtarget.parents('a').size()) {
            // ... stop the A tag taking us anywhere ...
            e.preventDefault();
            // ... and show the overlay.
            showOverlay();
        }
    });
});

[gadgetgirl]

thanks Skateside for your very helpful comments. I'm attempting the event delegation approach and understand it in theory. It works as you said but still not working on the first click. Where am I going wrong? here's the link again.

This post has been edited by gadgetgirl: 28 January 2012 - 02:54 PM

[Skateside]

gadgetgirl, on 28 January 2012 - 02:28 PM, said:

thanks Skateside for your very helpful comments. I'm attempting the event delegation approach and understand it in theory. It works as you said but still not working on the first click. Where am I going wrong? here's the link again.

Ah, I've had another look at your function. What's happening is the "overlayDiv" DIV has it's display style as "block" by default. Your function sees that it's "block" so it hides it. Try setting that DIV to display "none" first, either with JavaScript or CSS

[gadgetgirl]

This is curious because in the css the display of the overlayDiv has always explicitly been set to none. I played around with the css to no avail so I initialised the overlayDiv variable in the js to set the display to none. This of course had the effect of rendering the 'close' link useless, so I've created a new function hideOverlay which is called by the close link. Not sure if its the most elegant solution but it does work. Thanks again for your comments as they got me thinking along the right lines. As I'm sure you've surmised, I'm rubbish at javascript so any pointers are gratefully received!

the next step is to ensure that the appropriate text is displayed depending on which photo is clicked. I'm thinking an array and a loop using an index variable.

[Skateside]

gadgetgirl, on 28 January 2012 - 07:30 PM, said:

the next step is to ensure that the appropriate text is displayed depending on which photo is clicked. I'm thinking an array and a loop using an index variable.

I'd do this in a slightly different way than you're doing it now. Given that your descriptions aren't very long, add them to the alt attribute of the images. Then we'll just pass them to your showOverlay function. Instead of the code I gave you a little while ago, use this:

 $(function () {
    $('#photoWrap').click(function (e) {
        var jQtarget = $(e.target),
            text;

        // If the user clicked a link, the text we need is on the image inside it.
        if (jQtarget.is('a')) {
            text = jQtarget.find('img').attr('alt');
        // If they clicked the image, we can just use the alt attribute.
        } else if (jQtarget.is('img') && jQtarget.parents('a').size()) {
            text = jQtarget.attr('alt');
        }

        // Only do something if we managed to find some text.
        if (text) {
            e.preventDefault();
            showOverlay(text);
        }
    });
});

Then your showOverlay function will look like this:

function showOverlay(text){
	var overlayDiv=document.getElementById('overlayDiv');
	overlayDiv.innerHTML = "<p>" + text + "</p><p><a href='#' onclick='return hideOverlay;'>[close window]</a></p>";	
	overlayDiv.style.display = "block";
	return false;
}

See how well that works.

[gadgetgirl]

Works well, thanks!