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 not called on last page #8890

Closed
lukgit17 opened this issue May 26, 2020 · 25 comments
Closed

Table lazy virtual scroll not called on last page #8890

lukgit17 opened this issue May 26, 2020 · 25 comments

Comments

@lukgit17
Copy link

[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

**Current behavior**
p-table not fire event (onLazyLoad) when scroll to end

**Expected behavior**
When scroll to end of rows p-table must fire event onLazyLoad to load new records from remote data source.

Angular version: 9.1.3
PrimeNG version: 9.1.0
Browser: all 
@morula
Copy link

morula commented May 26, 2020

Same problem.

@mnhock
Copy link

mnhock commented May 27, 2020

Same problem here.

3 similar comments
@manuelmoyaroldan
Copy link

Same problem here.

@beleta
Copy link

beleta commented May 31, 2020

Same problem here.

@dsebastien
Copy link

Same problem here.

@dsebastien
Copy link

dsebastien commented Jun 8, 2020

@cagataycivici can you tell us if this scenario has been tested?
We've looked at this the whole morning and couldn't figure out what goes wrong.

In our case, we're using the new "flex" scrollHeight property, within a container that has a dynamic height (basically starts where it is and expands up to the bottom of the page) set with calc(...).

But even after switching back to a fixed size in px for scrollHeight, we've noticed that the table still doesn't fire the onLazyLoad calls; apart from the initial one.

Side note: I feel like your components should use the coerceBoolean utility provided by the Angular CDK to avoid misinterpreting booleans passed as strings, which are truthy (non empty strings).

@NicolasRigo
Copy link

Same problem here...

@jdc0331
Copy link

jdc0331 commented Jun 9, 2020

I'm having the same issue

@dinhtruonggiang
Copy link

I'm having same problem with virtual scroll & lazy load (data from backend) in p-table.

@kali-dali
Copy link

kali-dali commented Jun 24, 2020

Having the same issue too.

I looked into the table code and found out, that virtual scrolling is explicitly preventing lazy loading at ngOnInit phase:

ngOnInit() {
        if (this.lazy && this.lazyLoadOnInit) {
            if (!this.virtualScroll) {
                this.onLazyLoad.emit(this.createLazyLoadMetadata());
            }
            
            ...
        }

        this.initialized = true;
}

I'm going to look deeper into it for finding the reason that the scrolling does not activate the lazy loading.

EDIT:
It seems that the overall issue of the lazy loading not working with virtual scroll has already been fixed for the next version (9.1.1). If you clone the repository anf the run the showcase, then the virtual scroll with lazy loading works just fine, except for the initial load.

I would consider this issue as closed then?

@beleta
Copy link

beleta commented Jul 1, 2020

No, the problem still shows in version 9.1.2, and does not only affect the initial load.

This is a MAJOR problem.

@mnhock
Copy link

mnhock commented Jul 1, 2020

We stuck on version 9.0.6 as well and not able to update to all newer version due to this issue.

@beleta
Copy link

beleta commented Jul 1, 2020

I am opening a new issue to try to attract attention on it, because it seems that nobody at Prime cares about it.

@dinhtruonggiang
Copy link

We still encounter the virtual scroll issue with lazy load from server after upgrading primeng to 9.1.2. Please spend more time to fix it. Thanks.

@kali-dali
Copy link

I had hoped that the issue would be fixed with the new version 9.1.2 since the online demo table seems to work fine.
https://primefaces.org/primeng/showcase/#/table/virtualscroll

@chromey
Copy link

chromey commented Aug 3, 2020

I was about to subscribe to this issue because I experienced the same problem, but then I noticed there's simply a change in the API between the pre-CDK version of the virtual scrolling and the one since 9.1, so just in case you guys (or any other reader) fell into the same trap:

Previously the [value] attribute would be set to a dense array containing only the currently fetched slice of data.
In the new version [value] instead represents the complete dataset. It starts out as a sparse array containing only undefined values, and as data is fetched you fill slices of the array identified by the indices in the LazyLoadEvent.

Take a close look at the demo implementation: https://stackblitz.com/edit/primeng-tablevirtualscroll-demo?file=src%2Fapp%2Fapp.component.ts

HTH

@Casimodo72
Copy link

Casimodo72 commented Aug 6, 2020

I can confirm @chromey's observation. I noticed that just one dummy entry at the end of the array is enough to make virtual scrolling work. I'm using the following in an OData helper model for the table.
(The code does not work in an other scenario though: when "event.first" becomes zero again (e.g. when the sort field changes). May be a different problem).

const loadedItems = response.value;

if (opts.isVirtualScrolling) {

    const currentItems = opts.event.first !== 0 ? this.items : [];

    if (currentItems.length && currentItems[currentItems.length - 1] === undefined) {
        // Remove last dummy entry.
        currentItems.pop();
    }

    // Since we want to avoid using OData $count with virtual scrolling:
    //   total count is reached when we get less items than we requested.
    const isTotalCountReached = loadedItems.length < opts.event.rows;

    // Concat items and trigger change detection.
    // If we want more items to be loaded: add one dummy entry.
    this.items = Array.prototype.concat(currentItems, loadedItems, isTotalCountReached ? [] : Array.from({ length: 1 }));

    // console.debug(`## OData: previous: ${currentItems.length}, loaded: ${loadedItems.length}, current: ${this.items.length}, ` +
    //     `end reached: ${isTotalCountReached ? "yes" : "no"}, total: unknown`);   
}
else {
    this.items = loadedItems;
    this.pager.itemIndex = opts.event.first || 0;
    this.pager.totalCount = response["@odata.count"] || 0;

    // console.debug(`## OData: loaded: ${loadedItems.length}, total: ${this.pager.totalCount}`);
}

@kali-dali
Copy link

@Casimodo72 But how would you do that with an Observable stream? For example when you pass data from a http request directly into the p-table. I don't see how your approach is a good solution to getting virtual scrolling to work.

Why don't they just rely on the totalRecords Attribute when virtual scrolling is active, like with the paginator?

@Casimodo72
Copy link

Casimodo72 commented Aug 11, 2020

@mrdlink :

But how would you do that with an Observable stream? For example when you pass data from a http request directly into the p-table

Not sure what you mean exactly. I use a tiny helper class that acts as a view model for collection based components. PrimeNg's table component binds to the "items" field on my view model.
Here's the complete function where an HTTP request is performed. Is this what you meant?

private loadCore(opts: PrimeNgCollectionLoadingOptions): Observable<ODataResult<TItem[]>> {

    return new Observable((observer: Observer<ODataResult<TItem[]>>) => {
        const queryInfo = this.buildODataInfo(opts);

        this.isLoading = true;

        return this.httpClient.get<ODataResult<TItem[]>>(queryInfo.url)
            .pipe(finalize(() => {
                this.isLoading = false;
            }))
            .subscribe((response: ODataResult<TItem[]>) => {
                const loadedItems = response.value;

                if (false) {
                    // TODO: REMOVE: Just for dev purposes.
                    let index = queryInfo.skip;
                    for (const item of loadedItems) {
                        index++;
                        item["index"] = index;
                    }
                }

                if (opts.isVirtualScrolling) {

                    const currentItems = queryInfo.skip !== 0 ? this.items : [];

                    if (currentItems.length && currentItems[currentItems.length - 1] === undefined) {
                        // Remove last dummy entry.
                        currentItems.pop();
                    }

                    // Since we want to avoid using OData $count with virtual scrolling:
                    //   total count is reached when we get less items than we requested.
                    const isTotalCountReached = loadedItems.length < queryInfo.top;

                    // Concat items and trigger change detection.
                    // If we want more items to be loaded: add one dummy entry.
                    this.items = Array.prototype.concat(currentItems, loadedItems,
                        isTotalCountReached ? [] : Array.from({ length: 1 }));

                    // TODO: REMOVE:  console.debug(`## OData: previous: ${currentItems.length}, ` +
                    //     `loaded: ${loadedItems.length}, current: ${this.items.length}, ` +
                    //     `end reached: ${isTotalCountReached ? "yes" : "no"}, total: unknown`);
                }
                else {
                    this.items = loadedItems;
                    this.pager.itemIndex = queryInfo.skip;
                    this.pager.totalCount = response["@odata.count"] || 0;

                    // TODO: REMOVE:  console.debug(`## OData: loaded: ${loadedItems.length}, total: ${this.pager.totalCount}`);
                }

                observer.next(response);
                observer.complete();
            }, err => observer.error(err));
    });
}

@kali-dali
Copy link

@Casimodo72 Yes this is what I meant. It seems a bit hacky though.

@Casimodo72
Copy link

@mrdlink : You are right, it seems hacky - because it is incorrect :-) I just played a bit more with it and learned that is doesn't matter if I add a dummy entry at the end or not. So one can happily remove that.

