SpeedOfSmell
SpeedOfSmell

Reputation: 31

Having trouble getting the scroll animation to not trigger the scroll event

I currently have a number of div tags that can be seen below that are each the height of the viewport.

<!-- .current represents the div the user is currently viewing -->
<div class="full-height current" id="id1">
    <div class="container">

    </div>
</div>

<div class="full-height" id="id2">
    <div class="container">
    </div>
</div>

<div class="full-height" id="id3">
    <div class="container">
    </div>
</div>

<div class="full-height" id="id4">
    <div class="container">
    </div>
</div>

<div class="full-height" id="id5">
    <div class="container">
    </div>
</div>

I am attempting to implement a feature where upon a user scrolling, the window will scroll to the next div tag, always having only one div in view at a time. The way I implemented it works great, except for the fact that the animation triggers the scroll event again, resulting in an endless loop of the page scrolling once the user scrolls at all. I attempted to fix this by having a variable that will stop the event from triggering if the animation is in progress, but it does not seem to work. I am aware that I didn't do it for the scroll up, but I just wanted to see if it worked for the down first.

$(window).scroll(function(event) {
    var scrollTop = $(this).scrollTop();

    // If user scrolls down
    if ((scrollTop > lastScrollTop) && $(".current").next("div").length > 0) {
        if (animating == false) {
            console.log("down");

            $(".current").next("div").addClass("current");
            $(".current").first().removeClass("current");

            animating = true;
            $("html, body").animate({
                scrollTop: $(".current").offset().top    
            }, 1000, function() {animating = false});   
        }
    // If user scrolls up
    } else {
        if ($(".current").prev("div").length > 0) {
            console.log("up");

            $(".current").prev("div").addClass("current");
            $(".current").last().removeClass("current");

            $("html, body").animate({
                scrollTop: $(".current").offset().top    
            }, 1000);
        }            
    }

    lastScrollTop = scrollTop;
});

CSS included just in case. The 100vh - 111px is due to a fixed navbar at the top that is 111px high

/* Makes the element take up the entire height of the screen */
.full-height{
    height: -o-calc(100vh - 111px); /* opera */
    height: -webkit-calc(100vh - 111px); /* google, safari */
    height: -moz-calc(100vh - 111px); /* firefox */
}

#id1 {
    background-color: red;
}
#id2 {
    background-color: blue;
}

#id3 {
    background-color: yellow;
}
#id4 {
    background-color: green;
}

#id5 {
    background-color: purple;
}

If anyone could give me any ideas for fixing my problem, that would be great.

Upvotes: 2

Views: 1867

Answers (3)

xcrx news
xcrx news

Reputation: 91

If you define height 100vh ,scroll event wont trigger

Upvotes: 0

guest271314
guest271314

Reputation: 1

Try defining scroll event handler as named function ; defining lastScrollTop outside of scroll handler ; substituting .one() for .on() to allow animation to complete before re-attaching scroll event ; use .promise() which should be called at most once to avoid .animate() complete callback being called twice with selector "html, body" ; substituting single if to check for next or previous element .length and .animate() call for multiple if..else conditions and statements, animation function calls ; re-attach scroll event at .then() following animation completion .promise()

var lastScrollTop = 0;

function scroller(event) {

  var scrollTop = $(this).scrollTop();
  var direction = scrollTop > lastScrollTop ? "next" : "prev";
  var el = $(".current")[direction](".full-height");
  console.log(direction === "next" ? "down" : "up");
  if (el.is("*")) {
    $("html, body").animate({
      scrollTop: el.offset().top
    }, 1000).promise().then(function() {
      console.log(this)
      lastScrollTop = $(window).scrollTop();
      $(".current")
      .removeClass("current")[direction]("div")
      .addClass("current")
      setTimeout(function() {
        $(window).one("scroll", scroller)
      })
    });
  } else {
    lastScrollTop = scrollTop;
    $(window).one("scroll", scroller)
  }
}

$(window).one("scroll", scroller);
/* Makes the element take up the entire height of the screen */

.full-height {
  height: -o-calc(100vh - 111px);
  /* opera */
  height: -webkit-calc(100vh - 111px);
  /* google, safari */
  height: -moz-calc(100vh - 111px);
  /* firefox */
}
#id1 {
  background-color: red;
}
#id2 {
  background-color: blue;
}
#id3 {
  background-color: yellow;
}
#id4 {
  background-color: green;
}
#id5 {
  background-color: purple;
}
<script src="https://ajax.googleapis.com/ajax/libs/jquery/1.11.1/jquery.min.js"></script>
<!-- .current represents the div the user is currently viewing -->
<div class="full-height current" id="id1">
  <div class="container">

  </div>
</div>

<div class="full-height" id="id2">
  <div class="container">
  </div>
</div>

<div class="full-height" id="id3">
  <div class="container">
  </div>
</div>

<div class="full-height" id="id4">
  <div class="container">
  </div>
</div>

<div class="full-height" id="id5">
  <div class="container">
  </div>
</div>

Upvotes: 1

rism
rism

Reputation: 12132

You'll want to stop the event and preventDefault. Here is some code from a current landing page:

 $(document).ready(function () {
        $('a.page-scroll').on('click', function(event) {
            var link = $(this);
            $('html, body').stop().animate({
                scrollTop: $(link.attr('href')).offset().top - 50
            }, 500);
            event.preventDefault();
        });
        $('body').scrollspy({
            target: '.navbar-fixed-top',
            offset: 80
        });

    });

It uses bootstrap scrollspy so just ignore that. But notice that it stops any scroll animation that could be running and then also calls event.preventDefault() to stop the scroll event from bubbling and thus becoming recursive.

EDIT:

O.k. so I've a better look and the basic problem re: infinite scrolling is the code doesn't check if the scrollTop is already at 'where it needs to be'. You need an additional check to short circuit the scroll:

if (animating == false) {

        if ($(this).scrollTop() == lastScrollTop) return;

        if (($(this).scrollTop() > lastScrollTop) && $(".current").next("div")) {
        console.log("down");

        $(".current").next("div").addClass("current");
        $(".current").first().removeClass("current");

        animating = true;
        $("html, body").stop().animate({
            scrollTop: $(".current").offset().top
        }, 1000,
        function () { lastScrollTop = $(this).scrollTop(); animating = false; });

        // If user scrolls up
    } else {
        if ($(".current").prev("div").length > 0) {
            console.log("up");

            $(".current").prev("div").addClass("current");
            $(".current").last().removeClass("current");

            $("html, body").stop().animate({
                scrollTop: $(".current").offset().top
            }, 1000, function () { lastScrollTop = $(this).scrollTop(); animating = false; });
        }
    }
}

Otherwise as it stands it will always either scroll up or down and never settle. That said with that "working" it's a terrible user experience. It will jump about since your scroll to code will fight the user on current scrollTop if they keep scrolling. i.e. your code will make it jump back to a previous position.

Upvotes: 1

Related Questions