From 945544153aef7f236b0d46a2b670d0643ec140a6 Mon Sep 17 00:00:00 2001 From: Kalle <38327916+Sendouc@users.noreply.github.com> Date: Sun, 7 Jun 2026 21:34:21 +0300 Subject: [PATCH] Fix lonely page number display at pagination edges MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On the first page the mobile window was currentPage ± 1, i.e. {0, 1, 2}. The 0 clamped away leaving only page 2 before a jump straight to the last page, so page 1 of 8 rendered as "1 2 ... 8" with no bridging number (and the last page had the same issue mirrored). Nudge the window inward by one when the current page is the very first or last page so these render "1 2 3 ... 8" / "1 ... 6 7 8" instead. --- app/components/Pagination.test.ts | 108 ++++++++++++++++++++++++++++++ app/components/Pagination.tsx | 44 +++++++++--- 2 files changed, 141 insertions(+), 11 deletions(-) create mode 100644 app/components/Pagination.test.ts diff --git a/app/components/Pagination.test.ts b/app/components/Pagination.test.ts new file mode 100644 index 000000000..0fb720d49 --- /dev/null +++ b/app/components/Pagination.test.ts @@ -0,0 +1,108 @@ +import { describe, expect, it } from "vitest"; +import { getPageNumbers } from "./Pagination"; + +/** What is rendered on a narrow (mobile) viewport — `desktopOnly` items are hidden. */ +function mobileView(currentPage: number, pagesCount: number) { + return getPageNumbers(currentPage, pagesCount) + .filter((item) => !item.desktopOnly) + .map((item) => item.value); +} + +/** What is rendered on a wide (desktop) viewport — `mobileOnly` items are hidden. */ +function desktopView(currentPage: number, pagesCount: number) { + return getPageNumbers(currentPage, pagesCount) + .filter((item) => !item.mobileOnly) + .map((item) => item.value); +} + +describe("getPageNumbers", () => { + it("shows every page without ellipsis when there are 5 or fewer", () => { + expect(mobileView(2, 5)).toEqual([1, 2, 3, 4, 5]); + expect(desktopView(2, 5)).toEqual([1, 2, 3, 4, 5]); + }); + + it("shows every page on desktop when there are 9 or fewer", () => { + expect(desktopView(2, 9)).toEqual([1, 2, 3, 4, 5, 6, 7, 8, 9]); + expect(desktopView(5, 9)).toEqual([1, 2, 3, 4, 5, 6, 7, 8, 9]); + }); + + it("inserts a mobile ellipsis instead of silently hiding middle pages", () => { + // Regression: page=2 of 9 used to render "1 2 8 9" with no ellipsis + expect(mobileView(2, 9)).toEqual([1, 2, 3, "...", 9]); + }); + + it("shows a bridging number on the first page instead of a lonely jump", () => { + // Regression: page=1 of 8 used to render "1 2 ... 8" with nothing in between + expect(mobileView(1, 8)).toEqual([1, 2, 3, "...", 8]); + expect(mobileView(1, 20)).toEqual([1, 2, 3, "...", 20]); + expect(desktopView(1, 20)).toEqual([1, 2, 3, 4, "...", 20]); + }); + + it("shows a bridging number on the last page instead of a lonely jump", () => { + expect(mobileView(8, 8)).toEqual([1, "...", 6, 7, 8]); + expect(mobileView(20, 20)).toEqual([1, "...", 18, 19, 20]); + expect(desktopView(20, 20)).toEqual([1, "...", 17, 18, 19, 20]); + }); + + it("keeps the current page and its neighbours visible on mobile", () => { + const view = mobileView(5, 9); + expect(view).toContain(4); + expect(view).toContain(5); + expect(view).toContain(6); + // no confusing jump straight from 1 to the middle + expect(view).toEqual([1, "...", 4, 5, 6, "...", 9]); + }); + + it("always keeps the first and last page", () => { + for (const currentPage of [1, 7, 20]) { + const view = mobileView(currentPage, 20); + expect(view[0]).toBe(1); + expect(view.at(-1)).toBe(20); + } + }); + + it("windows around the current page with ellipses on both sides for many pages", () => { + expect(mobileView(10, 20)).toEqual([1, "...", 9, 10, 11, "...", 20]); + expect(desktopView(10, 20)).toEqual([ + 1, + "...", + 8, + 9, + 10, + 11, + 12, + "...", + 20, + ]); + }); + + it("omits the leading ellipsis when the current page is near the start", () => { + expect(desktopView(2, 20)).toEqual([1, 2, 3, 4, "...", 20]); + expect(mobileView(2, 20)).toEqual([1, 2, 3, "...", 20]); + }); + + it("omits the trailing ellipsis when the current page is near the end", () => { + expect(desktopView(19, 20)).toEqual([1, "...", 17, 18, 19, 20]); + expect(mobileView(19, 20)).toEqual([1, "...", 18, 19, 20]); + }); + + it("never produces duplicate page numbers", () => { + for (let pagesCount = 1; pagesCount <= 25; pagesCount++) { + for (let currentPage = 1; currentPage <= pagesCount; currentPage++) { + const values = getPageNumbers(currentPage, pagesCount) + .map((item) => item.value) + .filter((value): value is number => value !== "..."); + expect(new Set(values).size).toBe(values.length); + } + } + }); + + it("always includes the current page in both views", () => { + for (let pagesCount = 1; pagesCount <= 25; pagesCount++) { + for (let currentPage = 1; currentPage <= pagesCount; currentPage++) { + expect(mobileView(currentPage, pagesCount)).toContain(currentPage); + expect(desktopView(currentPage, pagesCount)).toContain(currentPage); + } + } + }); +}); diff --git a/app/components/Pagination.tsx b/app/components/Pagination.tsx index e33c910f9..c075d9f86 100644 --- a/app/components/Pagination.tsx +++ b/app/components/Pagination.tsx @@ -187,22 +187,24 @@ type PageItem = { mobileOnly?: boolean; }; -function getPageNumbers(currentPage: number, pagesCount: number): PageItem[] { +export function getPageNumbers( + currentPage: number, + pagesCount: number, +): PageItem[] { if (pagesCount <= 5) { return Array.from({ length: pagesCount }, (_, i) => ({ value: i + 1 })); } - if (pagesCount <= 9) { - return Array.from({ length: pagesCount }, (_, i) => ({ - value: i + 1, - desktopOnly: i >= 2 && i < pagesCount - 2, - })); - } + const showAllOnDesktop = pagesCount <= 9; - const mobileStart = Math.max(2, currentPage - 1); - const mobileEnd = Math.min(pagesCount - 1, currentPage + 1); - const desktopStart = Math.max(2, currentPage - 2); - const desktopEnd = Math.min(pagesCount - 1, currentPage + 2); + const { start: mobileStart, end: mobileEnd } = innerPageWindow( + currentPage, + pagesCount, + 1, + ); + const { start: desktopStart, end: desktopEnd } = showAllOnDesktop + ? { start: 2, end: pagesCount - 1 } + : innerPageWindow(currentPage, pagesCount, 2); const isMobileVisible = (page: number) => page >= mobileStart && page <= mobileEnd; @@ -245,3 +247,23 @@ function getPageNumbers(currentPage: number, pagesCount: number): PageItem[] { return pages; } + +/** + * Inclusive range of inner page numbers (excluding the always-shown first and + * last page) to render around the current page. The window is nudged inward by + * one when the current page is the very first or last page, so the edge view + * shows a bridging number instead of a lonely jump like "1 2 … 8". + */ +function innerPageWindow( + currentPage: number, + pagesCount: number, + radius: number, +): { start: number; end: number } { + const startNudge = currentPage === pagesCount ? 1 : 0; + const endNudge = currentPage === 1 ? 1 : 0; + + return { + start: Math.max(2, currentPage - radius - startNudge), + end: Math.min(pagesCount - 1, currentPage + radius + endNudge), + }; +}