Skip to content
This repository has been archived by the owner on Sep 18, 2024. It is now read-only.

[Issue #51]: debounce pagination #53

Merged
merged 10 commits into from
Jun 3, 2024
Merged

[Issue #51]: debounce pagination #53

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
5955129
add debounce on pagination
rylew1 May 25, 2024
908bf26
update debounce to 300ms
rylew1 May 29, 2024
1b4de35
Merge branch 'main' into rylew/51-debounce-pagination
rylew1 May 29, 2024
0b8c235
Merge branch 'main' into rylew/51-debounce-pagination
rylew1 May 31, 2024
8b36108
fix typing on search tests
rylew1 May 31, 2024
b8deb59
add delay for pagination debounce
rylew1 May 31, 2024
3a4b71f
add pagination debounce wait for clicking last page
rylew1 May 31, 2024
b6e1e6d
fix page reset test by selecting first
rylew1 Jun 3, 2024
b524b96
Merge branch 'main' into rylew/51-debounce-pagination
rylew1 Jun 3, 2024
19cd99d
format
rylew1 Jun 3, 2024
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
14 changes: 11 additions & 3 deletions frontend/src/components/search/SearchPagination.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
"use client";

import { Pagination } from "@trussworks/react-uswds";
import { useDebouncedCallback } from "use-debounce";
import { useFormStatus } from "react-dom";

export enum PaginationPosition {
Expand All @@ -19,6 +20,7 @@ interface SearchPaginationProps {
}

const MAX_SLOTS = 7;
const DEBOUNCE_TIME = 300;

export default function SearchPagination({
showHiddenInput,
Expand All @@ -31,6 +33,10 @@ export default function SearchPagination({
}: SearchPaginationProps) {
const { pending } = useFormStatus();

const debouncedHandlePageChange = useDebouncedCallback((newPage: number) => {
handlePageChange(newPage);
}, DEBOUNCE_TIME);
Copy link
Member

Choose a reason for hiding this comment

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

(just curious) Does this mean there will always be a 500 delay? Even on the first click?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Basically yes - I wanted to have a quick meeting also - that's why this is still in draft.

I also would need to update some of the e2e tests to wait for page change

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good. Let's meet to talk it thru. There might be a different UX we could use — e.g. instead of debouncing, disable the pagination while results are loading.


// If there's no results, don't show pagination
if (searchResultsLength < 1) {
return null;
Expand Down Expand Up @@ -59,9 +65,11 @@ export default function SearchPagination({
totalPages={totalPages}
currentPage={page}
maxSlots={MAX_SLOTS}
onClickNext={() => handlePageChange(page + 1)}
onClickPrevious={() => handlePageChange(page - 1)}
onClickPageNumber={(event, page) => handlePageChange(page)}
onClickNext={() => debouncedHandlePageChange(page + 1)}
onClickPrevious={() => debouncedHandlePageChange(page - 1)}
onClickPageNumber={(event: React.MouseEvent, page: number) =>
debouncedHandlePageChange(page)
}
/>
</>
);
Expand Down
36 changes: 27 additions & 9 deletions frontend/tests/e2e/search/search.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { Page, expect, test } from "@playwright/test";
import {
clickAccordionWithTitle,
clickLastPaginationPage,
Expand All @@ -20,9 +21,18 @@ import {
toggleCheckboxes,
waitForSearchResultsInitialLoad,
} from "./searchSpecUtil";
import { expect, test } from "@playwright/test";

test("should navigate from index to search page", async ({ page }) => {
import { BrowserContextOptions } from "playwright-core";

interface PageProps {
page: Page;
browserName?: string;
contextOptions?: BrowserContextOptions;
}

test("should navigate from index to search page", async ({
page,
}: PageProps) => {
// Start from the index page with feature flag set
await page.goto("/?_ff=showSearchV0:true");

Expand All @@ -48,15 +58,15 @@ test("should navigate from index to search page", async ({ page }) => {
});

test.describe("Search page tests", () => {
test.beforeEach(async ({ page }) => {
test.beforeEach(async ({ page }: PageProps) => {
// Navigate to the search page with the feature flag set
await page.goto("/search?_ff=showSearchV0:true");
});

test("should return 0 results when searching for obscure term", async ({
page,
browserName,
}) => {
}: PageProps) => {
// TODO (Issue #2005): fix test for webkit
test.skip(
browserName === "webkit",
Expand All @@ -80,7 +90,10 @@ test.describe("Search page tests", () => {
);
});

test("should show and hide loading state", async ({ page, browserName }) => {
test("should show and hide loading state", async ({
page,
browserName,
}: PageProps) => {
// TODO (Issue #2005): fix test for webkit
test.skip(
browserName === "webkit",
Expand All @@ -98,7 +111,10 @@ test.describe("Search page tests", () => {
await expect(loadingIndicator).toBeVisible();
await expect(loadingIndicator).toBeHidden();
});
test("should refresh and retain filters in a new tab", async ({ page }) => {

test("should refresh and retain filters in a new tab", async ({
page,
}: PageProps) => {
// Set all inputs, then refresh the page. Those same inputs should be
// set from query params.
const searchTerm = "education";
Expand Down Expand Up @@ -175,7 +191,9 @@ test.describe("Search page tests", () => {
}
});

test("resets page back to 1 when choosing a filter", async ({ page }) => {
test("resets page back to 1 when choosing a filter", async ({
page,
}: PageProps) => {
await clickPaginationPageNumber(page, 2);

// Verify that page 1 is highlighted
Expand All @@ -201,7 +219,7 @@ test.describe("Search page tests", () => {

test("last result becomes first result when flipping sort order", async ({
page,
}) => {
}: PageProps) => {
await selectSortBy(page, "opportunityTitleDesc");

await clickLastPaginationPage(page);
Expand All @@ -219,7 +237,7 @@ test.describe("Search page tests", () => {

test("number of results is the same with none or all opportunity status checked", async ({
page,
}) => {
}: PageProps) => {
const initialNumberOfOpportunityResults =
await getNumberOfOpportunitySearchResults(page);

Expand Down
5 changes: 5 additions & 0 deletions frontend/tests/e2e/search/searchSpecUtil.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,9 @@ export async function clickPaginationPageNumber(
`button[data-testid="pagination-page-number"][aria-label="Page ${pageNumber}"]`,
);
await paginationButton.first().click();

// Delay for pagination debounce
await page.waitForTimeout(400);
}

export async function clickLastPaginationPage(page: Page) {
Expand All @@ -156,6 +159,8 @@ export async function clickLastPaginationPage(page: Page) {
if (count > 2) {
await paginationButtons.nth(count - 2).click();
}
// Delay for pagination debounce
await page.waitForTimeout(400);
}

export async function getFirstSearchResultTitle(page: Page) {
Expand Down
Loading