Fixed row height optimization
Based on issue https://github.com/angular-ui/ui-scroll/issues/220.
- Visibility-watcher feature might be optional.
- When a row height is fixed, the performance could be improved.
@dhilt We are intensively testing this optimization and found an issue with the allow-visibilty-watch flag.
=> How it manifests: Scrolling with the mouse wheel does not work when the scroller is first rendered. After we scroll once using the scrollbar, then it starts to work properly
=> Why it behaves like this:
Because the wheelHandler() event handler checks for scrollTop === 0 && !buffer.bof. At that time, bof==false and it prevents the scroll (both directions).
=> Why it works with the visibility watcher
Because visibilityWatcher() calls doAdjust() which fetches the rows before first (negative index in my case). This sets bof=true. It seems to be a side effect here that makes the wheel work.
I'm not sure what wheelHandler() is for. I suppressed it for testing and didn't see an issue. But I bet there is a good reason for it :-)
A solution is to detect the scroll direction and add that to each condition, like bellow:
const deltaY = event.deltaY;
if ((deltaY<0 && scrollTop === 0 && !buffer.bof) || (deltaY>0 && scrollTop === yMax && !buffer.eof)) {
But checking deltaY to find the direction is not reliable, according to MDN.
Any idea?
@priandsf There was the issue years ago (https://github.com/angular-ui/ui-utils/issues/239) and it was fixed with PR https://github.com/Hill30/NGScroller/pull/39, which is responsible for wheelHandler code. We also have demo http://localhost:5005/scrollBubblingPrevent/scrollBubblingPrevent.html and test case ("prevent unwanted scroll bubbling").
Well, I took v1.7.4, commented out
wrapper.unregisterVisibilityWatcher = wrapper.scope.$watch(() => visibilityWatcher(wrapper));
line and run the demo app. And I didn't see your problem with Mac's track pad (wheelHandler did work). I will try to reproduce your issue, but at first glance it's not so obvious, and it would be helpful if you might say how to reproduce it with minimal changes over the latest version (1.7.4).
@dhilt Ok I narrowed down the problem in our app. I cannot reproduce it yet on one of the samples ;-(
To make a long story short, it happens because buffer.effectiveHeight in enqueueFetch returns 0. It returns 0 because the outerHeight of every element is 0, although the elements are properly inserted in the DOM but not yet "digested" (resolved: the ng-repeat is not yet executed). Hard to understand why, but our data retrieval code triggers some $digest which led to this situation.
Nevertheless, I checked-in a fix that makes it works in our case: when row-height is provided, the buffer uses it to calculate the effective height. This is consistent with rest of the code.
@dhilt Hi Denis, I'll be happy if you can have a look at the row-height. I put many optimizations into it:
- faster height calculation
- scroll debouncing
- scrollTo() method to scroll to an absolute position
- more resistant to a scroll frenzy, particularly when the rows are slow to render We did some internal tests and so far we haven't found any issue. I added a new sample (Datasource as a service with fixed row height) to show the performance differences when scrolling through a large dataset. Let me know what you think.
@dhilt I uploaded our latest code to my branch if you want to give a look. We did sone extensive testing on our application and we haven't found any major issue.
@priandsf I will take a look when I have time, especially I think I'm most excited about the performance demo!