current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 3 down vote favorite

I have the Javascript below that is used to make a sidebar widget stick to a fixed position once the page is scrolled down, once you scroll to the bottom of the page where the footer div is I then un-sticky it so that the stickied div does not overlap the footer.

It works great but I am not very good with Javascript, I am hoping to clean it up because it looks sloppy, any help from a Guru?

The Javascript

$(window).load(function () {
/*-----------------------*/
 * Fixed Widget on Scroll
 *-----------------------*/

    // set some Div vars
    var stickyID = $("#sticky-scroll-wrapper"),
        footerID = $('#footer'),
        stickyDivHeight = stickyID.outerHeight(),
        stickyIDMargin = 70;

    $(window).scroll(function () {
        var currentPosition = $(document).scrollTop() + stickyIDMargin;
        var stickyIDPosition = stickyID.offset().top;

        // cut off point/un sticky when we reach this far down the page
        var limit = footerID.offset().top - stickyDivHeight;

        //determines whether sidebar should stick and applies appropriate settings to make it stick
        if (currentPosition >= stickyIDPosition && currentPosition < limit) {
            $('#sticky-elements').css({
                'position': 'fixed',
                'top': '70px'
            });
        } else {
            $('#sticky-elements').css({
                'position': 'static',
            });
        }
        // Un-stick once we reach the #fotter section of the page to prevent overlapping
        if (limit < currentPosition) {
            var diff = limit - currentPosition;
            $('#sticky-elements').css({
                'top': diff + stickyIDMargin,
                'position': 'fixed',
            });
        }

    });




});

The HTML 

<div id="sticky-scroll-wrapper">
    <div id="sticky-elements" style="position: static; ">sidebar content</div>
</div>
share|improve this question
    
@ANeves You are right I fixed that but you should of also noticed that I only had 4 questions and some only with 1 answer which wasn't good at all. I'm not new I have nearly 8k score on SO. I agree with you though I had overlooked it –  JasonDavis Sep 17 '12 at 18:07

1 Answer 1

up vote 1 down vote

I can make a small improvement in your logic structure that may make a very small improvement in performance. If limit < currentPosition, all you need to do is that code block, and you can use else if and remove testing its opposite. See code:

$(window).load(function () {
/*-----------------------*/
 * Fixed Widget on Scroll
 *-----------------------*/

    // set some Div vars
    var stickyID = $("#sticky-scroll-wrapper"),
        footerID = $('#footer'),
        stickyDivHeight = stickyID.outerHeight(),
        stickyIDMargin = 70;

    $(window).scroll(function () {
        var currentPosition = $(document).scrollTop() + stickyIDMargin;
        var stickyIDPosition = stickyID.offset().top;

        // cut off point/un sticky when we reach this far down the page
        var limit = footerID.offset().top - stickyDivHeight;

        // Un-stick once we reach the #fotter section of the page to prevent overlapping
        if (limit <= currentPosition) {
            var diff = limit - currentPosition;
            $('#sticky-elements').css({
                'top': diff + stickyIDMargin,
                'position': 'fixed',
            });
        }
        //determines whether sidebar should stick and applies appropriate settings to make it stick
        else if (currentPosition >= stickyIDPosition) {
            $('#sticky-elements').css({
                'position': 'fixed',
                'top': '70px'
            });
        }
        else {
            $('#sticky-elements').css({
                'position': 'static',
            });
        }
    });

});

Also, you might consider consolidating var diff = limit - currentPosition; with 'top': diff + stickyIDMargin if you are comfortable with that. Otherwise, it looks great to me. :)

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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