1
\$\begingroup\$

I have made a mini image gallery which consists of one large div which has a background image of one of the galleries images. Underneath that are five thumbnail images, that when clicked, change the background image of the large div above.

All code works as expected but it looks like it could be improved. How can I rewrite it in a shorter, cleaner looking way?

$("#thumb2").on('click', function(){

    $(".main-image").css('background-image', 'url(img/lrg-img/use/dscn2717.png)');
});

$("#thumb1").on('click', function(){

    $(".main-image").css('background-image', 'url(img/lrg-img/use/dscn2714.png)');
});

$("#thumb3").on('click', function(){

    $(".main-image").css('background-image', 'url(img/lrg-img/use/dscn2720.jpg)');
});
$("#thumb4").on('click', function(){

    $(".main-image").css('background-image', 'url(img/lrg-img/use/dscn2733.jpg)');
});
$("#thumb5").on('click', function(){

    $(".main-image").css('background-image', 'url(img/lrg-img/use/dscn2735.jpg)');
});
\$\endgroup\$

3 Answers 3

4
\$\begingroup\$

You're using $(".main-image") a lot. It would be better to make a variable for that. All the id's start with "thumb", so it might be better to use one click event handler on a selector that looks like this: $("[id^='thumb']"). You can then, in the body of the click handler, get the id by using $(this).attr('id').

\$\endgroup\$
2
\$\begingroup\$

If you were to add, say, a data attribute (called something like data-full-image) to the thumbnails, and give each one a class of .thumbnail, then you could replace all of it with something like:

$(".thumbnail").click(function() {
    var imageName = $(this).data("full-image");
    $(".main-image").css('background-image', 'url(img/lrg-img/use/'+ imageName +'.png)');
});

Also note than click(function) is shorthand for on("click", function).

\$\endgroup\$
2
\$\begingroup\$

Yes. There is a lot of optimization potential in this code.

  1. You should define a class for your thumbnails (e.g. "thumbnail"). It would make querying a lot easier:

    $(".thumbnail")
    

    and you got all of them. Besides you could use the id for numbering, which makes it a lot more intuitive to get the right thumbnail

    $(".thumbnail[id='1']")
    

    gets the thumbnail with the id of "1".

  2. You could use a dispatching function, which you invonke "onClick":

    var thumbnails={"1":"img/lrg-img/use/dscn2717.png" ... } // same for the other ids 
    function getImageUrlByThumbnailId(id){ return thumbnails[id]; }
    

    This makes it possible to write code once and update dynamincally (if wanted). So you could write the code once, to update a thumbnail and invoke this "onClick".

  3. Perhaps you should cache the $(".main-image") selection.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.