3
\$\begingroup\$

I am new to jQuery and needed a solution for the following problem:

There are 3 images in a row and when the user clicks on one of them all three pictures are hiding and instead another image shows up (depending on the click). So while the markup and CSS is quite simple and only a quick draft the jQuery was the main issue. I came up with the following solution and would love to hear some advice, reviews, critiques or similar.

I also made a JSFiddle. for better understanding.

Here is my jQuery Code with similar click-functions (this bothers me a little):

var $button = '<button id="closebtn">Close</button>';

$($button).appendTo('#panzer');


// Pic1 hide and replace
$( "#pic1" ).click(function() {
    $('#featured li img').hide( "slow");
  $( ".undercon1" ).show( "fast", function() {
            $('#closebtn').show();
  });
});
// Pic2 hide and replace
$( "#pic2" ).click(function() {
    $('#featured li img').hide( "slow");
  $( ".undercon2" ).show( "fast", function() {
        $('#closebtn').show();
  });
});
// Pic3 hide and replace
$( "#pic3" ).click(function() {
    $('#featured li img').hide( "slow");
  $( ".undercon3" ).show( "fast", function() {
        $('#closebtn').show();
  });
});


// Button shows only with Replacement Images


$('button#closebtn').click(
     function ()
     {
         $(this).hide();

         $(".undercon1").hide( "slow");
         $(".undercon2").hide( "slow");
         $(".undercon3").hide( "slow");
         $('#featured li img').show( "slow");
     }
);
\$\endgroup\$
  • \$\begingroup\$ Can you be specific about what concerns you? \$\endgroup\$ – James Khoury Sep 11 '14 at 7:01
  • 1
    \$\begingroup\$ Its just the "Pic hide and replace" that repeats three times. I dont think its best practice to repeat something that often. I guess. \$\endgroup\$ – Steve Sep 11 '14 at 7:30
  • \$\begingroup\$ as a side note, your HTML in your fiddle is invalid. div is not allowed as direct child of a ul \$\endgroup\$ – Pevara Sep 12 '14 at 22:15
  • \$\begingroup\$ This question has an exact solution at - Switching Images using JQuery Click event \$\endgroup\$ – A100WebSolutions Sep 15 '14 at 7:10
4
\$\begingroup\$

Essentially I agree with @gerdi's answer, but I would do it slightly simpler.

Basically, you want to perform the same kind of action for each sample clicked:

  1. Hide all the samples
  2. Show the image corresponding to the sample
  3. Show the close button

The only variable is the image to show, which must correspond to the image clicked. A good place to express this relationship is a data-* attribute, which you can put on the same tag that will receive the clicks:

<li id="pic1" data-rel=".undercon1">...</li>

I called it data-rel (for "related", "relationship"), and set the value to a valid DOM selector expression. This way you can rewrite the click handler in a way that you can apply to all the #feature li elements, without having to write one by one:

$('#featured li').click(function() {
    $('#featured li img').hide("slow");
    var selector = $(this).data('rel');
    $(selector).show("fast", function() {
        $('#closebtn').show();
    });
});

The shortcut $(this).data('rel') will take the value of the data-rel attribute, equivalent to $(this).attr('data-rel').


Another thing, in the click handler of the close button, to avoid the repeated similar lines for .undercon1, .undercon2, ..., you could give them all a common undercon class:

<div class="row undercon undercon1">...</div>
<div class="row undercon undercon2">...</div>
<div class="row undercon undercon3">...</div>

so that you can hide them all easily with one line:

$(".undercon").hide("slow");
\$\endgroup\$
  • \$\begingroup\$ Nice. And - from my perspective - a tiny bit more easy to understand than the previous example from gerdi. Great. Thank you. \$\endgroup\$ – Steve Sep 11 '14 at 14:27
2
\$\begingroup\$

You could break it down to one click function by using $(this) and then creating a connection reference with a data-*attribute.

//top level

 <li data-el="pic1">...</li>

//bottom level

 <div class="row pic1">...</div>

$('#featured li').click(function(){
     $($(this) + ' img').hide( "slow");
     var showEl = "." + $($(this)).attr("data-el");

     $(showEl).show( "fast", function() {
        $('#closebtn').show().addClass(showEl); 
       }
)};

This close button can then be

$('button.' + showEl).click(function(){
     $(this).hide();
     $("#panzer " + showEl ).hide( "slow");
     $('#closebtn').removeClass(showEl);
});

I would like to take the time to suggest angularjs, backbonejs or any of the other frameworks that have been created to take this type of complexity in jquery, out.

If you are new to jquery then take the extra step to learn these frameworks. It is jquery mixed with black magic and i promise you the time you spend writing complex ( even not so complex ) jquery will be cut by a lot. Its an extra learning curve , but in the long run you will be saving massive amounts of time. And creating cooler stuff.

\$\endgroup\$
  • \$\begingroup\$ I can't say that I agree with you regarding frameworks. This code is already small, you're going to pile additional dependencies on top of that just to make it a tiny bit smaller? Using jQuery to do anything is slower than using vanilla JS by such a huge margin that it's not even funny. Add another framework on top? No thanks. \$\endgroup\$ – cimmanon Sep 11 '14 at 14:18
  • \$\begingroup\$ Thank you for your suggestion but i am already learning a framework and now i should learn a framework for a framework :) ? Thats too much for me right now. But i will take a closer look at your code. \$\endgroup\$ – Steve Sep 11 '14 at 14:25

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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