So I have this code that for the most part, does what it needs to do, but I'm having trouble making it more concise and elegant.


Intended functionality:

  • If you click a trigger, it's corresponding element must display
  • If you click another trigger, that trigger's element displays and the other closes
  • If you click the trigger whilst it is open, it simply closes the element

Any ideas on how to make this cleaner?

var trigger = ".trigger";
var recipient = ".target";
var not_trigger = $(".trigger").not(this);

$(trigger).click(function(e) {
  recipient = '#' + $(this).attr('data-target-id');
  var not_recipient = $(".target").not(recipient);

  $(this).toggleClass("active") ;

    TweenMax.to($(".target"), .3, {display:'none', y:'0%', autoAlpha:0});

      TweenMax.to(recipient, .3, {display:'none', y:'0%', autoAlpha:0});
      TweenMax.to(recipient, .3, {display:'block', y:'100%', autoAlpha:1});
  }else {
    TweenMax.to(recipient, .3, {display:'block', y:'100%', autoAlpha:1});

  • \$\begingroup\$ Is there a benefit in making variables containing the class-names as strings? Then using these variables instead of $('.trigger') etc. directly? Or is it just a writing-preference? \$\endgroup\$ – michael.zech May 25 '16 at 6:17
  • \$\begingroup\$ Is e.stopPropagation() really needed here? Moreover: Variable 'not_trigger' is unused. \$\endgroup\$ – michael.zech May 25 '16 at 6:19

Your Answer

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

Browse other questions tagged or ask your own question.