waypoints icon indicating copy to clipboard operation
waypoints copied to clipboard

Inview and initialisation after context has been scrolled to the bottom

Open brendon opened this issue 7 years ago • 8 comments

Hi there, I've really been enjoying getting to know Waypoints.

I'm developing a simple comments system with a modal popup that has a scrollable div, and within that a comments div, and within that the actual comment's divs. So:

<div class=".context">
  <div class=".comments">
    <div class=".comment">
      Comment
    </div>
    <div class=".comment">
      Comment
    </div>
    <div class=".comment">
      Comment
    </div>
  </div>
</div>

The comments are displayed with the most recent at the bottom. Currently I scroll the user to the bottom, then create the waypoints (using Inview):

content.animate { scrollTop: content.prop('scrollHeight') },
  duration: 1000
  complete: ->
    for comment in content.find('.comment').has('.metadata > span.new')
      do (comment) ->
        new Waypoint.Inview
          element: comment
          context: content[0]
          entered: (direction) ->
              this.destroy()
              console.log this.element
              console.log direction

Unfortunately, when I register the waypoints, they all trigger a 'down' direction immediately, even those that aren't at all in view (i.e. way up the scrollable area).

The idea situation would be for those waypoints that are completely in view to trigger immediately, but those that are partially or not in view at all to not.

Am I pushing the capability of Inview a bit here?

brendon avatar Jul 15 '16 05:07 brendon

I think I've found the reason, that there is an assumption that the waypoints are initialised when the context hasn't been scrolled yet (and is at the top of the page).

How would you suggest one work around this?

brendon avatar Jul 16 '16 05:07 brendon

In terms of code, changing this:

else if (freshWaypoint && axis.oldScroll >= waypoint.triggerPoint) {
  waypoint.queueTrigger(axis.forward)
  triggeredGroups[waypoint.group.id] = waypoint.group
}

to this:

else if (freshWaypoint && axis.oldScroll <= waypoint.triggerPoint) {
  waypoint.queueTrigger(axis.backward)
  triggeredGroups[waypoint.group.id] = waypoint.group
}

Fixes the problem for me and changes the assumption that we're at the top of the context to that we're at the bottom of the context when the waypoints are first freshly triggered.

Would you be interested in turning this into a setting of some kind?

brendon avatar Jul 16 '16 05:07 brendon

I figured that instead of hacking around the problem, it would make sense to only trigger fresh waypoints when they're actually in view. Then we're not making any assumptions about where we are scrolled at when a waypoint is initialised. Right now I think the assumption is that we're at the top of the context. I've love some input on this solution as I don't normally work in Javascript.

brendon avatar Jul 16 '16 11:07 brendon

I had a problem relating to this when creating a waypoint that was supposed to trigger when the user scrolled past it, in the "up" direction. Because of scroll throttling, I believe, the Waypoint was sometimes created below the current scroll position. This ment that the user had technically scrolled past the waypoint but the waypoint was never triggered. This problem does not exists for waypoint that should trigger in the "down" direction, because of the freshWaypoint check @brendon mentions. A solution to this would be appending the code he suggested, thereby allowing one check for fresh waypoint below the trigger point and one check for those above.

I have supplied a PR for this issue #530

JonatanJJ avatar Oct 25 '16 09:10 JonatanJJ

Hi @Jontis00, unfortunately no one ever replied to my work. Fingers crossed for yours. :)

brendon avatar Oct 25 '16 20:10 brendon

By the way, did my PR also solve your problem? I've not looked in detail but can see they deal with things a little differently.

brendon avatar Oct 25 '16 20:10 brendon

I do not believe it would, also I don't really agree with the premise.

It doesn’t make sense to trigger fresh waypoints that are already outside of the visible area of the scrollable context

Since the code your editing applies to all waypoints, not only Inview ones.

Consider a sticky navbar, if the user loads the page with an anchor link the fresh waypoint might be created after the browser already has scrolled past the point at which the navbar becomes sticky. Currently this would result in the waypoint getting triggered because of the freshWaypoint && axis.oldScroll >= waypoint.triggerPoint check

With your solution I believe the navbar would not get stuck until the user scrolled all the way up and then down again.

What I am instead suggesting is that the fresh waypoint logic not only gets applied waypoints created above the current scroll position, but also for once created below.

This might but us half way towards solving your issue, but I am very unsure about that.

JonatanJJ avatar Oct 26 '16 07:10 JonatanJJ

Thanks @Jontis00, yes I see that depending on the use case, default triggering might be a desirable thing, as you demonstrate. Perhaps this functionality should be an option that can be disabled then? In my example I don't want to trigger waypoints above where I am because (in my case) that would imply that the 'thing' had been seen which isn't the case.

brendon avatar Oct 26 '16 08:10 brendon