-
Notifications
You must be signed in to change notification settings - Fork 107
Recent Improvements to Scrolling has Removed Top Visible Item on scroll #70
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
This is also very apparent on the Preserves scroll position example |
So, the issue seems to be two fold. First, the calculateProperties function is only called on adjustBuffer which no longer gets called on every scroll event. This means that until it is called, no updates are sent to the adapter for the topVisibleItem. The second issue arrises if we try to include the calculate properties function outside of the adjust buffer function. In order to set the topVisible properties on the adapter, it requires the digest cycle to run which means we need to have the digest run on every single scroll event. I am running some tests to see if I can get it to only fire the digest cycle when the top visible item should be changed. |
this is strange - calculateProperties is called from both adjustBuffer and adjustBufferAfterFetch (see line 773) |
True, but the adjust buffer is no longer called during the scroll event (see lines 868-873). Only if new items are to be loaded does the adjustBuffer get called. |
I see your point. But then a direct call from scroll handler to calculate properties - in case there no new items coming should do the job - shouldn't it? If no new items are inserted, all items already in the buffer should already be fully instantiated. Am I missing something? |
As I mentioned above, the issue is with the digest cycle being required to set the topVisible properties. If you wrap calculateProperties in a timeout it works, but since we are trying to remove as many calls to the digest cycle as we can, it is counter productive to do this. I am exploring having the digest cycle only called if the topVisible item should change. |
Fixed by this commit |
With the recent improvements, the top visible item is no longer updated on scrolling. It is only updated when a call is made to the buffer because the calculateProperties function is now only called if the adjustBuffer is called. This is no longer called by default on the scroll event. Please see the link for a reproduction. If you scroll, the number should change when the top visible item is changed. You can see it only changes when new items are fetched from the datasource.
https://jsfiddle.net/ypdbmrt8/14/
I am working on a pull request right now. I am thinking that we should run the calculateProperties function in the resize scroll handler if the pending.length is not greater than 0.
The text was updated successfully, but these errors were encountered: