dequelabs / cauldron

https://cauldron.dequelabs.com/
Mozilla Public License 2.0
86 stars 20 forks source link

usePagination does not update when the total number of issues changes #925

Open thuey opened 1 year ago

thuey commented 1 year ago

Currently, the usePagination hook does not gracefully handle when the total number of issues changes, which makes it unusable in situations where the items are filtered.

If totalItems changes to a number that would make the currentPage, pageStart , or pageEnd invalid (such as by changing to a smaller number of total items resulting in currentPage being higher than the total number of items), then the pagination state should update to valid values.

If the currentPage is higher than the total number of items, it should update to the last valid page, or the first page if the total number of items is 0.

scurker commented 1 year ago

It sounds like there might be 2 separate requirements here. The 1st one sounds like a defect we need to address. The 2nd I would like to see maybe a little more speculative details to better understand the issue that we're trying to solve.

thuey commented 1 year ago

@scurker That's a good point. Would you like two separate issues?

For the 2nd requirement, the current usePagination hook just deals in numbers--it returns the current page number you're on, the number of items on a page, and the total number of pages.

This is fine for pure server-side pagination. However, it would be nice if it could handle the client-side pagination for you, such that it returns not only the current page number, but a list of the items on the current page. That would save the consumer from having to do the calculations themselves to grab the right range from their list of items and making sure they don't go out of index (for example, making sure they don't try to get more items than are in the list).

scurker commented 1 year ago

Only because one appears to be a feature request, while the other could be considered a bug. It would likely be easier to get the work in if we could parse it out.

thuey commented 1 year ago

@scurker I updated this to just report the "bug" and created https://github.com/dequelabs/cauldron/issues/946 for the feature request