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

Fix page out-of-range check #277

Merged
merged 1 commit into from
Oct 6, 2021

Conversation

codemonkey800
Copy link
Collaborator

@codemonkey800 codemonkey800 commented Oct 6, 2021

Description

This fixes an issue with the page out-of-range check not working when the results are reduced using the search query or plugin filters.

Initially the totalPages was being calculated using:

const totalPages = Math.ceil(
  searchFormStore.search.index.length / RESULTS_PER_PAGE,
);

However, when there's a search query or if a filter is enabled, the actual total may be less than the calculated amount. Because of this, the only way to get the correct totalPages value is to read the derived state.

Demo

Before

Visiting https://staging.napari-hub.org/?search=image&sort=relevance&page=5 will open a page with empty results. The page will also be out of range:

image

After

Visiting https://pagination-frontend.dev.imaging.cziscience.com/?search=image&sort=relevance&page=5 will now correctly change the page to 4.

@codemonkey800 codemonkey800 force-pushed the jeremy/fix-pagination-out-of-range-issues branch from 8875076 to 6febf90 Compare October 6, 2021 20:55
@codemonkey800 codemonkey800 marked this pull request as ready for review October 6, 2021 21:05
Copy link
Contributor

@justinelarsen justinelarsen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 🔨

* @param totalPages The max number of pages allowed.
*/
function setPageState(totalPages: number) {
if (page > totalPages) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is page zero-indexed or one-indexed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

love the question! pages are one-indexed 😆 if this was a programmer website, maybe it would have been zero-indexed lol

@codemonkey800 codemonkey800 merged commit 8c69e52 into main Oct 6, 2021
@codemonkey800 codemonkey800 deleted the jeremy/fix-pagination-out-of-range-issues branch October 6, 2021 22:21
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

Successfully merging this pull request may close these issues.

3 participants