diff --git a/site/src/components/PaginationWidget/PageButton.tsx b/site/src/components/PaginationWidget/PageButton.tsx deleted file mode 100644 index 9b89486359cfb..0000000000000 --- a/site/src/components/PaginationWidget/PageButton.tsx +++ /dev/null @@ -1,45 +0,0 @@ -import Button from "@mui/material/Button"; -import { css, useTheme } from "@emotion/react"; - -interface PageButtonProps { - activePage?: number; - page?: number; - placeholder?: string; - numPages?: number; - onPageClick?: (page: number) => void; - disabled?: boolean; -} - -export const PageButton = ({ - activePage, - page, - placeholder = "...", - numPages, - onPageClick, - disabled = false, -}: PageButtonProps): JSX.Element => { - const theme = useTheme(); - return ( - - ); -}; diff --git a/site/src/components/PaginationWidget/PageButtons.tsx b/site/src/components/PaginationWidget/PageButtons.tsx new file mode 100644 index 0000000000000..ceef484aa13b3 --- /dev/null +++ b/site/src/components/PaginationWidget/PageButtons.tsx @@ -0,0 +1,108 @@ +import { PropsWithChildren } from "react"; +import Button from "@mui/material/Button"; +import { useTheme } from "@emotion/react"; + +type NumberedPageButtonProps = { + pageNumber: number; + totalPages: number; + + onClick?: () => void; + highlighted?: boolean; + disabled?: boolean; +}; + +export function NumberedPageButton({ + pageNumber, + totalPages, + onClick, + highlighted = false, + disabled = false, +}: NumberedPageButtonProps) { + return ( + + {pageNumber} + + ); +} + +type PlaceholderPageButtonProps = PropsWithChildren<{ + pagesOmitted: number; +}>; + +export function PlaceholderPageButton({ + pagesOmitted, + children = <>…, +}: PlaceholderPageButtonProps) { + return ( + + {children} + + ); +} + +type BasePageButtonProps = PropsWithChildren<{ + name: string; + "aria-label": string; + + onClick?: () => void; + highlighted?: boolean; + disabled?: boolean; +}>; + +function BasePageButton({ + children, + onClick, + name, + "aria-label": ariaLabel, + highlighted = false, + disabled = false, +}: BasePageButtonProps) { + const theme = useTheme(); + + return ( + + ); +} + +function getNumberedButtonLabel( + page: number, + totalPages: number, + highlighted: boolean, +): string { + if (highlighted) { + return "Current Page"; + } + + if (page === 1) { + return "First Page"; + } + + if (page === totalPages) { + return "Last Page"; + } + + return `Page ${page}`; +} diff --git a/site/src/components/PaginationWidget/PaginationNavButton.tsx b/site/src/components/PaginationWidget/PaginationNavButton.tsx new file mode 100644 index 0000000000000..1bafe67f1fea8 --- /dev/null +++ b/site/src/components/PaginationWidget/PaginationNavButton.tsx @@ -0,0 +1,105 @@ +import { + type ButtonHTMLAttributes, + type ReactNode, + useEffect, + useState, +} from "react"; +import { useTheme } from "@emotion/react"; + +import Button from "@mui/material/Button"; +import Tooltip from "@mui/material/Tooltip"; + +type PaginationNavButtonProps = Omit< + ButtonHTMLAttributes, + | "aria-disabled" + // Need to omit color for MUI compatibility + | "color" +> & { + // Required/narrowed versions of default props + children: ReactNode; + disabled: boolean; + onClick: () => void; + "aria-label": string; + + // Bespoke props + disabledMessage: ReactNode; + disabledMessageTimeout?: number; +}; + +function PaginationNavButtonCore({ + onClick, + disabled, + disabledMessage, + disabledMessageTimeout = 3000, + ...delegatedProps +}: PaginationNavButtonProps) { + const theme = useTheme(); + const [showDisabledMessage, setShowDisabledMessage] = useState(false); + + // Inline state sync - this is safe/recommended by the React team in this case + if (!disabled && showDisabledMessage) { + setShowDisabledMessage(false); + } + + useEffect(() => { + if (!showDisabledMessage) { + return; + } + + const timeoutId = setTimeout( + () => setShowDisabledMessage(false), + disabledMessageTimeout, + ); + + return () => clearTimeout(timeoutId); + }, [showDisabledMessage, disabledMessageTimeout]); + + return ( + + {/* + * Going more out of the way to avoid attaching the disabled prop directly + * to avoid unwanted side effects of using the prop: + * - Not being focusable/keyboard-navigable + * - Not being able to call functions in response to invalid actions + * (mostly for giving direct UI feedback to those actions) + */} + + + {isMobile ? ( - + ) : ( - buildPagedList(numPages, page).map((pageItem) => { - if (pageItem === "left" || pageItem === "right") { - return ( - - ); - } - - return ( - onChange(pageItem)} - /> - ); - }) + )} - + ); }; + +type PaginationRowProps = { + currentPage: number; + totalPages: number; + onChange: (newPage: number) => void; +}; + +function PaginationRow({ + currentPage, + totalPages, + onChange, +}: PaginationRowProps) { + const pageInfo = buildPagedList(totalPages, currentPage); + const pagesOmitted = totalPages - pageInfo.length - 1; + + return ( + <> + {pageInfo.map((pageEntry) => { + if (pageEntry === "left" || pageEntry === "right") { + return ( + + ); + } + + return ( + onChange(pageEntry)} + /> + ); + })} + + ); +} diff --git a/site/src/components/PaginationWidget/utils.test.ts b/site/src/components/PaginationWidget/utils.test.ts index 01f79eb0a9f30..12c53af85dc75 100644 --- a/site/src/components/PaginationWidget/utils.test.ts +++ b/site/src/components/PaginationWidget/utils.test.ts @@ -1,40 +1,106 @@ -import { buildPagedList, getOffset } from "./utils"; - -describe("buildPagedList", () => { - it.each<{ - numPages: number; - activePage: number; - expected: (string | number)[]; - }>([ - { numPages: 7, activePage: 1, expected: [1, 2, 3, 4, 5, 6, 7] }, - { numPages: 17, activePage: 1, expected: [1, 2, 3, 4, 5, "right", 17] }, - { - numPages: 17, - activePage: 9, - expected: [1, "left", 8, 9, 10, "right", 17], - }, - { - numPages: 17, - activePage: 17, - expected: [1, "left", 13, 14, 15, 16, 17], - }, - ])( - `buildPagedList($numPages, $activePage)`, - ({ numPages, activePage, expected }) => { - expect(buildPagedList(numPages, activePage)).toEqual(expected); - }, - ); +import { buildPagedList, getOffset, isNonInitialPage } from "./utils"; + +describe(buildPagedList.name, () => { + it("has no placeholder entries when there are seven or fewer pages", () => { + for (let i = 1; i <= 7; i++) { + const expectedResult: number[] = []; + for (let j = 1; j <= i; j++) { + expectedResult.push(j); + } + + expect(buildPagedList(i, i)).toEqual(expectedResult); + } + }); + + it("has 'right' placeholder for long lists when active page is near beginning", () => { + expect(buildPagedList(17, 1)).toEqual([1, 2, 3, 4, 5, "right", 17]); + }); + + it("has 'left' placeholder for long lists when active page is near end", () => { + expect(buildPagedList(17, 17)).toEqual([1, "left", 13, 14, 15, 16, 17]); + }); + + it("has both placeholders for long lists when active page is in the middle", () => { + expect(buildPagedList(17, 9)).toEqual([1, "left", 8, 9, 10, "right", 17]); + }); + + it("produces an empty array when there are no pages", () => { + expect(buildPagedList(0, 0)).toEqual([]); + }); + + it("makes sure all values are unique (for React rendering keys)", () => { + type TestEntry = [numPages: number, activePage: number]; + const testData: TestEntry[] = [ + [0, 0], + [1, 1], + [2, 2], + [3, 3], + [4, 4], + [5, 5], + [6, 6], + [7, 7], + + [10, 3], + [7, 1], + [17, 1], + [17, 9], + ]; + + for (const [numPages, activePage] of testData) { + const result = buildPagedList(numPages, activePage); + const uniqueCount = new Set(result).size; + + expect(uniqueCount).toEqual(result.length); + } + }); }); -describe("getOffset", () => { +describe(getOffset.name, () => { it("returns 0 on page 1", () => { const page = 1; const limit = 10; expect(getOffset(page, limit)).toEqual(0); }); - it("returns the limit on page 2", () => { - const page = 2; - const limit = 10; - expect(getOffset(page, limit)).toEqual(limit); + + it("Returns the results for page 1 when input is invalid", () => { + const inputs = [0, -1, -Infinity, NaN, Infinity, 3.6, 7.4545435]; + + for (const input of inputs) { + expect(getOffset(input, 10)).toEqual(0); + } + }); + + it("Returns offset based on the current page for all valid pages after 1", () => { + expect(getOffset(2, 10)).toEqual(10); + expect(getOffset(3, 10)).toEqual(20); + expect(getOffset(4, 45)).toEqual(135); + }); +}); + +describe(isNonInitialPage.name, () => { + it("Should detect your page correctly if input is set and valid", () => { + const params1 = new URLSearchParams({ page: String(1) }); + expect(isNonInitialPage(params1)).toBe(false); + + const inputs = [2, 50, 500, 3722]; + + for (const input of inputs) { + const params = new URLSearchParams({ page: String(input) }); + expect(isNonInitialPage(params)).toBe(true); + } + }); + + it("Should act as if you are on page 1 if input is set but invalid", () => { + const inputs = ["", Infinity, -Infinity, NaN, 3.74, -3]; + + for (const input of inputs) { + const params = new URLSearchParams({ page: String(input) }); + expect(isNonInitialPage(params)).toBe(false); + } + }); + + it("Should act as if you are on page 1 if input does not exist", () => { + const params = new URLSearchParams(); + expect(isNonInitialPage(params)).toBe(false); }); }); diff --git a/site/src/components/PaginationWidget/utils.ts b/site/src/components/PaginationWidget/utils.ts index 9575396e29595..cb1b7729af856 100644 --- a/site/src/components/PaginationWidget/utils.ts +++ b/site/src/components/PaginationWidget/utils.ts @@ -1,67 +1,74 @@ /** * Generates a ranged array with an option to step over values. * Shamelessly stolen from: - * https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/from#sequence_generator_range + * {@link https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/from#sequence_generator_range} */ const range = (start: number, stop: number, step = 1) => Array.from({ length: (stop - start) / step + 1 }, (_, i) => start + i * step); export const DEFAULT_RECORDS_PER_PAGE = 25; -// Number of pages to the left or right of the current page selection. + +// Number of pages to display on either side of the current page selection const PAGE_NEIGHBORS = 1; -// Number of pages displayed for cases where there are multiple ellipsis showing. This can be -// thought of as the minimum number of page numbers to display when multiple ellipsis are showing. + +// Minimum number of pages to display at all times (assuming there are enough +// pages). Needed to handle case where the left and right placeholder/ellipsis +// elements are both showing const PAGES_TO_DISPLAY = PAGE_NEIGHBORS * 2 + 3; -// Total page blocks(page numbers or ellipsis) displayed, including the maximum number of ellipsis (2). -// This gives us maximum number of 7 page blocks to be displayed when the page neighbors value is 1. -const NUM_PAGE_BLOCKS = PAGES_TO_DISPLAY + 2; + +// Total number of pagination elements to display (accounting for visible pages +// and up to two ellipses placeholders). With 1 page neighbor on either side, +// the UI will show up to seven elements total +const TOTAL_PAGE_BLOCKS = PAGES_TO_DISPLAY + 2; /** - * Builds a list of pages based on how many pages exist and where the user is in their navigation of those pages. - * List result is used to from the buttons that make up the Pagination Widget + * Takes the total number of pages from a pagination result, and truncates it + * into a UI-friendly list. */ export const buildPagedList = ( numPages: number, activePage: number, ): ("left" | "right" | number)[] => { - if (numPages > NUM_PAGE_BLOCKS) { - let pages = []; - const leftBound = activePage - PAGE_NEIGHBORS; - const rightBound = activePage + PAGE_NEIGHBORS; - const beforeLastPage = numPages - 1; - const startPage = leftBound > 2 ? leftBound : 2; - const endPage = rightBound < beforeLastPage ? rightBound : beforeLastPage; + if (numPages <= TOTAL_PAGE_BLOCKS) { + return range(1, numPages); + } - pages = range(startPage, endPage); + const pageBeforeLast = numPages - 1; + const startPage = Math.max(activePage - PAGE_NEIGHBORS, 2); + const endPage = Math.min(activePage + PAGE_NEIGHBORS, pageBeforeLast); - const singleSpillOffset = PAGES_TO_DISPLAY - pages.length - 1; - const hasLeftOverflow = startPage > 2; - const hasRightOverflow = endPage < beforeLastPage; - const leftOverflowPage = "left" as const; - const rightOverflowPage = "right" as const; + let pages: ReturnType = range(startPage, endPage); - if (hasLeftOverflow && !hasRightOverflow) { - const extraPages = range(startPage - singleSpillOffset, startPage - 1); - pages = [leftOverflowPage, ...extraPages, ...pages]; - } else if (!hasLeftOverflow && hasRightOverflow) { - const extraPages = range(endPage + 1, endPage + singleSpillOffset); - pages = [...pages, ...extraPages, rightOverflowPage]; - } else if (hasLeftOverflow && hasRightOverflow) { - pages = [leftOverflowPage, ...pages, rightOverflowPage]; - } + const singleSpillOffset = PAGES_TO_DISPLAY - pages.length - 1; + const hasLeftOverflow = startPage > 2; + const hasRightOverflow = endPage < pageBeforeLast; - return [1, ...pages, numPages]; + if (hasLeftOverflow && !hasRightOverflow) { + const extraPages = range(startPage - singleSpillOffset, startPage - 1); + pages = ["left", ...extraPages, ...pages]; + } else if (!hasLeftOverflow && hasRightOverflow) { + const extraPages = range(endPage + 1, endPage + singleSpillOffset); + pages = [...pages, ...extraPages, "right"]; + } else if (hasLeftOverflow && hasRightOverflow) { + pages = ["left", ...pages, "right"]; } - return range(1, numPages); + return [1, ...pages, numPages]; }; -// pages count from 1 -export const getOffset = (page: number, limit: number): number => - (page - 1) * limit; +/** + * Calculates the current offset from the start of a paginated dataset + */ +export const getOffset = (page: number, pageSize: number): number => { + const pageIsValid = Number.isInteger(page) && page >= 1; + const pageToUse = pageIsValid ? page : 1; + + return (pageToUse - 1) * pageSize; +}; -export const nonInitialPage = (searchParams: URLSearchParams): boolean => { +export const isNonInitialPage = (searchParams: URLSearchParams): boolean => { const page = searchParams.get("page"); - const numberPage = page ? Number(page) : 1; - return numberPage > 1; + const conversion = Number(page); + + return Number.isInteger(conversion) && conversion > 1; }; diff --git a/site/src/pages/AuditPage/AuditPage.tsx b/site/src/pages/AuditPage/AuditPage.tsx index 2ed69c608fdfe..174bf517a480c 100644 --- a/site/src/pages/AuditPage/AuditPage.tsx +++ b/site/src/pages/AuditPage/AuditPage.tsx @@ -1,6 +1,6 @@ import { DEFAULT_RECORDS_PER_PAGE, - nonInitialPage, + isNonInitialPage, } from "components/PaginationWidget/utils"; import { useFeatureVisibility } from "hooks/useFeatureVisibility"; import { FC } from "react"; @@ -73,7 +73,7 @@ const AuditPage: FC = () => { page={pagination.page} limit={pagination.limit} onPageChange={pagination.goToPage} - isNonInitialPage={nonInitialPage(searchParamsResult[0])} + isNonInitialPage={isNonInitialPage(searchParamsResult[0])} isAuditLogVisible={isAuditLogVisible} error={error} filterProps={{ diff --git a/site/src/pages/AuditPage/AuditPageView.tsx b/site/src/pages/AuditPage/AuditPageView.tsx index 26e1b9f975dfc..04cc0a32d483b 100644 --- a/site/src/pages/AuditPage/AuditPageView.tsx +++ b/site/src/pages/AuditPage/AuditPageView.tsx @@ -133,10 +133,10 @@ export const AuditPageView: FC = ({ {count !== undefined && ( )} diff --git a/site/src/pages/UsersPage/UsersPage.tsx b/site/src/pages/UsersPage/UsersPage.tsx index 7d09df412b32e..dd60039056f02 100644 --- a/site/src/pages/UsersPage/UsersPage.tsx +++ b/site/src/pages/UsersPage/UsersPage.tsx @@ -28,7 +28,7 @@ import { prepareQuery } from "utils/filters"; import { Helmet } from "react-helmet-async"; import { DeleteDialog } from "components/Dialogs/DeleteDialog/DeleteDialog"; -import { nonInitialPage } from "components/PaginationWidget/utils"; +import { isNonInitialPage } from "components/PaginationWidget/utils"; import { ConfirmDialog } from "components/Dialogs/ConfirmDialog/ConfirmDialog"; import { ResetPasswordDialog } from "./ResetPasswordDialog"; import { pageTitle } from "utils/page"; @@ -157,7 +157,7 @@ export const UsersPage: FC<{ children?: ReactNode }> = () => { isLoading={isLoading} canEditUsers={canEditUsers} canViewActivity={entitlements.features.audit_log.enabled} - isNonInitialPage={nonInitialPage(searchParams)} + isNonInitialPage={isNonInitialPage(searchParams)} actorID={me.id} filterProps={{ filter: useFilterResult, diff --git a/site/src/pages/UsersPage/UsersPageView.tsx b/site/src/pages/UsersPage/UsersPageView.tsx index 5ea743d210781..8366d9ec7bf7b 100644 --- a/site/src/pages/UsersPage/UsersPageView.tsx +++ b/site/src/pages/UsersPage/UsersPageView.tsx @@ -102,10 +102,10 @@ export const UsersPageView: FC> = ({ {count && ( )} diff --git a/site/src/pages/WorkspacesPage/WorkspacesPageView.tsx b/site/src/pages/WorkspacesPage/WorkspacesPageView.tsx index 0a1a416363060..1fb88a71693f6 100644 --- a/site/src/pages/WorkspacesPage/WorkspacesPageView.tsx +++ b/site/src/pages/WorkspacesPage/WorkspacesPageView.tsx @@ -161,10 +161,10 @@ export const WorkspacesPageView = ({ {count !== undefined && ( )}