CODEWITHSUNDEEP
javascriptjquery
visevo
visevo

Reputation: 861

jQuery scrollTop being buggy

I'm trying to make a sub navigation menu animate a fixed position change after a user has scrolled down 200 pixels from the top. It works but it's very buggy, like when the user scrolls back to the top it doesn't always return to the original position, etc. I'm not strong with javascript / jquery, but I thought this would be simple to do. What am I missing?

Here's my fidde: http://jsfiddle.net/visevo/bx67Z/

and a code snippet:

(function() {
    console.log( "hello" );
    var target = $('#side-nav');
    var scrollDis = 200;
    var reset = 20;
    var speed = 500;
    $(window).scroll(function() {
        console.log( $(window).scrollTop() );
        if( $(window).scrollTop() > scrollDis ) {

            $(target).animate({
                top: reset + 'px'
            }, speed);
        } else {
            $(target).animate({
                top: scrollDis + 'px'
            }, speed);
        }
    });
})();

Upvotes: 1

Views: 737

Answers (3)

Jeff Davis
Jeff Davis

Reputation: 4797

Since I am commenting all over the place I should probably actually contribute an answer.

The issue is that you are adding scroll events every time a scroll occurs, which is causing more scrolling to occur, which causes more scroll events, hence infinite loop. While cancelling previous events will fix the problem, it's cleaner to only fire the event when you pass the threshold, IE:

(function () {
    console.log("hello");
    var target = $('#side-nav');
    var scrollDis = 200;
    var reset = 20;
    var speed = 500;
    var passedPosition = false;
    var bolMoving = false;
    $(window).scroll(function () {
        if (bolMoving) return; // Cancel double calls. 
        console.log($(window).scrollTop());
        if (($(window).scrollTop() > scrollDis) && !passedPosition) {
            bolMoving = true; // 
            $(target).animate({
                top: reset + 'px'
            }, speed, function() { bolMoving = false; passedPosition = true; });
        } else if (passedPosition && $(window).scrollTop() <= scrollDis) {
            bolMoving = true;
            $(target).animate({
                top: scrollDis + 'px'
            }, speed, function() { bolMoving = false; passedPosition = false; });
        }
    });
})();

http://jsfiddle.net/bx67Z/12/

Upvotes: 2

CRABOLO
CRABOLO

Reputation: 8793

http://jsfiddle.net/bx67Z/3/

I just added .stop() in front of the .animate() , and it works a lot better already.

        $(target).stop().animate({
            top: reset + 'px'
        }, speed);
    } else {
        $(target).stop().animate({
            top: scrollDis + 'px'
        }, speed);

You can also use .stop(true)

http://jsfiddle.net/bx67Z/5/

        $(target).stop(true).animate({
            top: reset + 'px'
        }, speed);
    } else {
        $(target).stop(true).animate({
            top: scrollDis + 'px'
        }, speed);

You can also use .stop(true, true)

http://jsfiddle.net/bx67Z/4/

        $(target).stop(true, true).animate({
            top: reset + 'px'
        }, speed);
    } else {
        $(target).stop(true, true).animate({
            top: scrollDis + 'px'
        }, speed);

So the reason .stop(true) works so well, is that it clears the animation queue. The reason yours was being "buggy" is because on every scroll the animation queue was "bubbling up" , thus it took a long time for it to reach the point where it would scroll back to the original position.

For information about .stop() , see here http://api.jquery.com/stop

Upvotes: 1

Lokesh Suthar
Lokesh Suthar

Reputation: 3202

How about a little bit of css and jquery both ??

What I did is added transition to side-nav to animate it and rectified your js to just change it's css. You can set how fast it moves by changing the time in transition.

FIDDLE

#side-nav {
    position: fixed;
    top: 100px;
    left: 10px;
    width: 100px;
    background: #ccc;
    -webkit-transition:all 0.5s ease-in-out;
}

(function () {
    var target = $('#side-nav');
    var scrollDis = 100;
    var reset = 20;
    var speed = 500;
    $(window).scroll(function () {
        if ($(this).scrollTop() >= scrollDis) {

            target.css("top", reset);
        } else {
            target.css("top", scrollDis);
        }
    });
})();

NOTE: When you cache a jQuery object like this

var target = $("#side-nav");

You don't need to use $ again around the variable.

Upvotes: 2

Related Questions