2
\$\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\$
1
  • \$\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\$ Commented Aug 12, 2016 at 12:35

1 Answer 1

Reset to default
6
\$\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\$
6
  • \$\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\$ Commented Aug 5, 2016 at 11:19
  • \$\begingroup\$ @JanDvorak No. I flagged the question for mod attention and asked them to migrate to CR.SE. \$\endgroup\$ Commented Aug 5, 2016 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\$ Commented Aug 5, 2016 at 11:21
  • \$\begingroup\$ @JanDvorak Right. That'll decrease burden on busy mods. Thanks. Will keep it in mind. :) \$\endgroup\$ Commented Aug 5, 2016 at 11:23
  • \$\begingroup\$ Does this not mean that the condition would only run if the body has a class of "home"? \$\endgroup\$ Commented Aug 5, 2016 at 11:28

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.