From b0d5e0613e56fafe4af2f1a0fb7235ec7a3dea91 Mon Sep 17 00:00:00 2001 From: Presley Pizzo <1290996+presleyp@users.noreply.github.com> Date: Wed, 26 Oct 2022 13:08:22 -0400 Subject: [PATCH] chore: refactor pagination (#4753) * Extract PageButton * Fix import * Extract utils * Format * Separate pagination - wip * Spawn pagination machine - buggy filter * Make labels optional * Layout, fix send reset bug * Format * Fix refresh data bug * Remove debugging line * Fix url updates setSearchParams overwrites all search params, rather than merging * Update Audit Page * Simplify pagination widget * Fix workspaces story * Fix Audit story * Fix pagination story and pagebutton highlight * Fix pagination tests * Add to utils tests * Format * Add tests --- .../PaginationWidget/PageButton.tsx | 52 ++++ .../PaginationWidget.stories.tsx | 50 ++-- .../PaginationWidget.test.tsx | 48 ++-- .../PaginationWidget/PaginationWidget.tsx | 159 +++--------- .../{buildPagedList.test.ts => utils.test.ts} | 15 +- site/src/components/PaginationWidget/utils.ts | 104 ++++++++ site/src/pages/AuditPage/AuditPage.test.tsx | 24 ++ site/src/pages/AuditPage/AuditPage.tsx | 40 +-- .../pages/AuditPage/AuditPageView.stories.tsx | 24 +- site/src/pages/AuditPage/AuditPageView.tsx | 26 +- .../pages/WorkspacesPage/WorkspacesPage.tsx | 45 +--- .../WorkspacesPageView.stories.tsx | 4 + .../WorkspacesPage/WorkspacesPageView.tsx | 24 +- site/src/xServices/audit/auditXService.ts | 94 +++---- .../pagination/paginationXService.ts | 66 +++++ .../workspaces/workspacesXService.ts | 234 +++++++++--------- 16 files changed, 548 insertions(+), 461 deletions(-) create mode 100644 site/src/components/PaginationWidget/PageButton.tsx rename site/src/components/PaginationWidget/{buildPagedList.test.ts => utils.test.ts} (68%) create mode 100644 site/src/components/PaginationWidget/utils.ts create mode 100644 site/src/xServices/pagination/paginationXService.ts diff --git a/site/src/components/PaginationWidget/PageButton.tsx b/site/src/components/PaginationWidget/PageButton.tsx new file mode 100644 index 0000000000..f5a650099f --- /dev/null +++ b/site/src/components/PaginationWidget/PageButton.tsx @@ -0,0 +1,52 @@ +import Button from "@material-ui/core/Button" +import { makeStyles } from "@material-ui/core/styles" + +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 styles = useStyles() + return ( + + ) +} + +const useStyles = makeStyles((theme) => ({ + pageButton: { + "&:not(:last-of-type)": { + marginRight: theme.spacing(0.5), + }, + }, + + activePageButton: { + borderColor: `${theme.palette.info.main}`, + backgroundColor: `${theme.palette.info.dark}`, + }, +})) diff --git a/site/src/components/PaginationWidget/PaginationWidget.stories.tsx b/site/src/components/PaginationWidget/PaginationWidget.stories.tsx index d64b9f9acf..6e36bff4ac 100644 --- a/site/src/components/PaginationWidget/PaginationWidget.stories.tsx +++ b/site/src/components/PaginationWidget/PaginationWidget.stories.tsx @@ -1,65 +1,53 @@ -import { action } from "@storybook/addon-actions" import { Story } from "@storybook/react" import { PaginationWidget, PaginationWidgetProps } from "./PaginationWidget" +import { createPaginationRef } from "./utils" export default { title: "components/PaginationWidget", component: PaginationWidget, + argTypes: { + prevLabel: { + defaultValue: "Previous", + }, + nextLabel: { + defaultValue: "Next", + }, + paginationRef: { + defaultValue: createPaginationRef({ page: 1, limit: 12 }), + }, + numRecords: { + defaultValue: 200, + }, + }, } const Template: Story = ( args: PaginationWidgetProps, ) => -const defaultProps = { - prevLabel: "Previous", - nextLabel: "Next", - onPrevClick: action("previous"), - onNextClick: action("next"), - onPageClick: action("clicked"), -} - export const UnknownPageNumbers = Template.bind({}) UnknownPageNumbers.args = { - ...defaultProps, + numRecords: undefined, } export const LessThan8Pages = Template.bind({}) LessThan8Pages.args = { - ...defaultProps, numRecords: 84, - numRecordsPerPage: 12, - activePage: 1, } export const MoreThan8Pages = Template.bind({}) -MoreThan8Pages.args = { - ...defaultProps, - numRecords: 200, - numRecordsPerPage: 12, - activePage: 1, -} export const MoreThan7PagesWithActivePageCloseToStart = Template.bind({}) MoreThan7PagesWithActivePageCloseToStart.args = { - ...defaultProps, - numRecords: 200, - numRecordsPerPage: 12, - activePage: 2, + paginationRef: createPaginationRef({ page: 2, limit: 12 }), } export const MoreThan7PagesWithActivePageFarFromBoundaries = Template.bind({}) MoreThan7PagesWithActivePageFarFromBoundaries.args = { - ...defaultProps, - numRecords: 200, - numRecordsPerPage: 12, - activePage: 4, + paginationRef: createPaginationRef({ page: 4, limit: 12 }), } export const MoreThan7PagesWithActivePageCloseToEnd = Template.bind({}) MoreThan7PagesWithActivePageCloseToEnd.args = { - ...defaultProps, - numRecords: 200, - numRecordsPerPage: 12, - activePage: 17, + paginationRef: createPaginationRef({ page: 17, limit: 12 }), } diff --git a/site/src/components/PaginationWidget/PaginationWidget.test.tsx b/site/src/components/PaginationWidget/PaginationWidget.test.tsx index ffc330de38..d98a07a2b5 100644 --- a/site/src/components/PaginationWidget/PaginationWidget.test.tsx +++ b/site/src/components/PaginationWidget/PaginationWidget.test.tsx @@ -1,6 +1,7 @@ import { screen } from "@testing-library/react" import { render } from "../../testHelpers/renderHelpers" import { PaginationWidget } from "./PaginationWidget" +import { createPaginationRef } from "./utils" describe("PaginatedList", () => { it("displays an accessible previous and next button", () => { @@ -8,20 +9,13 @@ describe("PaginatedList", () => { jest.fn()} - onNextClick={() => jest.fn()} />, ) - expect( - screen.getByRole("button", { name: "Previous page" }), - ).toBeInTheDocument() - expect( - screen.getByRole("button", { name: "Next page" }), - ).toBeInTheDocument() + expect(screen.getByRole("button", { name: "Previous page" })).toBeEnabled() + expect(screen.getByRole("button", { name: "Next page" })).toBeEnabled() }) it("displays the expected number of pages with one ellipsis tile", () => { @@ -29,12 +23,8 @@ describe("PaginatedList", () => { jest.fn()} - onNextClick={() => jest.fn()} - onPageClick={(_) => jest.fn()} numRecords={200} - numRecordsPerPage={12} - activePage={1} + paginationRef={createPaginationRef({ page: 1, limit: 12 })} />, ) @@ -49,12 +39,8 @@ describe("PaginatedList", () => { jest.fn()} - onNextClick={() => jest.fn()} - onPageClick={(_) => jest.fn()} numRecords={200} - numRecordsPerPage={12} - activePage={6} + paginationRef={createPaginationRef({ page: 6, limit: 12 })} />, ) @@ -63,4 +49,26 @@ describe("PaginatedList", () => { container.querySelectorAll(`button[name="Page button"]`), ).toHaveLength(5) }) + + it("disables the previous button on the first page", () => { + render( + , + ) + const prevButton = screen.getByLabelText("Previous page") + expect(prevButton).toBeDisabled() + }) + + it("disables the next button on the last page", () => { + render( + , + ) + const nextButton = screen.getByLabelText("Next page") + expect(nextButton).toBeDisabled() + }) }) diff --git a/site/src/components/PaginationWidget/PaginationWidget.tsx b/site/src/components/PaginationWidget/PaginationWidget.tsx index 040fb4d311..64ad521d6d 100644 --- a/site/src/components/PaginationWidget/PaginationWidget.tsx +++ b/site/src/components/PaginationWidget/PaginationWidget.tsx @@ -3,129 +3,40 @@ import { makeStyles, useTheme } from "@material-ui/core/styles" import useMediaQuery from "@material-ui/core/useMediaQuery" import KeyboardArrowLeft from "@material-ui/icons/KeyboardArrowLeft" import KeyboardArrowRight from "@material-ui/icons/KeyboardArrowRight" +import { useActor } from "@xstate/react" import { ChooseOne, Cond } from "components/Conditionals/ChooseOne" import { Maybe } from "components/Conditionals/Maybe" import { CSSProperties } from "react" +import { PaginationMachineRef } from "xServices/pagination/paginationXService" +import { PageButton } from "./PageButton" +import { buildPagedList } from "./utils" export type PaginationWidgetProps = { - prevLabel: string - nextLabel: string - onPrevClick: () => void - onNextClick: () => void - onPageClick?: (page: number) => void - numRecordsPerPage?: number + prevLabel?: string + nextLabel?: string numRecords?: number - activePage?: number containerStyle?: CSSProperties -} - -/** - * 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 - */ -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. -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. -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 - -/** - * 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 - */ -export const buildPagedList = ( - numPages: number, - activePage: number, -): (string | 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 - - pages = range(startPage, endPage) - - const singleSpillOffset = PAGES_TO_DISPLAY - pages.length - 1 - const hasLeftOverflow = startPage > 2 - const hasRightOverflow = endPage < beforeLastPage - const leftOverflowPage = "left" - const rightOverflowPage = "right" - - 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] - } - - return [1, ...pages, numPages] - } - - return range(1, numPages) -} - -interface PageButtonProps { - activePage: number - page: number - numPages: number - onPageClick?: (page: number) => void -} - -const PageButton = ({ - activePage, - page, - numPages, - onPageClick, -}: PageButtonProps): JSX.Element => { - const styles = useStyles() - return ( - - ) + paginationRef: PaginationMachineRef } export const PaginationWidget = ({ - prevLabel, - nextLabel, - onPrevClick, - onNextClick, - onPageClick, + prevLabel = "", + nextLabel = "", numRecords, - numRecordsPerPage = DEFAULT_RECORDS_PER_PAGE, - activePage = 1, containerStyle, + paginationRef, }: PaginationWidgetProps): JSX.Element | null => { - const numPages = numRecords ? Math.ceil(numRecords / numRecordsPerPage) : 0 - const firstPageActive = activePage === 1 && numPages !== 0 - const lastPageActive = activePage === numPages && numPages !== 0 const theme = useTheme() const isMobile = useMediaQuery(theme.breakpoints.down("sm")) const styles = useStyles() + const [paginationState, send] = useActor(paginationRef) + + const currentPage = paginationState.context.page + const numRecordsPerPage = paginationState.context.limit + + const numPages = numRecords ? Math.ceil(numRecords / numRecordsPerPage) : 0 + const firstPageActive = currentPage === 1 && numPages !== 0 + const lastPageActive = currentPage === numPages && numPages !== 0 // No need to display any pagination if we know the number of pages is 1 or 0 if (numPages <= 1 || numRecords === 0) { @@ -138,7 +49,7 @@ export const PaginationWidget = ({ className={styles.prevLabelStyles} aria-label="Previous page" disabled={firstPageActive} - onClick={onPrevClick} + onClick={() => send({ type: "PREVIOUS_PAGE" })} >
{prevLabel}
@@ -147,28 +58,27 @@ export const PaginationWidget = ({ - {buildPagedList(numPages, activePage).map((page) => + {buildPagedList(numPages, currentPage).map((page) => typeof page !== "number" ? ( - + /> ) : ( send({ type: "GO_TO_PAGE", page })} /> ), )} @@ -178,7 +88,7 @@ export const PaginationWidget = ({