1
\$\begingroup\$

I am a jQuery beginner and would like some pointers on whether the following code could be made shorter, or there is a better way to get the same result.

jQuery(document).ready(function(){
     var aa=jQuery('#navigation-wrapper');
     var bb=jQuery('#top');
     jQuery(window).scroll(function(){

        if(jQuery("body").hasClass("home")){    
            if(jQuery(this).scrollTop()>510){
                aa.addClass("navbar-fixed-top");
                //bb.css('marginTop', aa.height());
             }
            else{
                aa.removeClass("navbar-fixed-top");
                //bb.css('marginTop', 0);
            }
        }
        else {
           if(jQuery(this).scrollTop()>293){
             aa.addClass("navbar-fixed-top");
           //bb.css('marginTop', aa.height());
            }
            else{
                aa.removeClass("navbar-fixed-top");
                //bb.css('marginTop', 0);
            }
        }
     });
  });
\$\endgroup\$

migrated from stackoverflow.com Aug 12 '16 at 12:02

This question came from our site for professional and enthusiast programmers.

  • \$\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 Aug 12 '16 at 12:35
5
\$\begingroup\$

The code can made very short.

$(window).scroll(function () {
    var scrollTop = $(document.body).hasClass('home') ? 510 : 293;
    $('#navigation-wrapper').toggleClass('navbar-fixed-top', $(this).scrollTop() > scrollTop);
});
  1. There is no need of wrapping the code in $(document).ready() for binding events on window.
  2. Use ternary operator
  3. Use toggleClass() with second parameter to add/remove class based on a condition.
\$\endgroup\$
  • \$\begingroup\$ Did you just suggest the asker to go somewhere else, while at the same time preventing him with your answer from deleting his question? That's not ideal. \$\endgroup\$ – John Dvorak Aug 5 '16 at 11:19
  • \$\begingroup\$ @JanDvorak No. I flagged the question for mod attention and asked them to migrate to CR.SE. \$\endgroup\$ – Tushar Aug 5 '16 at 11:20
  • 1
    \$\begingroup\$ Understood. A better option would have been to suggest the asker to delete the question and reask it somewhere else. That way the mods don't have to get involved. \$\endgroup\$ – John Dvorak Aug 5 '16 at 11:21
  • \$\begingroup\$ @JanDvorak Right. That'll decrease burden on busy mods. Thanks. Will keep it in mind. :) \$\endgroup\$ – Tushar Aug 5 '16 at 11:23
  • \$\begingroup\$ Does this not mean that the condition would only run if the body has a class of "home"? \$\endgroup\$ – ADRIAN Aug 5 '16 at 11:28

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.