7
\$\begingroup\$

I've written a function that works fine, but being new to jQuery, I'd like some reviews on writing this more cleanly. Anything advice would help; just trying to learn!

function displayContent() {
  var $link1 = $('.row.nav li a.bio');
  var $link2 = $('.row.nav li a.stylist');
  var $link3 = $('.row.nav li a.contact');
  var $content = $('#text-content')
  var $bio = $("#bio");
  var $stylist = $("#stylist");
  var $contact = $("#contact");
  var $overlay = $('.content-overlay');

  //link1
  $link1.click(function (e) {
    e.stopPropagation();
    $link2.removeClass('active');
    $link3.removeClass('active');
    $link1.addClass('active');
    $contact.hide();
    $stylist.hide();
    $bio.fadeIn(700);
    $overlay.show();
  });
  //link2
  $link2.click(function (e) {
   //same code here
  });
  //link3
  $link3.click(function (e) {
    //same code here
  });


  //close overlay/hide content
  $('html').click(function (e) {
//hide code
  });

}
\$\endgroup\$
  • \$\begingroup\$ When you say // same code here does that mean that both of those handlers are identical? \$\endgroup\$ – cookie monster Feb 28 '14 at 4:06
  • \$\begingroup\$ Same code as the $link1, except now the 'active' is being applied to $link2, etc \$\endgroup\$ – RENE VILLA Feb 28 '14 at 4:11
4
\$\begingroup\$

Using $(this) is very helpful in situations where you have similar/same code. The way it works, is if say you have a class .general , which 50 different div's on the page all have that class. But lets say, if a user clicks on any one of those 50 div's , you want ONLY that div that was clicked on to have a border of 5px solid red. So instead of writing

$('.general').click(function() {
    $('.general').css({border: '5px solid red'});
}); 

which would give all 50 div's a border, you can write this

$('.general').click(function() {
    $(this).css({border: '5px solid red'});
}); 

and now only the div that was clicked on will have the border. Also, it will work more than once, so if you then clicked on 10 other div's with class general, they would all get the border red as well.


This can be applied to your code like so : (only included the relevant info)

function displayContent() {

  var $links = $('.row.nav li a.bio, .row.nav li a.stylist, .row.nav li a.contact');

}

  $links.click(function (e) {  // so you just need one click function instead of 3
    e.stopPropagation();
    $links.removeClass('active');  // removes the class active from all $links
    $(this).addClass('active');  // gives the class active to the one clicked on

  });
\$\endgroup\$
  • 1
    \$\begingroup\$ Thanks for the explanation @LowerClassOverflowian! Much appreciated! \$\endgroup\$ – RENE VILLA Feb 28 '14 at 4: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.