Condensing/Optimizing jQuery code

Question

I have the following code for sliding down a hidden content area. It works, but I suspect it's too clumsy and may fire too many requests or events. Can someone suggest a way to combinee these functions more efficiently?

$(".toggler").click(function (event){
event.stopPropagation();
$("#mobile-top").animate({'height':'toggle'}, 250);
});

$("#mobile-top").click(function(event){
event.stopPropagation();
});

$('.toggler').toggle(function() {
    $(this).html('Close This Box <em class="fa fa-chevron-up"></em>');
}, function() {
    $(this).html('Connect With Us! <em class="fa fa-chevron-down"></em>');
});

$('html').click(function(){
$("#mobile-top").slideUp();
$(".toggler").html('Connect With Us! <em class="fa fa-chevron-down"></em>');
});

Working code that you want improvement suggestions for generally belongs on codereview.stackexchange.com. — jfriend00
– jfriend00, Commented Sep 29, 2014 at 23:10

Amir5000 · Accepted Answer · 2014-10-02 20:11:44Z

1

I think the code could be improved by using variables when you can especially when using a certain selector more than once. and try and use ID's not classes.

EDIT

UPDATED JSFIDDLE

Here is the code that is working and wont get confused when you click outside in the HTML area. (note that while naming a variable it can't contain a - character like I had put before)

$(document).ready(function(){
  var toggler = $("#toggler");
  var mobileTop = $("#mobile-top");
  //top drop-down content animation
  toggler.click(function(event){
    event.stopPropagation();
    mobileTop.slideToggle(250);
    $(this).toggleClass('open');
    $(this).html('Connect With Us! <em class="fa fa-chevron-down"></em>');
    $(".open").html('Close This Box <em class="fa fa-chevron-up"></em>');
  });
  $('html').click(function(){
    mobileTop.slideUp(250);
    toggler.html('Connect With Us! <em class="fa fa-chevron-down"></em>');
    toggler.removeClass('open');
  });
  $(mobileTop).click(function(event){ 
    event.stopPropagation(); 
  });
});

And this should be more efficient as well.

a good read: http://code.tutsplus.com/tutorials/10-ways-to-instantly-increase-your-jquery-performance--net-5551

edited Oct 2, 2014 at 20:11

answered Sep 29, 2014 at 23:03

Amir5000

1,72815 silver badges19 bronze badges

Sign up to request clarification or add additional context in comments.

5 Comments

geochanto Over a year ago

Thanks, I just noticed one issue, maybe you can help figure out. When I drop it down, then click outside the area ('html') to slide it back up, and then try to drop it down again, the order is reversed. i.e. it says "close this box" when it's already closed and vice-versa. I think clicking outside the area (this line: $("#toggler").html('Connect With Us! <em class="fa fa-chevron-down"></em>');) resets the ordering. Is there a way around this?

Amir5000 Over a year ago

If you could create a JSFIDDLE or CodePen I could help you out more for sure

geochanto Over a year ago

I created jsfiddle but it's not working the same way for some reason: jsfiddle.net/8e1auupr. You can look at the demo for a more accurate version: 76.163.3.49

Amir5000 Over a year ago

@GeorgeC check my updated answer it should fix your issues and also using animation toggle with slideUp was causing it to not slide smoothly and so I just stuck to the slideToggle for all instances.

geochanto Over a year ago

Thanks, in addition, to prevent sliding up when people click on the dropped-down area the code needs: jQuery(mobileTop).click(function(event){ event.stopPropagation(); });

Collectives™ on Stack Overflow

Condensing/Optimizing jQuery code

1 Answer 1

5 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

5 Comments

Your Answer

Sign up or log in

Post as a guest

Related