Repository navigation
Conversation
…ighbours (CapSoftware#3566) CapPagination only rendered page 1, current page, and current + 1 with an arbitrary trailing ellipsis, never linking to totalPages or allowing users to jump to the last page. In addition, PaginationNext rendered unconditionally, linking back to currentPage when on the last page. Added getPaginationItems in apps/web/lib/pagination-items.ts to compute the first and last page links, neighbouring active pages, and appropriately placed ellipses. Hid Next button on the last page. Added comprehensive unit tests. Closes CapSoftware#3566
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| {items.map((item, index) => | ||
| item === "ellipsis" ? ( | ||
| <PaginationEllipsis key={`ellipsis-${index}`} /> | ||
| ) : ( | ||
| <PaginationItem key={item}> | ||
| <PaginationLink | ||
| className={ | ||
| item === currentPage | ||
| ? "h-10 min-w-10" | ||
| : "h-10 min-w-10 hover:bg-gray-3" |
There was a problem hiding this comment.
CapPagination now renders all five numbered links when totalPages is 5, but the row cannot wrap. On a 320px-wide phone, those links, gaps, and Next exceed the dashboard's padded width and can leave links clipped. The old first-page row had only two numbered links. Add a narrow-screen layout so the extra links stay within view.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/web/app/(org)/dashboard/caps/components/CapPagination.tsx
Line: 38-47
Comment:
**Page links exceed phone width**
`CapPagination` now renders all five numbered links when `totalPages` is 5, but the row cannot wrap. On a 320px-wide phone, those links, gaps, and `Next` exceed the dashboard's padded width and can leave links clipped. The old first-page row had only two numbered links. Add a narrow-screen layout so the extra links stay within view.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| }); | ||
|
|
||
| it("always links first and last page with ellipses for large page counts", () => { | ||
| // Viewing start |
There was a problem hiding this comment.
The new Viewing start, Viewing middle, and Viewing end comments repeat the page positions already shown by the assertions. AGENTS.md explicitly bans comments that merely narrate the code. Remove these comments or split the groups into named tests. This repository requirement must be satisfied before merging.
Context Used: AGENTS.md (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/web/__tests__/unit/cap-pagination.test.ts
Line: 19
Comment:
**Test comments break repository rules**
The new `Viewing start`, `Viewing middle`, and `Viewing end` comments repeat the page positions already shown by the assertions. `AGENTS.md` explicitly bans comments that merely narrate the code. Remove these comments or split the groups into named tests. This repository requirement must be satisfied before merging.
**Context Used:** AGENTS.md ([source](https://github.com/capsoftware/cap/blob/main/AGENTS.md))
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Summary
Fixes #3566
Linear: CAP-956
CapPaginationdid not link tototalPages, displayed trailing ellipses unconditionally whencurrentPage > 2, and renderedPaginationNextunconditionally on the last page.Root Cause
CapPagination.tsxhardcoded page links for1,currentPage, andcurrentPage + 1, followed by{currentPage > 2 && <PaginationEllipsis />}. It never renderedtotalPages, leaving users unable to navigate to the last page directly or view missing intermediate page numbers. Furthermore,PaginationNextalways rendered and linked tocurrentPagewhen on the final page.Changes
getPaginationItems(currentPage, totalPages)inapps/web/lib/pagination-items.tsto compute:totalPages <= 5without ellipses.1andtotalPages) alongside current page neighbours.CapPagination.tsxto map overgetPaginationItemsand render clean, responsive links.PaginationNextwhencurrentPage < totalPages.apps/web/__tests__/unit/cap-pagination.test.tsverifying small counts, start, middle, and end positions, single page boundaries, and bridge insertions.Test plan
bun test apps/web/__tests__/unit/cap-pagination.test.ts(4 pass, 15 assertions)bun test apps/web/__tests__/unit/pagination-params.test.ts(6 pass, 23 assertions)bun test apps/web/__tests__/unit/activity-time-ago.test.ts(3 pass, 13 assertions)bun test apps/web/__tests__/unit/video-thumbnail-duration.test.ts(4 pass, 17 assertions)[1, 2, 3, 4, Next], viewing page 4 shows[Prev, 1, 2, 3, 4](no Next button, last page fully accessible).The comments rule must be satisfied before merging; the small-screen layout also deserves a follow-up fix.
Findings
Fix with agent prompt
Summary
The PR adds
getPaginationItemsand uses it to show first, last, and neighboring pages.Reviews (1) · Last reviewed commit: "fix(web): render complete pagination lin..." · Reviewed by Greptile