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.

Sign up for GitHub

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

[Issue #3522] refine fix for pagination showing with no results #3533

Merged
merged 4 commits into from
Jan 21, 2025
Merged

[Issue #3522] refine fix for pagination showing with no results #3533

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
33bea3e
set pagination values correctly
doug-s-nava Jan 15, 2025
25fdf7a
tests and fixes
doug-s-nava Jan 15, 2025
621edc4
remove unused export
doug-s-nava Jan 17, 2025
b388fe5
Merge branch 'main' into dschrashun/3522-pagination-bug
doug-s-nava Jan 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 32 additions & 11 deletions frontend/src/components/search/SearchPagination.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import { QueryContext } from "src/app/[locale]/search/QueryProvider";
import { useSearchParamUpdater } from "src/hooks/useSearchParamUpdater";

import { useContext } from "react";
import { useContext, useEffect } from "react";
import { Pagination } from "@trussworks/react-uswds";

export enum PaginationPosition {
Expand All @@ -14,40 +14,61 @@ export enum PaginationPosition {
interface SearchPaginationProps {
page: number;
query: string | null | undefined;
total?: number | null;
totalPages?: number | null;
scroll?: boolean;
totalResults?: string;
loading?: boolean;
}

const MAX_SLOTS = 7;

// in addition to handling client side page navigation, this client component handles setting client state for:
// - total pages of search results
// - total number of search results
export default function SearchPagination({
page,
query,
total = null,
totalPages = null,
scroll = false,
totalResults = "",
loading = false,
}: SearchPaginationProps) {
const { updateQueryParams } = useSearchParamUpdater();
const { updateTotalPages, updateTotalResults } = useContext(QueryContext);
const { totalPages } = useContext(QueryContext);
// Shows total pages from the query context before it is re-fetched from the API.
const pages = total || Number(totalPages);
const {
updateTotalPages,
updateTotalResults,
totalPages: totalPagesFromQuery,
} = useContext(QueryContext);

// will re-run on each fetch, as we switch from the Suspended version of the component to the live one
useEffect(() => {
if (!loading) {
updateTotalPages(String(totalPages));
}
}, [updateTotalPages, totalPages, loading]);

useEffect(() => {
if (!loading) {
updateTotalResults(String(totalResults));
}
}, [updateTotalResults, totalResults, loading]);

const updatePage = (page: number) => {
updateTotalPages(String(total));
updateTotalResults(totalResults);
Copy link
Collaborator Author

@doug-s-nava doug-s-nava Jan 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

with these functions only being called in updatePage the client side pagination state was only being updated on pagination clicks, not in response to the search results updating from the completed API request. This is why the pagination was not responding to adding filters the way we expected, resulting in the pagination still sometimes showing up even if there were no search results. With this new setup, the state will be set purely based on the results of the search request whenever a new request is made

updateQueryParams(String(page), "page", query, scroll);
};

// Shows total pages from the query context before it is re-fetched from the API.
// This is only used for display due to race conditions, otherwise totalPages prop
// is the source of truth
const pageCount = totalPages || Number(totalPagesFromQuery);

return (
<div className={`grants-pagination ${loading ? "disabled" : ""}`}>
{totalResults !== "0" && pages > 0 && (
{totalResults !== "0" && pageCount > 0 && (
<Pagination
aria-disabled={loading}
pathname="/search"
totalPages={pages}
totalPages={pageCount}
currentPage={page}
maxSlots={MAX_SLOTS}
onClickNext={() => updatePage(page + 1)}
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/components/search/SearchPaginationFetch.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export default async function SearchPaginationFetch({
return (
<>
<SearchPagination
total={totalPages}
totalPages={totalPages}
page={page}
query={query}
scroll={scroll}
Expand Down
8 changes: 2 additions & 6 deletions frontend/src/components/search/SearchResultsHeader.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { QueryContext } from "src/app/[locale]/search/QueryProvider";
import { useTranslations } from "next-intl";
import { useContext } from "react";

import SearchSortyBy from "./SearchSortBy";
import SearchSortBy from "./SearchSortBy";

export default function SearchResultsHeader({
sortby,
Expand Down Expand Up @@ -36,11 +36,7 @@ export default function SearchResultsHeader({
{t("resultsHeader.message", { count: total })}
</h2>
<div className="tablet-lg:grid-col-auto">
<SearchSortyBy
totalResults={total}
sortby={sortby}
queryTerm={queryTerm}
/>
<SearchSortBy sortby={sortby} queryTerm={queryTerm} />
</div>
</div>
);
Expand Down
23 changes: 9 additions & 14 deletions frontend/src/components/search/SearchSortBy.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,26 +1,19 @@
"use client";

import { QueryContext } from "src/app/[locale]/search/QueryProvider";
import { useSearchParamUpdater } from "src/hooks/useSearchParamUpdater";
import { SortOption } from "src/types/search/searchRequestTypes";

import { useTranslations } from "next-intl";
import { useContext } from "react";
import { useCallback } from "react";
import { Select } from "@trussworks/react-uswds";

interface SearchSortByProps {
queryTerm: string | null | undefined;
sortby: string | null;
totalResults: string;
}

export default function SearchSortBy({
queryTerm,
sortby,
totalResults,
}: SearchSortByProps) {
export default function SearchSortBy({ queryTerm, sortby }: SearchSortByProps) {
const { updateQueryParams } = useSearchParamUpdater();
const { updateTotalResults } = useContext(QueryContext);
const t = useTranslations("Search");

const SORT_OPTIONS: SortOption[] = [
Expand Down Expand Up @@ -52,11 +45,13 @@ export default function SearchSortBy({
},
];

const handleChange = (event: React.ChangeEvent<HTMLSelectElement>) => {
const newValue = event.target.value;
updateTotalResults(totalResults);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

there's no need to set total results in multiple places in the app, since both of these components are responding to the same request

updateQueryParams(newValue, "sortby", queryTerm);
};
const handleChange = useCallback(
(event: React.ChangeEvent<HTMLSelectElement>) => {
const newValue = event.target.value;
updateQueryParams(newValue, "sortby", queryTerm);
},
[queryTerm, updateQueryParams],
);

return (
<div id="search-sort-by">
Expand Down
Loading