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 lazy virtual scroll triggers multiple separate calls to onLazyLoad() function #9316

Closed
brian428 opened this issue Sep 25, 2020 · 4 comments
Assignees
Labels
LTS-FIXED-9.2.0 Fixed in PrimeNG LTS 9.2.0 LTS-FIXED-10.0.5 Fixed in PrimeNG LTS 10.0.5 Type: Bug Issue contains a bug related to a specific component. Something about the component is not working
Milestone

Comments

@brian428
Copy link
Contributor

brian428 commented Sep 25, 2020

I'm submitting a ... (check one with "x")

[X] bug report => Search github for a similar issue or PR before submitting
[ ] feature request => Please check if request is not on the roadmap already https://github.com/primefaces/primeng/wiki/Roadmap
[ ] support request => Please do not submit support request here, instead see http://forum.primefaces.org/viewforum.php?f=35

The current implementation of Table onScrollIndexChange() uses a forEach() loop to trigger separate calls to loadPage(). For example, on initial load of the table, when the pageRange is calculated to be [0,1], two separate lazy events are triggered. e.g. one load for rows 0-40 (page 0), and another load for 40-80 (page 1).

As a result, the user sees the table starting at row 40, not row 0.

(Edit:)
Here is a Stackblitz example of the problem. Load it and note that the ID of the first row is 101 and not 1. https://stackblitz.com/edit/primeng-tablevirtualscroll-lazy-bug?file=src%2Fapp%2Fapp.component.ts
The example used in the docs actually keeps ALL of the previously loaded pages in the virtual array, rather than just the rows for the last requested page. This isn't feasible for large data sets, since that virtual array will get gigantic for large data sets. My demo only keeps the current page's data in the virtual array, which is how anyone with a large data set would need to use the lazy virtual scrolling.

On a related note, as my bug demo shows, setting [totalRecords] seems to have no effect on informing the table what the total row count should be.
(end edit)

I'm not sure what the logic was to have onScrollIndexChange() trigger multiple different onLazyLoad() emits, but that seems to be the root of the problem. I'm also not sure what the point of using a range array is, since as far as I know, only one page at a time should ever be loaded.

So it looks like the two options to solve this would be:

A) Drop the use of pageRange and just use the index to load the page index being requested. E.g:

onScrollIndexChange = function( index: number ) {
  if( this.dt.lazy ) {
    this.loadPage( index );
  }
};

Or B) Move the emit for onLazyLoad() into onScrollIndexChanged() and only fire it once, with the cumulative data on the pages loaded. Something like:

onScrollIndexChange(index: number) {
    if (this.dt.lazy) {
        let pageRange = this.createPageRange(Math.floor(index / this.dt.rows));
        pageRange.forEach(page => this.loadPage(page));
        
        this.dt.onLazyLoad.emit({
            first: this.dt.rows * pageRange[0], 
            rows: this.dt.rows * pageRange.length,
            sortField: this.dt.sortField,
            sortOrder: this.dt.sortOrder,
            filters: this.dt.filters,
            globalFilter: this.dt.filters && this.dt.filters['global'] ? this.dt.filters['global'].value : null,
            multiSortMeta: this.dt.multiSortMeta
        });
    }
}
@venash
Copy link

venash commented Oct 5, 2020

Yes, I have same issue. When i scroll down over many data, onLazyLoad emit many unnecessary events.

Maybye component can emit onScrollIndexChange to write custom logic.

@jupin-r
Copy link
Contributor

jupin-r commented Oct 12, 2020

This issue is a big problem for me. I have a big dataset whitch is lazy loaded from server. If I scroll down to the middle of table's scrollbar, several events onLazyLoad are emitted. This events fires several request to the server e.g. ten requests, but I can not use any debounce strategy because no event is fired when I scroll up or down and already requested page is not requested again.

@cagataycivici cagataycivici self-assigned this Nov 10, 2020
@cagataycivici cagataycivici added the Type: Bug Issue contains a bug related to a specific component. Something about the component is not working label Nov 10, 2020
@cagataycivici cagataycivici added this to the 10.1.0 milestone Nov 10, 2020
@cagataycivici
Copy link
Member

@mertsincan I believe you fixed it for PRO users already?

@TKul6
Copy link

TKul6 commented Nov 18, 2020

Hi @cagataycivici ,
Is this fix is going to be release for version 10 as well.

I'm having the same issue. And I believe your solution will solve other issues I have as well.

Thanks

@yigitfindikli yigitfindikli added LTS-FIXED-10.0.5 Fixed in PrimeNG LTS 10.0.5 LTS-FIXED-9.2.0 Fixed in PrimeNG LTS 9.2.0 and removed LTS-PORTABLE labels Nov 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
LTS-FIXED-9.2.0 Fixed in PrimeNG LTS 9.2.0 LTS-FIXED-10.0.5 Fixed in PrimeNG LTS 10.0.5 Type: Bug Issue contains a bug related to a specific component. Something about the component is not working
Projects
None yet
Development

No branches or pull requests

6 participants