dhilt / ngx-ui-scroll

Virtual/infinite scroll for Angular
https://dhilt.github.io/ngx-ui-scroll/
MIT License
222 stars 19 forks source link

Component seems to auto scroll on calling adapter.reload #288

Closed ThomasDevens closed 2 years ago

ThomasDevens commented 2 years ago

(Angular 11.2.13) Small repro of my setup: typescript:

public datasource: Datasource = new Datasource({
        get: (index: number, count: number, success: Function) => {
            if (this.listItems == null) {
                return success([]);
            }
            let data = [];
            let start = Math.max(index, 0);
            let end = Math.min(count + index - 1, this.listItems.length - 1);
            for (let i = start; i <= end; i++) {
                data.push(this.listItems[i]);
            }
            return success(data);
        },
        settings: {
            minIndex: 0,
            startIndex: 0,
            bufferSize: 10,
            padding: 0.3
        }
    });

.....
ngOnInit() {
    (asynchronous rxjs stuff).subscribe(items => {
        this.listItems = items;
        this.datasource.adapter.reload();
    });
}

template:

<ul class="card-body pl-3 tree-view-card" *ngIf="listItems?.length > 0" role="tree">
    <some-custom-component  *uiScroll="let item of datasource" [item]="item"></some-custom-component>
</ul>

I recently updated from 1.7.0 to 1.11.1 and started to encounter an issue where the list scrolls down to around the 8th item (same every time, but seems arbitrary) whenever adapter.reload is called. I haven't changed anything since updating, so I'm wondering if there is some small change I may have missed and need to fix in my implementation. FWIW everything else appears to work normally, and trying to update Angular to 12 and this to the latest appears to still have the same issue.

dhilt commented 2 years ago

@ThomasDevens Why not to use 2.2.2 instead of 1.11.1? There are no breaking changes in the API, the main change is that the core is extracted into a separate package (vscroll), so v2 should be fully backward-compatible with v1. Also, if it's a library bug, it is easier to fix it in v2 rather than in v1 and v2.

I have not found anything suspicious in your sample. Perhaps lack of infrastructure... Maybe dev log can help. Also, it would be great if you could fork this demo and make stable repro.

ThomasDevens commented 2 years ago

@dhilt Thanks for the quick response! I have tried upgrading to 2.2.2 and experienced the same issue, but will stick with the newer version going forward.

As for the code above, I tried to cut out anything that's not related to ui-scroll (mostly to see if it was just a simple change I was missing) - it's worth noting that the items in the list are setup as expandable/collapsible nodes with children that are also expandable, so it might be an issue related to that.

FWIW, here is the debug logging after loading the page (same result both with and without the adapter.reset() call it seems):

vscroll Workflow has been started, core: vscroll v1.3.2, consumer: ngx-ui-scroll v2.2.2, workflow instance: 1, adapter instance: 1
vscroll.esm5.js:3113 fallback startIndex to settings.startIndex (0)

vscroll.esm5.js:3113 vscroll settings object { adapter: false, startIndex: 0, minIndex: 0, maxIndex: Infinity, itemSize: NaN, bufferSize: 10, padding: 0.3, infinite: false, horizontal: false, windowViewport: false, viewportElement: null, inverse: false, onBeforeClip: null, sizeStrategy: average, debug: true, immediateLog: true, logProcessRun: false, logTime: false, throttle: 40, initDelay: 1, initWindowDelay: 40, cacheData: false, cacheOnReload: false, dismissOverflowAnchor: true, directionPriority: backward, instanceIndex: 1, initializeDelay: 1, viewport: null }

