fix: show pagination warning on all pages (fixes #11968) #11973
Conversation
Signed-off-by: Sebast1aan <[email protected]>
There was a problem hiding this comment.
there's a flaw in this logic -- see below. @Sebast1aan can you raise a follow-up PR for an additional fix? EDIT: I fixed this myself in #11979
| @@ -19,7 +19,7 @@ export function PaginationPanel(props: {pagination: Pagination; onChange: (pagin | |||
| }> | |||
| Next page <i className='fa fa-chevron-right' /> | |||
| </button> | |||
| {props.pagination.limit > 0 && props.pagination.limit <= props.numRecords ? ( | |||
There was a problem hiding this comment.
hmm so there's a reason this logic was here -- if you're on the first page and you only have 10 total workflows while paginating 50 workflows, this warning shouldn't show. You're already listing all of them.
This new logic is actually causing a bug now, see this screenshot where I have only 10 total workflows and paginate on 50 -- it shows the warning, but it shouldn't:
Previously it didn't show this warning.
Here's the PaginationPanel's props from the React debugger:
this actually needs some more nuanced logic to show up on a page that isn't first but not on the first page when there's enough records:
| {props.pagination.limit > 0 && props.pagination.limit <= props.numRecords ? ( | |
| {/* if pagination is used, and we're either not on the first page, or are on the first page and have more records than the page limit */} | |
| {props.pagination.limit > 0 && (props.pagination.offset || (!props.pagination.offset && props.numRecords >= props.pagination.limit)) ? ( |
This works correctly on the previous example and should handle your scenario of the last page as well:
There was a problem hiding this comment.
Good catch, missed that specific case.
When limiting the number of visible records in the workflow page, the pagination warning is now shown on all pages (not depending on the number of records on that page).
Fixes #11968