search

angular-ui / ui-scroll

Unlimited bidirectional scrolling over a limited element buffer for AngularJS applications
http://angular-ui.github.io/ui-scroll/demo/
MIT License
327 stars 107 forks source link

View jumps while scrolling up #205

Closed conraddamon closed 4 years ago

conraddamon commented 6 years ago

Sometimes when scrolling up, the view jumps. The behavior can be seen in any of the demos. Here's a screenshot video of it happening:

https://youtu.be/Sp7tjqWPc6I

Watch for when items 61 (and later, 96) come into view, and follow them with your eye. You'll see that they jump down quite a bit as scrolling continues. The mouse moves in circles to emphasize that the view has jumped.

conraddamon commented 6 years ago

I've traced through the code and I can see what's happening, though I'm not sure yet why or what the best fix is. To summarize, stale values for scrollTop in events that arrive after scrollTop has been adjusted in onAfterPrepend() cause the scroller to perform some extra prepends. After one to three extra data fetches, the scrollTop gets set to the value it should have had in the first place, but given the extra items that have been prepended, it's not nearly enough and the view is moved down.

  1. Scroll event triggers prepend. Scroll top is at 500, say.
  2. onAfterPrepend sets scrollTop to 900 to account for 400px worth of prepended items.
  3. Another scroll event arrives.
  4. Scroll top is 500 though it was just reset (!), making it appear that more items need to be prepended.
  5. Steps 2-4 repeat one to three times.
  6. At some point, scroll top reflects the value that was set (though it accounts for just one chunk of prepended items) and fetching stops.

My hazy guess at this point is that a few scroll events were in the event queue when scrollTop was reset, and that they reflect the state of the view before that happened, even though they arrive afterward. Or maybe setting scrollTop does nothing if scroll events are pending.

conraddamon commented 6 years ago

I've seen the jumping in the Mac versions of Chrome and Safari, but not Firefox. I haven't tried Edge or IE. I have a local fix which detects the failed setting of scrollTop and prevents additional prepending. Possible PR in a few days.

dhilt commented 6 years ago

@conraddamon Thanks for the issue, I can't reproduce it on Win 10 Chrome, but it definitely happens on Mac Chrome. Currently I give all my free time to ngx-ui-scroll, so PR would be great!

tiltsoftware commented 6 years ago

Hey at @conraddamon did you get a chance to look at a PR for this? Keen to see your fix if you don't mind sharing?

Thanks

conraddamon commented 6 years ago

https://github.com/angular-ui/ui-scroll/pull/206

dhilt commented 6 years ago

This PR had been merged into scroll-on-mac branch and the work isn't finished. Initial PR's code breaks some use cases and doesn't take into account the jQuery case. That's why we didn't see the tests fall, they use jQuery. I fixed jQuery case and some test cases, but it's not the end.

conraddamon commented 6 years ago

Let me know if there's anything I can look at. I'm not sure which use cases are broken. We're using the scroller with the fix for our product (a log analysis platform that does a lot of upward scrolling), and it has helped a lot.

dhilt commented 6 years ago

@conraddamon Hi Conrad! We are in a difficult situation. On the one hand your fix does really help. But from the other... it does not cover 100% of cases, and I'm able to reproduce the issue when the Datasource.get delay is big enough (current solution demo is available at rawgit).

I spent a lot of time digging into this problem during my work with ngx-ui-scroll last days. The problem stems from the inertia scroll implementation. Our issue can be reproduced on Chrome running on Mac with touchpad, so I would say that it relates to a combination of touch device, OS and Browser. In two words, inertia scroll ignores (sometimes) external scroll position updates. For example, let's imagine scrollTop change log:

// user inertia scroll process starts
400 - natural
398 - natural
396 - natural
394 - natural
// fetching/rendering rows and synthetic scroll position correction (+200)
594 - synthetic
392 - natural (synthetic was ignored)
390 - natural

In most cases the next position after synthetic change does meet the expectations (594 minus some delta, but not 594 minus 200 minus delta). But it is really possible that the inertia scroll process usurps the scrollTop node attribute and does not allow it to be changed programmatically. Such an usurpation is unpredictable and impermanent. The following log is also possible:

400 - natural
398 - natural
396 - natural
394 - natural
594 - synthetic
392 - natural (synthetic was ignored)
594 - synthetic (new smart correction)
390 - natural (new synthetic was ignored)
594 - synthetic (new smart correction again)
388 - natural (new synthetic was ignored)
594 - synthetic (new smart correction again)
592 - natural (win!)

Unfortunately, the correction procedure I imagine should be repeatable and depend on time parameter, and to overcome all possible inertia scroll cases this parameter should be pretty big (even 125ms does not fix 100%, while it already gives unacceptable side effects).

I'm going to continue my investigation and if you have any thoughts I would be glad to hear them. Meanwhile, do you want to see current results as, say, angular-ui-scroll v1.8.0-rc or we can postpone new release?

conraddamon commented 6 years ago

Hi Denis! Sorry for the delayed response, I was on vacation last week. I'd say go with whatever you feel is best as far as releases are concerned. The log management startup I work for (Scalyr) uses a customized version of ui-scroll, so we have the Mac scrolling fix as well as a few adapter extensions I added. We're a bit of an unusual use case in that we place the user at the bottom so most scrolling is upward. I added adapter methods to scroll to the bottom, center an item, and to call a method when the buffer is full and loading has stopped. Fortunately, our data fetches are fast so I never saw a lot of repeated failures in setting scrollTop.

I arrived at my fix by doing the same logging you've done. There's nothing particularly clever about the fix; it just checks to see if the synthetic setting of scrollTop worked and retries if it didn't. Since the root problem is in the browser, it's hard to think of anything but a reactive approach. I don't think there's anything in the scroll event object to indicate that it arrived via inertial scroll; I have a vague memory of thinking we could ignore inertial scroll events after setting scrollTop, until a non-inertial event arrives.

What is the nature of your time parameter? Is it a debounce period so we think inertial scroll events have stopped streaming in? Or is it a delay in setting scrollTop?

dhilt commented 4 years ago

angular-ui-scroll v1.8.0 is available at npm, it fixes this issue, full details can be found in the description of PR #238