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

Table: VirtualScroll + Lazy Load incorrectly paginates load requests, displays loadingbody too frequently #11789

Closed
john8329 opened this issue Aug 7, 2022 · 27 comments
Assignees
Labels
Type: Enhancement Issue contains an enhancement related to a specific component. Additional functionality has been add
Milestone

Comments

@john8329
Copy link

john8329 commented Aug 7, 2022

Describe the bug

This behavior seems a regression from the previous virtualscroll implementation. You can verify in the PrimeNg demo that when scrolling a table with lazy loading and virtual scrolling, the rows very frequently become their loadingbody templates, especially when reaching specific thresholds.

Untitled.mp4

Environment

nil

Reproducer

No response

Angular version

14.1.1

PrimeNG version

14.0.0

Build / Runtime

Angular CLI App

Language

TypeScript

Node version (for AoT issues node --version)

14.19.3

Browser(s)

No response

Steps to reproduce the behavior

Please check the video. Scrolling on the simplest table implementation reproduces this issue.

Expected behavior

I'd expect only the newly loaded rows to behave like that, and to load them before they appear in the view for a better user experience. With a normal implementation like this, the user would see the table very frequently reload its rows, which is unacceptable.

@john8329 john8329 added the Type: Bug Issue contains a bug related to a specific component. Something about the component is not working label Aug 7, 2022
@cetincakiroglu cetincakiroglu removed the Type: Bug Issue contains a bug related to a specific component. Something about the component is not working label Aug 8, 2022
@cetincakiroglu
Copy link
Contributor

Hi,

This is by design, the loading body has to be unique for each user depending on the API and request logic. This is just a dummy example to show what it seems like. You can customize it to your needs.

@john8329
Copy link
Author

john8329 commented Aug 8, 2022

Maybe I expressed myself badly, the loading body shows too frequently, in the previous versions I could scroll slowly and smoothly without the list becoming unusable so frequently and making the content disappear. It looks it's showing all elements of the list as loading, while it should show only the actual ones that get loaded at the end of the current view.

@john8329
Copy link
Author

Can you guys please check this? It's blocking the upgrade of our project, and until it gets fixed we have to stay with LTS. Thanks

@john8329
Copy link
Author

I'm sorry for insisting, but can you please reopen the issue? The bug is present, significant, clear and reproducible, and no matter all the research I'm trying to do on your source code, I'm unable to find a cause. Thanks

@mertsincan mertsincan reopened this Aug 17, 2022
@mertsincan mertsincan self-assigned this Aug 17, 2022
@mertsincan mertsincan added the Status: Pending Review Issue or pull request is being reviewed by Core Team label Aug 17, 2022
@mertsincan
Copy link
Member

No worries! Could you please change numToleratedItems according to your needs?

@john8329
Copy link
Author

It seems that adding the options doesn't change the issue, the table still shows everything as loading in the exact moment it tries to fetch a new chunk.

@john8329
Copy link
Author

I've made for your convenience a stackblitz example here showing now only that the loading body shows whenever the chunk is loaded, but if you check the console, the lazy load requests are strange.

For example, I'd expect the list to ask for 0-50, 51-100, 101-150, instead it seems to go wild and asks like 0-48, 5-74, 29-98, 67-136. There's a lot of overlap, the client is basically requesting records it already asked for, and that's wildly inefficient compared to the previous implementation. I presume it might be a possible cause for this glitch, since it asks for rows in the current view.

It's perfectly reasonable to build the request size from the height, virtualScrollItemSize and numToleratedItems, not so much to keep requesting the same data.

@edriburk
Copy link

This is also an issue for us also. We are not able to upgrade our project to v14 until this is fixed.

@mrp67
Copy link

mrp67 commented Aug 30, 2022

I'm experiencing a similar issue with the index and chunk size is all over the place. Even with 100 items loaded into the table, the lazy load event gets triggered after scrolling past a low number of rows. This behavior occurs at a variety of scroll heights and other settings. Lazy loading works exactly as expected with pagination, leading me to believe the issue is more related to virtual scroll or the combination of virtual scroll with lazy loading

@john8329
Copy link
Author

I've checked it's definitely related to the combination of virtual scrolling and lazy loading. It doesn't happen in any other case.

EDIT: I mistakingly closed the issue

@john8329 john8329 reopened this Aug 30, 2022
@joelcorrales
Copy link

Same problem here, when you combine virtualScroll and lazy loading the load of pages is all over the place, no matter the configuration of the scroller options.

@woppa684
Copy link

woppa684 commented Sep 1, 2022

We have also encountered this issue indeed. Apart from that, it seems the scroller re-inits when the items change, then triggers a lazy load, that again causes the items to change. Hence, we arrive in some infinite loop of updates.

