Skip to content

Pagination with table layout and hardcoded "showing X to X of X entries" #1181

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

Open
2 tasks done
RedGlow opened this issue Dec 12, 2023 · 2 comments
Open
2 tasks done
Labels
🚀 enhancement New feature or request
Milestone

Comments

@RedGlow
Copy link

RedGlow commented Dec 12, 2023

  • I have searched the Issues to see if this bug has already been reported
  • I have tested the latest version

Steps to reproduce

  1. Create a Pagination element with layout='table', a fixed totalPages (like 23) and a variable currentPage
  2. Notice that there is this behaviour while navigating:
    • currentPage = 1 => "Showing 1 to 5 of 23 Entries"
    • currentPage = 2 => "Showing 1 to 5 of 23 Entries"
    • currentPage = 3 => "Showing 1 to 5 of 23 Entries"
    • currentPage = 4 => "Showing 2 to 6 of 23 Entries"
    • currentPage = 5 => "Showing 3 to 7 of 23 Entries"
    • (this behaviour goes on)

Current behavior

The "Showing" label doesn't seem to actually display information related to the page.

Expected behavior

Showing the actual range of items displayed, like "Showing 1 to 10 of 230 Entries", "Showing 11 to 20 of 230 Entries" and so on. This, though, would require the pagination element to take parameters regarding this range, since it depends on the actual results displayed in a table and could be variable (e.g.: last page could contain less elements than the full page).

Other notes

I have noticed in the code that this info is extracted from:

  const lastPage = Math.min(Math.max(layout === 'pagination' ? currentPage + 2 : currentPage + 4, 5), totalPages);
  const firstPage = Math.max(1, lastPage - 4);

Which seems to be used both for the default "pagination" layout, where it probably makes sense (I haven't checked), but also for the table layout, where the label doesn't (or shouldn't? or doesn't appear to?) refer to pages but rather to single items.

@tulup-conner tulup-conner added the 🚀 enhancement New feature or request label Dec 12, 2023
@tulup-conner tulup-conner added this to the 1.0.0 milestone Dec 12, 2023
@tulup-conner
Copy link
Collaborator

Yeah, I've noticed this as well. This is a hybrid of bugfix and new feature. We have weird behavior in some environments where the first couple of clicks don't change the active page, and also, there should definitely be a property to set the number of items per page.

@RedGlow
Copy link
Author

RedGlow commented Dec 13, 2023

I think it either needs the number of items per page + total number of items, or starting item index + last item index, otherwise there isn't enough information to correctly compute the indices of the last page.
E.g., if we have 35 elements and 10 elements per page, we will have the ranges 1-10, 11-20, and then the last page is 21-35, which is not the same as the component could compute on its own just with the page size (which would result in 21-40).

If I find some time I can try to make a pull request for this issue (more likely during the upcoming winter holidays rather than right now).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🚀 enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants