Skip to content

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

Closed
observstream opened this issue Mar 22, 2016 · 7 comments
Closed

Comments

@observstream
Copy link

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.

@observstream
Copy link
Author

This is also very apparent on the Preserves scroll position example

@observstream
Copy link
Author

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.

@mfeingold
Copy link
Contributor

this is strange - calculateProperties is called from both adjustBuffer and adjustBufferAfterFetch (see line 773)

@observstream
Copy link
Author

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.

@mfeingold
Copy link
Contributor

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?

@observstream
Copy link
Author

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.

@observstream
Copy link
Author

Fixed by this commit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants