Skip to content

Commit

Permalink
Merge pull request #229 from NYPL/no_ref/more-loading-fixes
Browse files Browse the repository at this point in the history
No ref: Pagination fixes pre-launch
  • Loading branch information
7emansell authored Nov 14, 2024
2 parents 13e46de + f5eb807 commit 3315067
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 14 deletions.
2 changes: 1 addition & 1 deletion app/src/components/lane/laneLoading.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export default function LaneLoading({ withTitle = true }) {
<SkeletonLoader contentSize={0} showImage={false} headingSize={1} />
</Flex>
)}
<DCSimpleGrid>
<DCSimpleGrid marginTop="s">
<SkeletonLoader imageAspectRatio="landscape" contentSize={1} />
<SkeletonLoader imageAspectRatio="landscape" contentSize={1} />
<SkeletonLoader imageAspectRatio="landscape" contentSize={1} />
Expand Down
30 changes: 17 additions & 13 deletions app/src/components/pages/divisionPage/divisionPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ import { CardsGrid } from "../../grids/cardsGrid";
import {
totalNumPages,
createAdobeAnalyticsPageName,
displayResults,
} from "../../../utils/utils";
import useBreakpoints from "../../../hooks/useBreakpoints";
import { DC_URL } from "@/src/config/constants";
import { Lane as DCLane } from "../../lane/lane";
import LaneLoading from "../../lane/laneLoading";
Expand All @@ -38,9 +38,7 @@ export default function DivisionPage({ data }: any) {
Number(queryParams.get("page")) || 1
);

const { replace } = useRouter();

const { isLargerThanLargeTablet } = useBreakpoints();
const { push } = useRouter();

const totalPages = totalNumPages(data.numFound, data.perPage);

Expand All @@ -49,8 +47,12 @@ export default function DivisionPage({ data }: any) {
params.set("page", pageNumber.toString());
setCurrentPage(pageNumber);
const url = `${pathname}?${params.toString()}#${data.slug}`;
replace(url);
headingRef.current?.focus;
setIsLoaded(false);
push(url);
setTimeout(() => {
setIsLoaded(true);
headingRef.current?.focus();
}, 2000);
};

useEffect(() => {
Expand Down Expand Up @@ -110,24 +112,26 @@ export default function DivisionPage({ data }: any) {
<HorizontalRule sx={{ marginTop: "xxl", marginBottom: "xxl" }} />

<Heading
size="heading5"
sx={{ marginBottom: "m" }}
ref={headingRef}
tabIndex={-1}
level="h2"
id={slug}
size="heading3"
style={{ width: "fit-content" }}
width="max-content"
>
{displayResults(data.numFound, data.perPage, data.page)}
</Heading>

<Heading level="h2" size="heading3" style={{ width: "fit-content" }}>
{`Collections in the ${data.name}`}
</Heading>

{isLoaded ? (
<CardsGrid records={data.collections} />
) : (
<>
<LaneLoading withTitle={false} />
<LaneLoading withTitle={false} />
Array(Math.ceil(data.collections.length / 4)).fill(
<LaneLoading withTitle={false} />
</>
)
)}
{totalPages > 1 && (
<Pagination
Expand Down
27 changes: 27 additions & 0 deletions app/src/utils/utils.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import {
slugToString,
stringToSlug,
createAdobeAnalyticsPageName,
displayResults,
} from "./utils";

// TODO:
Expand Down Expand Up @@ -176,3 +177,29 @@ describe("createAdobeAnalyticsPageName generates the correct Adobe Analytics pag
);
});
});

describe("displayResults", () => {
test("displays correct result range for first page with enough results", () => {
expect(displayResults(100, 10, 1)).toBe("Results: 1-10 of 100");
});

test("displays correct result range for middle page", () => {
expect(displayResults(100, 10, 5)).toBe("Results: 41-50 of 100");
});

test("displays correct result range for last page when there are fewer results than perPage", () => {
expect(displayResults(45, 10, 5)).toBe("Results: 41-45 of 45");
});

test("displays correct result range when numFound is less than perPage", () => {
expect(displayResults(8, 10, 1)).toBe("Results: 1-8 of 8");
});

test("displays correct result range for the last page when it's the same as perPage", () => {
expect(displayResults(50, 10, 5)).toBe("Results: 41-50 of 50");
});

test("displays correct range when perPage is greater than numFound", () => {
expect(displayResults(5, 10, 1)).toBe("Results: 1-5 of 5");
});
});
10 changes: 10 additions & 0 deletions app/src/utils/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,3 +112,13 @@ export const createAdobeAnalyticsPageName = (
): string => {
return recordName ? `${base}|${stringToSlug(recordName)}` : base;
};

export function displayResults(
numFound: number,
perPage: number,
page: number
) {
const start = (page - 1) * perPage + 1;
const end = Math.min(page * perPage, numFound);
return `Results: ${start}-${end} of ${numFound}`;
}

0 comments on commit 3315067

Please sign in to comment.