diff --git a/src/components/navigation/Pagination.tsx b/src/components/navigation/Pagination.tsx index a8514239..2d2d3cc3 100644 --- a/src/components/navigation/Pagination.tsx +++ b/src/components/navigation/Pagination.tsx @@ -1,7 +1,7 @@ import classnames from 'classnames'; import type { ComponentChildren } from 'preact'; -import { pageNumberOptions } from '../../util/pagination'; +import { paginationItems } from '../../util/pagination'; import { ArrowLeftIcon, ArrowRightIcon } from '../icons'; import Button from '../input/Button'; @@ -75,7 +75,7 @@ export default function Pagination({ // Pages are 1-indexed const hasNextPage = currentPage < totalPages; const hasPreviousPage = currentPage > 1; - const pageNumbers = pageNumberOptions(currentPage, totalPages); + const pageNumbers = paginationItems(currentPage, totalPages); const changePageTo = (pageNumber: number, element: HTMLElement) => { onChangePage(pageNumber); diff --git a/src/components/navigation/test/Pagination-test.js b/src/components/navigation/test/Pagination-test.js index 4d5f90b8..f71a94c4 100644 --- a/src/components/navigation/test/Pagination-test.js +++ b/src/components/navigation/test/Pagination-test.js @@ -4,7 +4,7 @@ import Pagination, { $imports } from '../Pagination'; describe('Pagination', () => { let fakeOnChangePage; - let fakePageNumberOptions; + let fakePaginationItems; const findButton = (wrapper, title) => wrapper.find('button').filterWhere(n => n.props().title === title); @@ -22,10 +22,10 @@ describe('Pagination', () => { beforeEach(() => { fakeOnChangePage = sinon.stub(); - fakePageNumberOptions = sinon.stub().returns([1, 2, 3, 4, null, 10]); + fakePaginationItems = sinon.stub().returns([1, 2, 3, 4, null, 10]); $imports.$mock({ - '../../util/pagination': { pageNumberOptions: fakePageNumberOptions }, + '../../util/pagination': { paginationItems: fakePaginationItems }, }); }); @@ -103,7 +103,7 @@ describe('Pagination', () => { describe('page number buttons', () => { it('should render buttons for each page number available', () => { - fakePageNumberOptions.returns([1, 2, 3, 4, null, 10]); + fakePaginationItems.returns([1, 2, 3, 4, null, 10]); const wrapper = createComponent(); [1, 2, 3, 4, 10].forEach(pageNumber => { @@ -116,7 +116,7 @@ describe('Pagination', () => { }); it('should invoke the onChangePage callback when page number button clicked', () => { - fakePageNumberOptions.returns([1, 2, 3, 4, null, 10]); + fakePaginationItems.returns([1, 2, 3, 4, null, 10]); const wrapper = createComponent(); [1, 2, 3, 4, 10].forEach(pageNumber => { diff --git a/src/util/pagination.ts b/src/util/pagination.ts index 5190e094..055e6a16 100644 --- a/src/util/pagination.ts +++ b/src/util/pagination.ts @@ -4,70 +4,145 @@ */ type PageNumber = number | null; +export type Options = { + /** + * Number of pages to display at the start and end, including the start/end + * page. + * + * This must be >= 1. + */ + boundaryCount?: number; + + /** + * Number of pages to display before and after the current page. + */ + siblingCount?: number; +}; + /** * Determine the set of (pagination) page numbers that should be provided to - * a user, given the current page the user is on, the total number of pages - * available, and the number of individual page options desired. + * a user. * - * The first, last and current pages will always be included in the returned - * results. Additional pages adjacent to the current page will be added until - * `maxPages` is reached. Gaps in the sequence of pages are represented by - * `null` values. + * The result includes a mixture of page numbers that should be shown, plus + * `null` values indicating elided page numbers. The goals of the selection + * are: * - * @example - * pageNumberOptions(1, 10, 5) => [1, 2, 3, 4, null, 10] - * pageNumberOptions(3, 10, 5) => [1, 2, 3, 4, null, 10] - * pageNumberOptions(6, 10, 5) => [1, null, 5, 6, 7, null, 10] - * pageNumberOptions(9, 10, 5) => [1, null, 7, 8, 9, 10] - * pageNumberOptions(2, 3, 5) => [1, 2, 3] + * - To always provide page numbers for the first, last and current pages. + * Additional adjacent pages are provided according to the `boundaryCount` + * and `siblingCount` options. + * - To try and keep the number of pagination items consistent as the current + * page changes. If each item is rendered with approximately the same width, + * this keeps the overall width of the pagination component and the location + * of child controls consistent as the user navigates. This helps to avoid + * mis-clicks due to controls moving around under the cursor. * - * @param currentPage - The currently-visible/-active page of results. - * Note that pages are 1-indexed - * @param maxPages - The maximum number of numbered pages to make available - * @return Set of navigation page options to show. `null` values represent gaps - * in the sequence of pages, to be represented later as ellipses (...) + * @param currentPage - The 1-based currently-visible/-active page number. + * @param totalPages - The total number of pages + * @param options - Options for the number of pages to show at the boundary and + * around the current page. */ -export function pageNumberOptions( +export function paginationItems( currentPage: number, totalPages: number, /* istanbul ignore next */ - maxPages = 5, + { boundaryCount = 1, siblingCount = 1 }: Options = {}, ): PageNumber[] { if (totalPages <= 1) { return []; } - // Start with first, last and current page. Use a set to avoid dupes. - const pageNumbers = new Set([1, currentPage, totalPages]); + currentPage = Math.max(1, Math.min(currentPage, totalPages)); + boundaryCount = Math.max(boundaryCount, 1); + siblingCount = Math.max(siblingCount, 0); + + const pageNumbers: PageNumber[] = []; + const beforeCurrent = currentPage - 1; + const afterCurrent = totalPages - currentPage; + + // Index of indicator for pages elided before current. + let elideBeforeIndex = null; + // Number of last page elided before current. + let elideBeforeValue = 1; - // Fill out the `pageNumbers` with additional pages near the currentPage, - // if available - let increment = 1; - while (pageNumbers.size < Math.min(totalPages, maxPages)) { - // Build the set "outward" from the currently-active page - if (currentPage + increment <= totalPages) { - pageNumbers.add(currentPage + increment); + // Add pages before current. + const elideBeforeCurrent = boundaryCount + siblingCount < beforeCurrent; + if (elideBeforeCurrent) { + for (let page = 1; page <= boundaryCount; page++) { + pageNumbers.push(page); } - if (currentPage - increment >= 1) { - pageNumbers.add(currentPage - increment); + elideBeforeIndex = pageNumbers.length; + elideBeforeValue = currentPage - siblingCount - 1; + pageNumbers.push(null); + for (let page = currentPage - siblingCount; page < currentPage; page++) { + pageNumbers.push(page); + } + } else { + for (let page = 1; page < currentPage; page++) { + pageNumbers.push(page); } - ++increment; } - const pageOptions: PageNumber[] = []; + pageNumbers.push(currentPage); + + // Index of indicator for pages elided after current. + let elideAfterIndex = null; + // Number of first page elided after current. + let elideAfterValue = currentPage; + + const elideAfterCurrent = boundaryCount + siblingCount < afterCurrent; + if (elideAfterCurrent) { + for ( + let page = currentPage + 1; + page <= currentPage + siblingCount; + page++ + ) { + pageNumbers.push(page); + elideAfterValue = page + 1; + } + elideAfterIndex = pageNumbers.length; + pageNumbers.push(null); + for ( + let page = totalPages - boundaryCount + 1; + page <= totalPages; + page++ + ) { + pageNumbers.push(page); + } + } else { + for (let page = currentPage + 1; page <= totalPages; page++) { + pageNumbers.push(page); + } + } + + // Calculate the maximum number of items we will show for the total number + // of pages and options. + const maxItems = Math.min( + // First and last pages + 2 * boundaryCount + + // Pages adjacent to current page + 2 * siblingCount + + // Current page, indicators for elided pages before and after current. + 3, + totalPages, + ); + + // To keep the number of items consistent as the current page changes, + // expand the elided ranges until we reach the maximum. + while (pageNumbers.length < maxItems) { + if (elideAfterIndex !== null) { + // Expand ahead of current page if possible, starting with numbers closest + // to current page. + pageNumbers.splice(elideAfterIndex, 0, elideAfterValue); + ++elideAfterIndex; + ++elideAfterValue; + } else if (elideBeforeIndex !== null) { + // Otherwise expand behind, starting with numbers closest to current page. + pageNumbers.splice(elideBeforeIndex + 1, 0, elideBeforeValue); + --elideBeforeValue; + } else { + break; + } + } - // Construct a numerically-sorted array with `null` entries inserted - // between non-sequential entries - [...pageNumbers] - .sort((a, b) => a - b) - .forEach((page, idx, arr) => { - if (idx > 0 && page - arr[idx - 1] > 1) { - // Two page entries are non-sequential. Push a `null` value between - // them to indicate the gap, which will later be represented as an - // ellipsis - pageOptions.push(null); - } - pageOptions.push(page); - }); - return pageOptions; + return pageNumbers; } diff --git a/src/util/test/pagination-test.js b/src/util/test/pagination-test.js index 0f4442f3..a27f8d36 100644 --- a/src/util/test/pagination-test.js +++ b/src/util/test/pagination-test.js @@ -1,22 +1,115 @@ -import { pageNumberOptions } from '../pagination'; - -describe('sidebar/util/pagination', () => { - describe('pageNumberOptions', () => { - [ - { args: [1, 10, 5], expected: [1, 2, 3, 4, null, 10] }, - { args: [3, 10, 5], expected: [1, 2, 3, 4, null, 10] }, - { args: [6, 10, 5], expected: [1, null, 5, 6, 7, null, 10] }, - { args: [9, 10, 5], expected: [1, null, 7, 8, 9, 10] }, - { args: [2, 3, 5], expected: [1, 2, 3] }, - { args: [3, 10, 7], expected: [1, 2, 3, 4, 5, 6, null, 10] }, - { args: [1, 1, 5], expected: [] }, - ].forEach(testCase => { - it('should produce expected available page numbers', () => { - assert.deepEqual( - pageNumberOptions(...testCase.args), - testCase.expected, - ); +import { paginationItems } from '../pagination'; + +describe('paginationItems', () => { + [ + // Only one page + { + current: 1, + total: 1, + expected: [], + }, + + // No pages elided. + { + current: 1, + total: 3, + expected: [1, 2, 3], + }, + + // Pages elided after current. + { + current: 1, + total: 4, + expected: [1, 2, null, 4], + }, + + // Pages elided before current. + { + current: 4, + total: 6, + expected: [1, null, 3, 4, 5, 6], + }, + + // Pages elided before and after current. + { + current: 4, + total: 7, + expected: [1, null, 3, 4, 5, null, 7], + }, + + // Custom boundary count + { + current: 6, + total: 10, + boundaryCount: 2, + expected: [1, 2, null, 5, 6, 7, null, 9, 10], + }, + + // Custom sibling count + { + current: 6, + total: 10, + siblingCount: 2, + expected: [1, null, 4, 5, 6, 7, 8, null, 10], + }, + ].forEach(({ current, total, boundaryCount, siblingCount, expected }) => { + it('should produce expected items', () => { + const items = paginationItems(current, total, { + boundaryCount, + siblingCount, }); + assert.deepEqual(items, expected); }); }); + + function* configurations() { + for (let boundaryCount = 1; boundaryCount <= 3; boundaryCount++) { + for (let siblingCount = 0; siblingCount <= 2; siblingCount++) { + for (let total = 1; total <= 10; total++) { + yield { total, boundaryCount, siblingCount }; + } + } + } + } + + function isSorted(list) { + return list.every((item, idx) => idx === 0 || item >= list[idx - 1]); + } + + it('should produce expected items for generated configurations', () => { + for (const { total, boundaryCount, siblingCount } of configurations()) { + let expectedItems = null; + for (let current = 1; current <= total; current++) { + const items = paginationItems(current, total, { + boundaryCount, + siblingCount, + }); + + if (total === 1) { + assert.deepEqual(items, []); + continue; + } + + // Total number of items should always be the same for a given total + // page count and configuration. + if (expectedItems === null) { + expectedItems = items.length; + } else { + assert.equal(items.length, expectedItems); + } + + // Check that first, current and last pages are present. + assert.include(items, 1); + assert.include(items, current); + assert.include(items, total); + + // There should be at most two elided page indicators + assert.isAtMost(items.filter(it => it === null).length, 2); + + // Page numbers should always be in sorted order. + const numbers = items.filter(it => typeof it === 'number'); + assert.isTrue(isSorted(numbers), `${numbers.join(', ')} is not sorted`); + } + } + }); });