Skip to content
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

Virtual Scroll paints all items to DOM when the view are is 0 height. #123

Closed
aaronfrost opened this issue Sep 19, 2017 · 4 comments
Closed

Comments

@aaronfrost
Copy link
Contributor

In one of my views where I am using this component, if I go to the view a very specific way, the view takes 18 seconds to init. I finally found out what is going on... even though I haven't been able to create a plunker to reproduce it for you. Let me do my best to explain. I think it is an easy fix.

I have 20,000 items in an array. I am using virtual-scroll to efficiently display these items (thank you very much for this wonderful component). It is very efficient. In one specific scenario, however, instead of only painting the items that will fit in the viewport, it paints all 20,000 of them to the DOM, which is what causes the slow transition. The sad thing is... that immediately after wards, it updates and removes almost all of them, and repaints only the ones that are viewable.

When my component (that contains the virtual-scroll) inits, when the calculateDimensions runs, it returns 0 for viewWidth, viewHeight, childWidth, childHeight. This is problematic because in the function calculateItems sets the end to -1 when the view dimensions are 0. When end == -1, the following line of code creates the issue:

this.viewPortItems = items.slice(start, end);

when start is 0, and end is -1, and the items is an array of 20,000 things, it slices 19,999 of them, and sets the viewPortItems equal to that. This means that even though I am trying to use an efficient virtual scroll, I am in a scenario that allows me to paint all of the items to the screen.

Something like the following would prevent this edge case from happening:

this.viewPortItems = items.slice(start, end >= 0 ? end : 0);

This code should be added as a safety guard, in case anyone else gets inexplicably into the scenario that I am in. I have set heights and min-heights and everything. And no matter what I do the client-height is always zero (only in this one specific scenario... all others scenarios/click-paths are fine). It only happens when this view is the first view loaded in a lazy-loaded module. If it is the second view or beyond, it works fine. Only when it is the first view in the lazyloaded module. I load the app, and then go to the route in the lazyloaded module that contains the virtual-scroll. That is the only way I can reproduce it.

To make it worse, I'm not using the Angular-cli, I am using Brandon Roberts' lazy loader.

My point being, there are a lot of edge cases, and a lot of moving variables, so... we should add this line of code as a safety net so that people can't accidentally load all of the items that they are giving to the virtual-scroll. That should never happen, given the nature and goals of the component.

Thoughts?

aaronfrost added a commit to aaronfrost/angular2-virtual-scroll that referenced this issue Sep 19, 2017
… happen, even if you have hard coded heights and widths and such) then the virtual scroll would do a slice with a -1 as the end, which means to slice the entire array from the beginning to N-1. This is bad, cause if N = 20000, then this slice will add 19999 items to the viewPortItems, which will totally kill the viewPort, as it tries to paint 19999 items. This code change makes that impossible. If it ever tries to slice a -1 as the end, it will instead slice a 0 as the end, which will return an array of zero in length.
@aaronfrost
Copy link
Contributor Author

Added a PR for this. #124

@rhefner
Copy link

rhefner commented Jan 23, 2018

@aaronfrost: Also seeing something similar to this -- You find any workarounds (without patching node_modules) that can be used until your PR gets merged/released?

@rhefner
Copy link

rhefner commented Jan 23, 2018

Err, may have answered my own question - Looks like using <virtual-scroll ... [bufferAmount]="10"> keeps things sane.

rintoj added a commit that referenced this issue Jan 25, 2018
fix(#123) - When the element's height/width are zero (which can happe…
@speige
Copy link
Collaborator

speige commented Jul 19, 2018

Closing since PR has been merged & bufferAmount is another potential workaround. Please re-open if you're still experiencing this issue.

@speige speige closed this as completed Jul 19, 2018
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

3 participants