What I also experienced: Sometimes the lazy loading with virtual scrolling does not work initially for me. Maybe that's why I got confused and tried to add the dummy entry.
Plus it still also stops working for me when the table is sorted.

By the way: I'm using PrimeNg v10.0.0-rc.1. Forgot to mention that.

@TodorKlasnakov
Copy link

+1

@kali-dali
Copy link

@Casimodo72 Okay, but this still does not work for me, because I use OnPush change detection in my component. Even if I have an array of values to pass to the p-table and change the reference on each lazy load event, the view will not update.
It will only update if I pass the table items as an observable and subscribing using the async pipe.

@TodorKlasnakov
Copy link

TodorKlasnakov commented Aug 21, 2020

The problem that we get after sorting/filtering comes from ScrollableView.prototype.loadPage. There we have the following check - !this.loadedPages.includes(page). So the way it works is that while you are scrolling you are adding loadPages number into the array but after sorting/filtering it does not get cleared. Therfore, while you are at first page after sorting the onLazyLoad will not trigger because it is already included in loadedPages. All you need to do to fix that is to call the clearCache() method on p-table. This will clear the loadedPages and will start emitting the onLazyLoad event again. I hope this helps.

@cagataycivici cagataycivici self-assigned this Sep 2, 2020
@cagataycivici cagataycivici added the Type: Bug Issue contains a bug related to a specific component. Something about the component is not working label Sep 2, 2020
@cagataycivici cagataycivici added this to the 10.0.0-rc.4 milestone Sep 2, 2020
@cagataycivici cagataycivici changed the title 9.1.0 p-table lazy loading virtual scroll not working Table lazy virtual scroll not called on last page Sep 2, 2020
@cagataycivici cagataycivici removed LTS-PORTABLE Type: Bug Issue contains a bug related to a specific component. Something about the component is not working labels Sep 2, 2020
@cagataycivici cagataycivici removed their assignment Sep 2, 2020
@cagataycivici cagataycivici removed this from the 10.0.0-rc.4 milestone Sep 2, 2020
@cagataycivici
Copy link
Member

cagataycivici commented Sep 2, 2020

I can't seem to replicate on 10.0.0-rc.4, fixed a couple of issues on virtual scroll but could not see this one. Please note that virtual scrolling since 9.1.0 is not backward compatible since we switches to CDK for it. If the issue still persists, please create a new ticker with a stackblitz case demonstrating the issue and our team will review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests