Skip to content

Commit

Permalink
fix: Improve Pagination type for onSelectPage
Browse files Browse the repository at this point in the history
The previous iteration of these type was accounting for the generated
page numbers being either a string (`SEPARATOR`) or numbers.

In this commit I introduce a much narrower type for the
`getPageItemsToDisplay` function. In doing so we more closely map to the
reality of the implementation and can use a more appropriately narrow
type for `onSelectPage`.

This resolves some challenges with ECO.
  • Loading branch information
jherdman committed Jun 3, 2024
1 parent 33a0105 commit 9f3861e
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 6 deletions.
2 changes: 1 addition & 1 deletion src/Pagination/Pagination.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ describe("Pagination", () => {
onSelectPage={onSelectPageCallback}
/>
);
const clickPage = (pageNum) => {
const clickPage = (pageNum: number) => {
fireEvent.click(getAllByLabelText("Go to page {{count}}")[pageNum]);
};
clickPage(2);
Expand Down
10 changes: 5 additions & 5 deletions src/Pagination/Pagination.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ import PageNumber from "./PageNumber";
import PreviousButton from "./PreviousButton";
import NextButton from "./NextButton";

const SEPARATOR = "...";
const SEPARATOR = "..." as const;

export const getPageItemsToDisplay = (totalPages: number, currentPage: number) => {
export const getPageItemsToDisplay = (totalPages: number, currentPage: number): Array<typeof SEPARATOR | number> => {
const MAX_PAGES_TO_SHOW = 6;

const pages = Array.from({ length: totalPages }, (v, i) => i + 1);
Expand All @@ -26,19 +26,19 @@ export const getPageItemsToDisplay = (totalPages: number, currentPage: number) =
return [1, SEPARATOR, ...pages.slice(currentPage - 2, currentPage + 2), SEPARATOR, totalPages];
};

type PaginationProps = FlexProps & {
interface PaginationProps extends FlexProps {
currentPage: number;
totalPages: number;
onNext?: () => void;
onPrevious?: () => void;
onSelectPage?: (page: string | number) => void;
onSelectPage?: (page: number) => void;
nextLabel?: ReactNode;
nextAriaLabel?: string;
previousLabel?: ReactNode;
previousAriaLabel?: string;
scrollToTopAfterPagination?: boolean;
scrollTargetRef?: RefObject<HTMLElement>;
};
}

function Pagination({
currentPage,
Expand Down

0 comments on commit 9f3861e

Please sign in to comment.