2
\$\begingroup\$

This presumably simple task resulted in much more scripting logic than I thought would be necessary.

Goal: when the user clicks a button inside one of the divs, it affects said div.

Issues: I did not wish to have my function target specific elements in my DOM. (ex. targeting <a> by ID). Also, because my function is traversing the DOM and being called in multiple instances, I had to use $(this) format so as not to be specific to one ID.

Anyways, here is my code:

$(".button").toggle(function(){
    $(this).parent().children(".innerBox").fadeIn();
}, function(){
    $(this).parent().children(".innerBox").fadeOut();
});

It works fine. However, can someone with a little more jQuery savvy please let me how I could execute this function in the future, without having to parse through siblings and making it more convoluted than need be?

\$\endgroup\$
  • 4
    \$\begingroup\$ You could simplify it some by doing: $(this).siblings(".innerBox")... \$\endgroup\$ – Shmiddty Oct 16 '12 at 21:18
  • \$\begingroup\$ You could just use .toggleClass('on') / .toggleClass('off') and add transitions to your CSS for the fade effect. \$\endgroup\$ – Kevin Boucher Oct 16 '12 at 21:20
  • \$\begingroup\$ @Kevin Boucher That wouldn't decrease the amount of code at all, but I see what you were thinking. The amount of code I'm writing doesn't need to be truncated by a CMS or something of the sort. I merely wanted a "better way of doing this". \$\endgroup\$ – Justin Ward Oct 16 '12 at 21:47
  • \$\begingroup\$ @JustinWard, probably something like I answered is what you are looking for \$\endgroup\$ – Alexander Oct 16 '12 at 21:49
4
\$\begingroup\$

You can certainly cache the lookups, abusing of the scopes.

$(".button").each(function(){
  var $btn = $(this);
  var $box = $btn.parent().children(".innerBox");
  $btn.toggle(function(){
    $box.fadeIn();
  }, function(){
    $box.fadeOut();
  });
});

Using .fadeToggle() instead of .fadeIn() and .fadeOut().

$(".button").each(function(){
  var $btn = $(this);
  var $box = $btn.parent().children(".innerBox");
  $btn.click(function(){
    $box.fadeToggle();
  });
});

And last but not least, .siblings().

$(".button").each(function(){
  var $btn = $(this);
  var $box = $btn.siblings(".innerBox");
  $btn.click(function(){
    $box.fadeToggle();
  });
});
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Thanks, Alexander. .fadeToggle is nice! Good call. I too was creating variables, as I thought it might make things simpler, but it becomes a bit too confusing if others are intended to make sense of the code. Personal preferences, I guess. It's for that same reason I'm not a big fan of SASS (CSS) either. \$\endgroup\$ – Justin Ward Oct 16 '12 at 22:01
  • \$\begingroup\$ @JustinWard, be advised, jQuery is a monster that you can't use without caching \$\endgroup\$ – Alexander Oct 16 '12 at 22:04
  • \$\begingroup\$ Haha. Yeah, a bit of an oxymoron, eh? Using JQuery but not being a fan of variables...I just meant, I usually don't like creating even more variables. \$\endgroup\$ – Justin Ward Oct 16 '12 at 22:54
1
\$\begingroup\$

My only alteration would be

$(this).parent().children(".innerBox") 

to instead

$(this).siblings(".innerBox")

see here: http://jsbin.com/azamot/1/edit

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Haha! a selector called 'siblings'? It can't be that simple can it? Ohh. JQuery, you've done it again - making things as blatantly simple as a slap across the face. Thanks, ceepee! \$\endgroup\$ – Justin Ward Oct 16 '12 at 21:42

Your Answer

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