Large values for dataSource.maxIndex causes too many dataSource.get() calls

[m0j0nz0r]

Reproduce: Try creating a scrollable list where item height:600px and set dataSource.maxIndex = 10000000

Effect: It makes viewport.bottomDataPos() behave strangely causing the component to request way more items than it needs.

[dhilt]

@m0j0nz0r It is so due to maximum possible DOM element height is restricted by the browser. Each time the ui-scroll does render/clip elements, the height of padding elements is being adjusted. This adjustment is based on min/max indexes and item heights (https://github.com/angular-ui/ui-scroll/blob/v1.7.1/src/modules/viewport.js#L164).

I made some experiments on my Win 10 Chrome 67 and I found that the maximum value of the DOM element height is 33554400. It means if I set style.height to, say, 10e9, then the result value of style.height will be 33554400.

So, starting from some point, item height and min/max indexes that are big enough break the ui-scroll internal calculations because padding elements heights become invalid.

[m0j0nz0r]

That is exactly right. Does that make it a non-issue?

[dhilt]

@m0j0nz0r Why and how do you think we should handle such a situation on the ui-scroll side?

[m0j0nz0r]

Why: the whole point of using an infinite scroll that loads/destroys records as needed is to be able to display very long lists efficiently. This falls into such a use-case. (In my specific case it's a list that can be filtered so the user can find whatever he needs easily and quickly)

How: The only reason to use maxIndex to determine if you should load more records or not is to determine EOF. I'm not familiar enough with the ui-scroll code to understand why does it use the bottom padding size, which is derived from maxItems, to determine if it should load more records or not, but in any case this can calculated without having to rely on the DOM to do it for you (and screwing the result for very large lists while at it).

Alternatively you can also calculate if a determined list item is visible within the viewport without using the bottom padding size at all or even creating the item in the DOM (thus preventing an unnecessary update to the DOM) with something like:

var itemTop = itemIndex * avgItemHeight; var itemBottom = itemTop + avgItemHeight; var isItemInViewport = itemBottom > viewport.scrollTop() && itemTop < viewport.scrollTop() + viewport.height;

For anyone else who might be having the same issue. The workaround I'm currently using is hardCapping maxIndex to something like Math.floor(33554400 / avgItemHeight) - 1

[dhilt]

@m0j0nz0r Bottom (and top) paddings are necessary for proper scrollbar parameters. Virtual items are being emulated via these paddings. The final height of the scrollable element must satisfy the cumulative height of all indexed items. Otherwise, we'll get inconsistent UI.

Your workaround does not take into the account current Browser specifics. The floor value should be calculated dynamically, it should be Browser specific. For example, here are some thoughts on how to get such a limit with the binary search.

Also, it's a good idea for tiny client side lib, getting max possible DOM element size. Probably we may include it in the ui-scroll initialization process, if it does not take, say, more than 150ms... or make this check optional. But for now I would prefer to handle this kind of requirement outside the ui-scroll.

[Jerczu]

I don't understand why argue over stuff - fork the repo @m0j0nz0r and modify the library to do what you need it to do. I did that and use my own fork of the UI-Scroll as it not necessarily cover my use case for it. I modified the repo in my own fork and actively change it to fit my use case.

[m0j0nz0r]

@dhilt Thanks for the binary search idea. Very useful. In my box it takes around 100ms to find the maximum element size with binary search.

@Jerczu I'm sorry if I came across as argumentative. I simply thought it would have been an improvement for the library as a whole.

In any case, thanks for your input, it is very appreciated. Have an awesome day!