vscroll.esm5.js:3113 setting scroll position at 0 [cancelled]
vscroll.esm5.js:3113 stat initialization, pos: 0, size: 184, bwd_p: 0, fwd_p: 184, default: no, items: 0, range: no
vscroll.esm5.js:3113    ~~~ WF Cycle 1-1 STARTED ~~~  
vscroll.esm5.js:3113 ---=== loop 1-1-1 start
vscroll.esm5.js:3113 skipping fetch backward direction [initial loop]
vscroll.esm5.js:3113 forcing fetch forward direction [no item size]
vscroll.esm5.js:3113 fetch interval: [0..9]
vscroll.esm5.js:3113 fetch direction is "forward"
vscroll.esm5.js:3113 going to fetch 10 items started from index 0
vscroll.esm5.js:3113 resolved 10 items (index = 0, count = 10)
vscroll.esm5.js:3113 stat before new items render, pos: 0, size: 184, bwd_p: 0, fwd_p: 184, default: no, items: 0, range: no
vscroll.esm5.js:3113 default size has been updated: 31
vscroll.esm5.js:3113 stat after new items render, pos: 0, size: 495, bwd_p: 0, fwd_p: 184, default: 31, items: 10, range: [0..9]
vscroll.esm5.js:3113 skipping clip [initial cycle]
vscroll.esm5.js:3113 forward padding will be increased by 184 to fill the viewport
vscroll.esm5.js:3113 stat after paddings adjustments, pos: 0, size: 495, bwd_p: 0, fwd_p: 184, default: 31, items: 10, range: [0..9]
vscroll.esm5.js:3113 ---=== loop 1-1-1 done, loop 1-1-2 start
vscroll.esm5.js:3113 start delta is -134  (+-134 offset)
vscroll.esm5.js:3113 fetch interval: [0..25]
vscroll.esm5.js:3113 fetch interval after Buffer flushing: [10..25]
vscroll.esm5.js:3113 fetch direction is "forward"
vscroll.esm5.js:3113 going to fetch 16 items started from index 10
vscroll.esm5.js:3113 resolved 16 items (index = 10, count = 16)
vscroll.esm5.js:3113 stat before new items render, pos: 0, size: 495, bwd_p: 0, fwd_p: 184, default: 31, items: 10, range: [0..9]
vscroll.esm5.js:3113 default size has been updated: 35
vscroll.esm5.js:3113 stat after new items render, pos: 0, size: 1087, bwd_p: 0, fwd_p: 184, default: 35, items: 26, range: [0..25]
vscroll.esm5.js:3113 skipping clip [initial cycle]
vscroll.esm5.js:3113 stat after paddings adjustments, pos: 0, size: 903, bwd_p: 0, fwd_p: 0, default: 35, items: 26, range: [0..25]
vscroll.esm5.js:3113 first index = 4, delta = -3
vscroll.esm5.js:3113 setting scroll position at 134
vscroll.esm5.js:3113 stat after scroll adjustment, pos: 134, size: 903, bwd_p: 0, fwd_p: 0, default: 35, items: 26, range: [0..25]
vscroll.esm5.js:3113 ---=== loop 1-1-2 done, loop 1-1-3 start
vscroll.esm5.js:3113 start delta is -134  (+-134 offset)
vscroll.esm5.js:3113 fetch interval: [1..33]
vscroll.esm5.js:3113 fetch interval after Buffer flushing: [26..33]
vscroll.esm5.js:3113 fetch interval after bufferSize adjustment: [26..35]
vscroll.esm5.js:3113 fetch direction is "forward"
vscroll.esm5.js:3113 going to fetch 10 items started from index 26
vscroll.esm5.js:3113 resolved 10 items (index = 26, count = 10)
vscroll.esm5.js:3113 stat before new items render, pos: 134, size: 903, bwd_p: 0, fwd_p: 0, default: 35, items: 26, range: [0..25]
vscroll.esm5.js:3113 default size has been updated: 110
vscroll.esm5.js:3113 stat after new items render, pos: 134, size: 3972, bwd_p: 0, fwd_p: 0, default: 110, items: 36, range: [0..35]
vscroll.esm5.js:3113 skipping clip [initial cycle]
vscroll.esm5.js:3113 stat after paddings adjustments, pos: 134, size: 3972, bwd_p: 0, fwd_p: 0, default: 110, items: 36, range: [0..35]
vscroll.esm5.js:3113 first index = 8, delta = -17
vscroll.esm5.js:3113 skipping scroll 134 [pre-synthetic]
vscroll.esm5.js:3113 setting scroll position at 268
vscroll.esm5.js:3113 stat after scroll adjustment, pos: 268, size: 3972, bwd_p: 0, fwd_p: 0, default: 110, items: 36, range: [0..35]
vscroll.esm5.js:3113 ---=== loop 1-1-3 done, loop 1-1-4 start
vscroll.esm5.js:3113 start delta is -134  (+-134 offset)
vscroll.esm5.js:3113 fetch interval: [6..30]
vscroll.esm5.js:3113 fetch interval after Buffer flushing: no
vscroll.esm5.js:3113 ---=== loop 1-1-4 done
vscroll.esm5.js:3113    ~~~ WF Cycle 1-1 FINALIZED ~~~  
vscroll.esm5.js:3113 skipping scroll 268 [synthetic]

Looking at this, I adjusted the buffer size from 10 to 35 and it seems to work fine now - might just need to do some testing to figure out a good number to use there as I've just been using 10 as the default across all my implementation of this

dhilt commented 2 years ago

@ThomasDevens I tried to replicate your infrastructure and made this demo. If you open it, you will see that the log is the same until the following line, which is present in your log and not present in my:

forward padding will be increased by 184 to fill the viewport

This message appears only if there are not enough items to fill the viewport after fetch and render, and I don't see how this is possible if the previous steps and params are the same. Maybe styles? Scroller relies on getBoundingClientRect API, and maybe there are some cases, some specific styling, when it mistakes. Can you apply your CSS to the demo?

ThomasDevens commented 2 years ago

I don't have anything applied that would affect height/padding/margin of the individual items - It almost certainly has to do with the possibility of each item to have expanded children when loaded e.g.

Like I said in my previous comment however, changing the buffer size does seem to fix the issue

dhilt commented 2 years ago

@ThomasDevens By increasing the bufferSize setting you tell the Scroller to load more items initially, and the physical gap we found on the first inner loop does not appear. But this approach has a side-effect: minimal load size increases too. So if you need at some point, for example, only 2 items to complete the view and bufferSize is 40, then you'll get 38 extra items. Of course, they all will be consistently in the hidden area and it might not be a problem at all, but you should not be surprised why do you get so many items from time to time in the DOM.

I'd prefer to research this initial gap problem, as it may cover something important. If you can give me a runnable repro, please post the details, we'll reopen this issue and fix it.