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

#3858 #3896 - fix row grouping with pagination #4011

Merged

Conversation

kyle-zadara
Copy link
Contributor

@kyle-zadara kyle-zadara commented Jun 1, 2023

As detailed in #3858 and #3896, row grouping is not working with pagination. This is due to the getRowIndex function adding this.first to the row index. getRowIndex is called by as part of the shouldRenderRowGroupHeader call.

The problem is that when pagination is enabled, the DataTable competent slices the data passed to TableBody in the dataToRender function, so the size of the value array being passed to TableBody is the page size. dataToRender is being called when the value prop is being passed to TableBody.

This means if the are 100 rows total, with a rows per page of 20, only 20 rows get passed to TableBody at any given time. If you were on page 2 for example the getRowIndex function is adding this.first to the index, the index it is trying to compare would be 20 + index, which is out of range for the value array of 20 (max) items.

The proposed fix in the PR is to simply stop adding this.value to the indexes. This has fixed the issue in my local testing.

@vercel
Copy link

vercel bot commented Jun 1, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
primevue ⬜️ Ignored (Inspect) Jun 1, 2023 0:48am

@jkocyigi
Copy link

@tugcekucukoglu sorry in advance, but I am unfamiliar with the PrimeVue process. I was wondering how fast the pull requests are being completed?
I, and probably more people would really like this particular PR to be merged

@fireblazer9
Copy link

Also hitting this problem - preventing us from using row grouping because of poor user experience when going to page 2 and beyond :(

@mertsincan mertsincan merged commit 7529764 into primefaces:master Aug 18, 2023
1 check passed
@mertsincan
Copy link
Member

Thanks a lot for your contribution!

Best Regards,

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