-
Notifications
You must be signed in to change notification settings - Fork 107
View jumps while scrolling up #205
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
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.
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. |
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. |
@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! |
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 |
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. |
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. |
@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:
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:
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? |
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? |
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 |
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.
The text was updated successfully, but these errors were encountered: