1
\$\begingroup\$

The function is to click a button, the browser window then scrolls to the top of the page, then a class is added to a menu item (with CSS to make the sub menu appear). Then when the cursor leaves the sub menu, the class is removed to allow the sub menu to disappear.

Can I make the following code shorter or more efficient? I have written it to work with Joomla and jQuery with no conflict.

jQuery(document).ready(function(){
    jQuery("#instruct-us-readmore").click(function(){
        jQuery('html, body').animate({scrollTop:0}, 800);
        setTimeout(
        function() {
            jQuery("#iceMenu_144 ul.sub_level_1").addClass("instruct-button-click");
        },950);


     });
});


  jQuery(document).ready(function(){
        jQuery("#iceMenu_144 ul.sub_level_1").mouseleave(function(){
            jQuery("#iceMenu_144 ul.sub_level_1").removeClass("instruct-button-click");
    });
      });
\$\endgroup\$
3
  • \$\begingroup\$ shorter is not necessarily more efficient... \$\endgroup\$
    – pppp
    Sep 29, 2016 at 15:02
  • \$\begingroup\$ As we all want to make our code more efficient or improve it in one way or another, try to write a title that summarizes what your code does, not what you want to get out of a review. Please see How to get the best value out of Code Review - Asking Questions for guidance on writing good question titles. \$\endgroup\$
    – BCdotWEB
    Sep 29, 2016 at 15:10
  • \$\begingroup\$ Ok, so you wouldn't suggest any amendments? \$\endgroup\$
    – ADRIAN
    Sep 29, 2016 at 15:10

1 Answer 1

1
\$\begingroup\$

jQuery provides itself as first argument in the ready callback. You can assign it the $ name so you can use it shorthand inside the callback.

jQuery(function($){
  // Use jQuery as $ inside here
});

Also, you don't need multiple ready handlers. Whenever possible, just bunch them up into one.

jQuery(function($){
  // Click handler
  // Mouse handler
});

Next is that we don't know for certain how jquery.animate schedules animation. This might affect how sooner or later the callback for setTimeout is executed.

However, jquery.animate accepts a function as the last argument which is run when the animation is complete. Use that instead of doing a timeout.

$('html, body').animate({scrollTop:0}, 800, function(){
  // done animating
});
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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