EDIT:
The stackblitz on the site (https://www.primefaces.org/primeng/table/virtualscroll) contains

//trigger change detection
this.virtualCars = [...this.virtualCars];

and the source code on the same page contains

//trigger change detection
event.forceUpdate();

I assume that this has changed then, since the second method does not trigger the infinite update-loop, but the stackblitz has not been updated? By the way, the infinite loop was introduced in d0626b1, since it no longer checks whether the lengths of the previous and current value differ (which indeed wasn't a correct check anyway, but now causes the "old" change detection trigger to trigger a bit more than that). See the stackblitz here for the inifite loop (small adaptation of the one made by @john8329)

Sorry to hijack the subject a bit, still curious to see an answer to the original question because that bothers us as well :) Should I make a separate issue out of this? All and all this is preventing us to upgrade to 14.

EDIT2: I see the stackblitz on the site still uses PrimeNG 13, so no infinite loop there

@joelcorrales
Copy link

joelcorrales commented Sep 1, 2022

Yes, I think both problems are related, the infinite loop for the update of [value] and the small increments when scrolling, also we had [rows] property in the table, that is completely ignored in the new scroller component.

@john8329 john8329 changed the title Table: VirtualScroll + Lazy Load shows loadingbody too frequently Table: VirtualScroll + Lazy Load incorrectly paginates load requests, displays loadingbody too frequently Sep 13, 2022
@john8329
Copy link
Author

Any update from the devs? The issue seems serious and is definitely blocking adoption for several users. Thanks

@geffreyw
Copy link

geffreyw commented Sep 29, 2022

Following, we also have troubles with the virtual scroller on lazy mode if we upgrade to version 14.1.2. Currently I'm trying to work around this issue, but when scrolling up after all items were loaded gives some weird behavior. When logging all things happening, I can see that the first and rows property of the lazyLoadEvent aren't what you'd expect.

Please provide us with an update.

@pete-mcwilliams
Copy link
Contributor

Also experienced issue after the update with infinite loop loading when replacing the table data to trigger change detection. ( data = [...data])
Also experiencing the loading skeleton displays on rows already loaded in a previous query... as I scroll down slowly I expect only the new rows scrolled to to be displayed as skeletons, not all the rows table.

I am using [lazy]="true" and [virtualScroll]="true".

@BenjaminWiemann
Copy link

BenjaminWiemann commented Oct 14, 2022

We also have this Issue. Any idea how long till it could be fixed? Does anyone have a temperary workaround that can be used?

@joelcorrales
Copy link

No solutions for now, basically [lazy] + [virtualScroll] is completely broken in last version (v14.1.2).

@joelcorrales
Copy link

joelcorrales commented Oct 27, 2022

@john8329 Can you add the tag defect again? Or maybe close this ticket and reopen it, it's seems to me like the debs took a look at this, removed the defect flag and just ignore the rest of the comments and updates...

If you create a new issue, please add the link in this so we can subscribe to it too.

@john8329
Copy link
Author

I don't think I'm able to add labels, and it's still "under review", so this is being purposefully ignored anyway. I prefer not to open another ticket, to keep the history clear. I believe the best thing to do is to poke @cetincakiroglu and ask kindly if they could take a look and add the tag again.
I moved to another table implementation anyway, this always gave me headaches and keeps changing.

@mertsincan
Copy link
Member

mertsincan commented Nov 7, 2022

Hi all,

Sorry for the delayed response! I made some changes to this. The rows prop can be used to load items in a specific order. Exp;

<p-table [columns]="cols" [value]="virtualCars" [scrollable]="true" scrollHeight="250px" [rows]="100"
            [virtualScroll]="true" [virtualScrollItemSize]="46" [lazy]="true" (onLazyLoad)="loadCarsLazy($event)">
            <ng-template pTemplate="header" let-columns>
                <tr>
                    <th *ngFor="let col of columns" style="width: 20%;">
                        {{col.header}}
                    </th>
                </tr>
            </ng-template>
            <ng-template pTemplate="body" let-rowData let-columns="columns">
                <tr style="height:46px">
                    <td *ngFor="let col of columns">
                        {{rowData[col.field]}}
                    </td>
                </tr>
            </ng-template>
            <ng-template pTemplate="loadingbody" let-columns="columns">
                <tr style="height:46px">
                    <td *ngFor="let col of columns; let even = even">
                        <p-skeleton [ngStyle]="{'width': even ? (col.field === 'year' ? '30%' : '40%') : '60%'}"></p-skeleton>
                    </td>
                </tr>
            </ng-template>
        </p-table>

@mertsincan mertsincan added Type: Enhancement Issue contains an enhancement related to a specific component. Additional functionality has been add and removed Status: Pending Review Issue or pull request is being reviewed by Core Team labels Nov 7, 2022
@mertsincan mertsincan added this to the 14.2.0 milestone Nov 7, 2022
@dshpindler
Copy link

dshpindler commented Feb 8, 2023

The same issue still exists on latest version, any workaround to it?
any way to handle the lazy loaded manually? without showing the loading body at all?

even if I set the value to cars in your demo, every time I scroll, the loading skeleton shows

@john8329
Copy link
Author

Also see #12206

@jessica-ewald
Copy link

I'm also having issues with virtualScoll + lazy loading. In my case, the table will only show a maximum of rows*4. So if rows = 100, it appears to lazy load after I hit 100, but I can only scroll to the 400th row.

If I set rows to my data length/4, I can access everything but then the chunk size is so large that it's not really worth it.

@pete-mcwilliams
Copy link
Contributor

Make sure you create a sparse array for the entire count of the records, then fill that in with the lazy loaded data.

this.dataRows = Array(getCount());

lazy...
this.dataRows.splice(
this.searchCriteria.offset,
this.searchCriteria.limit,
...returnResult.results
);

@joelcorrales
Copy link

This was closed but only the broken loading process was fixed, the main issue where you have a list of unknown amount of items to be loaded page by page as you scroll is still there, a better description of the issue HERE

@therealsquidgee
Copy link

@mertsincan perhaps I am missing something but your fix doesn't appear to be working for me. We have been setting rows all along. I noticed from testing before vs after the upgrade that the lazyload event fires a lot more and with smaller parameters for rows in the upgraded version.

I increased the value of rows which did make the flickering effect (loadingbody appearing more frequently than before the upgrade) happen less, but it still is showing up a lot more than before upgrading. Any ideas on what else I can do or where else to look for what change caused this? Thanks :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Enhancement Issue contains an enhancement related to a specific component. Additional functionality has been add
Projects
None yet
Development

No branches or pull requests