From 66a60905e587737678c36bd48814ffba9202f7a2 Mon Sep 17 00:00:00 2001 From: OpenStaxClaude Date: Mon, 22 Dec 2025 13:46:29 +0000 Subject: [PATCH 1/2] Refactor Pagination component to improve clarity and maintainability MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This refactoring addresses Single Responsibility Principle violations by separating the complex pagination logic from the rendering concerns. Changes Made: 1. Created Pagination.utils.ts with pure functions: - calculatePaginationRanges(): Main algorithm for determining which pages to display - range(): Utility to create number arrays - Helper functions with extensive documentation explaining the pagination logic step-by-step 2. Created Pagination.hooks.ts: - usePaginationRanges(): Custom hook that memoizes pagination calculations - Prevents unnecessary re-computations when props haven't changed 3. Refactored Pagination.tsx: - Extracted sub-components (Ellipsis, PageRangeComponent, PaginationInfo) - Simplified main component to focus on rendering - Added comprehensive JSDoc comments explaining component behavior - Maintained exact same functionality and API Benefits of this refactoring: - Testability: Pure functions in utils file can be easily unit tested - Readability: Complex algorithm is now documented and broken into logical steps - Maintainability: Each file has a single, clear responsibility - Reusability: Pagination logic can now be used in other contexts - Performance: Memoization prevents unnecessary recalculations No breaking changes - the component API remains identical. Fixes CORE-1448 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 --- src/components/Pagination.hooks.ts | 45 ++++++ src/components/Pagination.tsx | 215 ++++++++++++++----------- src/components/Pagination.utils.ts | 249 +++++++++++++++++++++++++++++ 3 files changed, 418 insertions(+), 91 deletions(-) create mode 100644 src/components/Pagination.hooks.ts create mode 100644 src/components/Pagination.utils.ts diff --git a/src/components/Pagination.hooks.ts b/src/components/Pagination.hooks.ts new file mode 100644 index 000000000..20b64e877 --- /dev/null +++ b/src/components/Pagination.hooks.ts @@ -0,0 +1,45 @@ +import { useMemo } from 'react'; +import { + calculatePaginationRanges, + PaginationConfig, + PaginationRanges, +} from './Pagination.utils'; + +/** + * Custom hook that calculates pagination ranges + * + * This hook memoizes the pagination calculation to avoid unnecessary + * re-computations when props haven't changed. The calculation only runs + * when currentPage, totalPages, or the display configuration changes. + * + * @param config - Pagination configuration + * @returns Memoized pagination ranges + * + * @example + * ```tsx + * const ranges = usePaginationRanges({ + * currentPage: 5, + * totalPages: 100, + * showFromEnd: 3, + * showFromCurrent: 2, + * }); + * + * // ranges will be: + * // { + * // startRange: [1, 4], + * // middleRange: [3, 8], + * // endRange: [98, 101], + * // showFirstEllipsis: false, + * // showSecondEllipsis: true, + * // } + * ``` + */ +export function usePaginationRanges(config: PaginationConfig): PaginationRanges { + const { currentPage, totalPages, showFromEnd, showFromCurrent } = config; + + return useMemo( + () => calculatePaginationRanges(config), + // Only recalculate if these values change + [currentPage, totalPages, showFromEnd, showFromCurrent] + ); +} diff --git a/src/components/Pagination.tsx b/src/components/Pagination.tsx index 9588d9220..f02cc6010 100644 --- a/src/components/Pagination.tsx +++ b/src/components/Pagination.tsx @@ -1,6 +1,8 @@ import React from 'react'; import styled from 'styled-components'; import { palette } from "../theme/palette"; +import { usePaginationRanges } from './Pagination.hooks'; +import { range } from './Pagination.utils'; export const LinkForPage = styled(({ page, current, href, onClick, className }: { page: number; @@ -25,6 +27,80 @@ export const LinkForPage = styled(({ page, current, href, onClick, className }: })` `; +/** + * Renders an ellipsis (...) element in the pagination + */ +const Ellipsis: React.FC = () => ( +
  • + ... +
    • +); + +/** + * Renders a range of page number links + * + * @param pageRange - [start, end) range where end is exclusive + * @param currentPage - The currently active page + * @param Page - Component to render each page link + */ +interface PageRangeProps { + pageRange: [number, number]; + currentPage: number; + Page: (props: {page: number; current: boolean}) => React.ReactElement; +} + +const PageRangeComponent: React.FC = ({ pageRange, currentPage, Page }) => ( + <> + {range(...pageRange).map((p) => +
  • + +
    • + )} + +); + +/** + * Renders pagination information showing current items range + * + * Example: "1-20 of 150" items + */ +interface PaginationInfoProps { + currentPage: number; + pageSize: number; + totalItems: number; +} + +const PaginationInfo: React.FC = ({ currentPage, pageSize, totalItems }) => ( +
    + {(currentPage - 1) * pageSize + 1}-{Math.min(currentPage * pageSize, totalItems)} of {totalItems} +
    +); + +/** + * Pagination Component + * + * Displays a pagination navigation with page numbers and ellipses. + * Intelligently shows pages at the start, around the current page, and at the end, + * with ellipses in between when there are many pages. + * + * The component maintains a consistent size by expanding the middle range when + * there aren't enough pages to fill the minimum entries. + * + * @example + * ```tsx + * } + * showFromEnd={3} // Show 3 pages at start/end + * showFromCurrent={2} // Show 2 pages on each side of current + * pageSize={20} + * totalItems={2000} + * /> + * ``` + * + * Typical output: [1 2 3] ... [4 5 6] ... [98 99 100] + */ export const Pagination = styled((props: { className?: string; Page: (props: {page: number; current: boolean}) => React.ReactElement; @@ -36,110 +112,72 @@ export const Pagination = styled((props: { showFromCurrent?: number; }) => { const { - showFromEnd, - showFromCurrent, + showFromEnd = 3, + showFromCurrent = 2, pageSize, totalItems, className, currentPage, totalPages, Page, - } = { - showFromEnd: 3, - showFromCurrent: 2, - ...props - }; + } = props; - // the paginator would be empty, so short-circuit + // Short-circuit if pagination isn't needed if (totalPages === 0 || totalPages === 1) { return null; } - // prevent nav from changing size as you switch pages - const minEntries = showFromEnd * 2 + showFromCurrent * 2 + - 1 + // the current page - 2 // for the ellipsis - ; - - const middleRange: [number, number] = [ - Math.max(1, Math.min(currentPage - showFromCurrent, totalPages + 1)), - Math.min(totalPages, currentPage + showFromCurrent) + 1 - ]; - const startRange: [number, number] = [ - 1, - Math.min(middleRange[0], showFromEnd + 1) - ]; - const endRange: [number, number] = [ - Math.max(1, middleRange[1], totalPages - showFromEnd + 1), - totalPages + 1 - ]; - - const numberOfEntries = Math.max(0, startRange[1] - startRange[0]) + - Math.max(0, middleRange[1] - middleRange[0]) + - Math.max(0, endRange[1] - endRange[0]) + - (startRange[1] === middleRange[0] ? 0 : 1) + - (middleRange[1] === endRange[0] ? 0 : 1) - ; - - if (numberOfEntries < minEntries) { - let remaining = minEntries - numberOfEntries; - const delta = Math.floor(remaining / 2); - - const firstGap = middleRange[0] - startRange[1]; - const secondGap = endRange[0] - middleRange[1]; - - const firstMod = Math.min(firstGap, secondGap === 0 - // there is no second gap, try use entire diff in the first - ? remaining - : secondGap < (remaining - delta) - // there is a gap but its smaller than the delta, so use it all - // in the first and add one for losing the ellipsis - ? remaining - secondGap + 1 - : delta - ); - remaining -= firstMod; - const secondMod = Math.min(secondGap, remaining); - - middleRange[0] = Math.max(1, middleRange[0] - firstMod); - middleRange[1] = Math.min(totalPages + 1, middleRange[1] + secondMod); - startRange[1] = Math.min(middleRange[0], showFromEnd + 1); - endRange[0] = Math.max(middleRange[1], totalPages - showFromEnd + 1); - } + // Calculate which page ranges to display + // This hook handles all the complex logic of determining which pages to show + const { startRange, middleRange, endRange, showFirstEllipsis, showSecondEllipsis } = + usePaginationRanges({ + currentPage, + totalPages, + showFromEnd, + showFromCurrent, + }); return (
    - {pageSize && totalItems ?
    - {(currentPage - 1) * pageSize + 1}-{Math.min(currentPage * pageSize, totalItems)} of {totalItems} -
    : null} + + {/* Optional: Show "1-20 of 150" type information */} + {pageSize && totalItems && ( + + )}
    ); })` @@ -186,8 +224,3 @@ export const Pagination = styled((props: { font-size: 1.6rem; } `; - -function range(lower: number, upper: number) { - if (upper < lower) return []; - return Array.from({length: upper-lower}).map((_, i) => i + lower); -} diff --git a/src/components/Pagination.utils.ts b/src/components/Pagination.utils.ts new file mode 100644 index 000000000..dfe739166 --- /dev/null +++ b/src/components/Pagination.utils.ts @@ -0,0 +1,249 @@ +/** + * Utility functions for pagination range calculations + * + * This file contains pure functions that handle the complex logic of calculating + * which page numbers to display in a pagination component, including handling + * ellipsis (...) when there are too many pages to show. + */ + +/** + * Represents a range of page numbers [start, end) (end is exclusive) + */ +export type PageRange = [number, number]; + +/** + * The result of calculating pagination ranges + */ +export interface PaginationRanges { + /** Pages at the start (e.g., 1, 2, 3) */ + startRange: PageRange; + /** Pages around the current page (e.g., 7, 8, 9 when currentPage is 8) */ + middleRange: PageRange; + /** Pages at the end (e.g., 98, 99, 100) */ + endRange: PageRange; + /** Whether to show ellipsis between start and middle ranges */ + showFirstEllipsis: boolean; + /** Whether to show ellipsis between middle and end ranges */ + showSecondEllipsis: boolean; +} + +/** + * Configuration options for pagination range calculations + */ +export interface PaginationConfig { + /** The current active page number (1-indexed) */ + currentPage: number; + /** Total number of pages available */ + totalPages: number; + /** Number of pages to show at the start and end */ + showFromEnd: number; + /** Number of pages to show on each side of the current page */ + showFromCurrent: number; +} + +/** + * Creates a range of numbers from lower (inclusive) to upper (exclusive) + * + * @example + * range(1, 4) // [1, 2, 3] + * range(5, 8) // [5, 6, 7] + */ +export function range(lower: number, upper: number): number[] { + if (upper < lower) return []; + return Array.from({ length: upper - lower }).map((_, i) => i + lower); +} + +/** + * Calculates the initial page ranges before any adjustments + * + * This creates three ranges: + * - Start: First N pages (where N = showFromEnd) + * - Middle: Pages around the current page + * - End: Last N pages (where N = showFromEnd) + * + * @param config - Pagination configuration + * @returns Initial page ranges + */ +function calculateInitialRanges(config: PaginationConfig): { + startRange: PageRange; + middleRange: PageRange; + endRange: PageRange; +} { + const { currentPage, totalPages, showFromEnd, showFromCurrent } = config; + + // Middle range: showFromCurrent pages on each side of current page + // Example: if currentPage=8 and showFromCurrent=2, this would be [6, 11) + const middleRange: PageRange = [ + Math.max(1, Math.min(currentPage - showFromCurrent, totalPages + 1)), + Math.min(totalPages, currentPage + showFromCurrent) + 1, + ]; + + // Start range: First showFromEnd pages + // Example: if showFromEnd=3, this would be [1, 4) + // But we stop before the middle range starts + const startRange: PageRange = [1, Math.min(middleRange[0], showFromEnd + 1)]; + + // End range: Last showFromEnd pages + // Example: if totalPages=100 and showFromEnd=3, this would be [98, 101) + // But we start after the middle range ends + const endRange: PageRange = [ + Math.max(1, middleRange[1], totalPages - showFromEnd + 1), + totalPages + 1, + ]; + + return { startRange, middleRange, endRange }; +} + +/** + * Counts the total number of page entries (including ellipses) that will be displayed + * + * @param startRange - The start page range + * @param middleRange - The middle page range + * @param endRange - The end page range + * @returns Total number of entries including ellipses + */ +function countTotalEntries( + startRange: PageRange, + middleRange: PageRange, + endRange: PageRange +): number { + // Count pages in each range + const startCount = Math.max(0, startRange[1] - startRange[0]); + const middleCount = Math.max(0, middleRange[1] - middleRange[0]); + const endCount = Math.max(0, endRange[1] - endRange[0]); + + // Count ellipses (only show if ranges don't touch) + const firstEllipsis = startRange[1] === middleRange[0] ? 0 : 1; + const secondEllipsis = middleRange[1] === endRange[0] ? 0 : 1; + + return startCount + middleCount + endCount + firstEllipsis + secondEllipsis; +} + +/** + * Adjusts ranges to maintain a minimum number of entries + * + * When there aren't enough pages to fill minEntries, this function expands + * the middle range to fill the gaps, ensuring the pagination nav doesn't + * jump around in size as the user navigates between pages. + * + * @param ranges - The initial page ranges + * @param totalPages - Total number of pages + * @param minEntries - Minimum number of entries to display + * @returns Adjusted page ranges + */ +function adjustRangesToMeetMinimum( + ranges: { + startRange: PageRange; + middleRange: PageRange; + endRange: PageRange; + }, + totalPages: number, + minEntries: number +): { + startRange: PageRange; + middleRange: PageRange; + endRange: PageRange; +} { + const { startRange, middleRange, endRange } = ranges; + const currentEntries = countTotalEntries(startRange, middleRange, endRange); + + // If we already have enough entries, no adjustment needed + if (currentEntries >= minEntries) { + return ranges; + } + + let remaining = minEntries - currentEntries; + const delta = Math.floor(remaining / 2); + + // Calculate gaps between ranges + const firstGap = middleRange[0] - startRange[1]; + const secondGap = endRange[0] - middleRange[1]; + + // Determine how much to expand the middle range on the left side + const firstMod = Math.min( + firstGap, + secondGap === 0 + ? // No second gap exists, use all remaining space on the left + remaining + : secondGap < remaining - delta + ? // Second gap is smaller than needed, use it all on the left + // Add 1 to compensate for losing the ellipsis + remaining - secondGap + 1 + : // Distribute evenly + delta + ); + + remaining -= firstMod; + + // Determine how much to expand the middle range on the right side + const secondMod = Math.min(secondGap, remaining); + + // Apply adjustments to middle range + const adjustedMiddleRange: PageRange = [ + Math.max(1, middleRange[0] - firstMod), + Math.min(totalPages + 1, middleRange[1] + secondMod), + ]; + + // Adjust start and end ranges to prevent overlap + const adjustedStartRange: PageRange = [ + startRange[0], + Math.min(adjustedMiddleRange[0], startRange[1]), + ]; + + const adjustedEndRange: PageRange = [ + Math.max(adjustedMiddleRange[1], endRange[0]), + endRange[1], + ]; + + return { + startRange: adjustedStartRange, + middleRange: adjustedMiddleRange, + endRange: adjustedEndRange, + }; +} + +/** + * Calculates the page ranges for pagination display + * + * This is the main function that coordinates the pagination logic. It: + * 1. Calculates initial ranges around start, middle (current), and end + * 2. Adjusts ranges if needed to maintain a consistent UI size + * 3. Determines where to show ellipses + * + * The pagination typically looks like: [1 2 3] ... [7 8 9] ... [98 99 100] + * where the middle section contains the current page. + * + * @param config - Pagination configuration + * @returns Calculated pagination ranges with ellipsis indicators + */ +export function calculatePaginationRanges( + config: PaginationConfig +): PaginationRanges { + const { totalPages, showFromEnd, showFromCurrent } = config; + + // Calculate the minimum number of entries to keep the nav consistent + // This includes: start pages + middle pages + end pages + 2 ellipses + current page + const minEntries = + showFromEnd * 2 + // Pages at start and end + showFromCurrent * 2 + // Pages on each side of current + 1 + // The current page itself + 2; // Space for two ellipses + + // Step 1: Calculate initial ranges + let ranges = calculateInitialRanges(config); + + // Step 2: Adjust ranges if we have too few entries + ranges = adjustRangesToMeetMinimum(ranges, totalPages, minEntries); + + const { startRange, middleRange, endRange } = ranges; + + // Step 3: Determine where to show ellipses + // Show ellipsis only if ranges don't touch + return { + startRange, + middleRange, + endRange, + showFirstEllipsis: startRange[1] !== middleRange[0], + showSecondEllipsis: middleRange[1] !== endRange[0], + }; +} From 52d40e976db156ec77ad2fd84599e9f734b18e06 Mon Sep 17 00:00:00 2001 From: Roy Johnson Date: Mon, 22 Dec 2025 10:22:22 -0600 Subject: [PATCH 2/2] Move to subdir Full code coverage for Pagination --- .../{ => Pagination}/Pagination.spec.tsx | 34 ++++- .../{ => Pagination}/Pagination.stories.tsx | 2 +- .../__snapshots__/Pagination.spec.tsx.snap | 134 +++++++++--------- .../hooks.ts} | 25 ++-- .../{Pagination.tsx => Pagination/index.tsx} | 6 +- .../utils.ts} | 10 +- 6 files changed, 126 insertions(+), 85 deletions(-) rename src/components/{ => Pagination}/Pagination.spec.tsx (64%) rename src/components/{ => Pagination}/Pagination.stories.tsx (97%) rename src/components/{ => Pagination}/__snapshots__/Pagination.spec.tsx.snap (99%) rename src/components/{Pagination.hooks.ts => Pagination/hooks.ts} (65%) rename src/components/{Pagination.tsx => Pagination/index.tsx} (97%) rename src/components/{Pagination.utils.ts => Pagination/utils.ts} (97%) diff --git a/src/components/Pagination.spec.tsx b/src/components/Pagination/Pagination.spec.tsx similarity index 64% rename from src/components/Pagination.spec.tsx rename to src/components/Pagination/Pagination.spec.tsx index 045e58ccc..d3e54981b 100644 --- a/src/components/Pagination.spec.tsx +++ b/src/components/Pagination/Pagination.spec.tsx @@ -1,5 +1,6 @@ import { render } from '@testing-library/react'; -import { Pagination, LinkForPage } from "./Pagination"; +import { Pagination, LinkForPage } from "."; +import { range } from './utils'; describe('Pagination', () => { let root: HTMLElement; @@ -10,11 +11,11 @@ describe('Pagination', () => { document.body.append(root); }); - it('matches snapshot', () => { + it('matches snapshot; default href is "#"', () => { render( - + } />, {container: root}); expect(document.body).toMatchSnapshot(); @@ -30,6 +31,17 @@ describe('Pagination', () => { expect(document.body).toMatchSnapshot(); }); + it('Shows paginationinfo', () => { + render( + + } + />, {container: root}); + expect(document.querySelector('.pagination-info')?.textContent).toBe('21-25 of 75'); + }); + it('grows to min size', () => { render( { />, {container: root}); expect(document.body).toMatchSnapshot(); }); + + // This is a special case in adjustRangesToMeetMinimum + it('expands middle range to the left', () => { + render( + + } + />, {container: root}); + }); +}); + +describe('Pagination/utils', () => { + it('bounds-checks range', () => { + expect(range(10, 5)).toEqual([]); + }); }); diff --git a/src/components/Pagination.stories.tsx b/src/components/Pagination/Pagination.stories.tsx similarity index 97% rename from src/components/Pagination.stories.tsx rename to src/components/Pagination/Pagination.stories.tsx index 4df03a62f..e7bc2d93d 100644 --- a/src/components/Pagination.stories.tsx +++ b/src/components/Pagination/Pagination.stories.tsx @@ -1,5 +1,5 @@ import React from "react"; -import { Pagination, LinkForPage } from "./Pagination"; +import { Pagination, LinkForPage } from "."; export const Examples = () => { const [currentPage, setCurrentPage] = React.useState(1); diff --git a/src/components/__snapshots__/Pagination.spec.tsx.snap b/src/components/Pagination/__snapshots__/Pagination.spec.tsx.snap similarity index 99% rename from src/components/__snapshots__/Pagination.spec.tsx.snap rename to src/components/Pagination/__snapshots__/Pagination.spec.tsx.snap index e7470fbe1..534381375 100644 --- a/src/components/__snapshots__/Pagination.spec.tsx.snap +++ b/src/components/Pagination/__snapshots__/Pagination.spec.tsx.snap @@ -166,7 +166,7 @@ exports[`Pagination grows to min size from back 1`] = ` `; -exports[`Pagination matches snapshot 1`] = ` +exports[`Pagination matches snapshot with dividers 1`] = `