-
Notifications
You must be signed in to change notification settings - Fork 0
Add pagination to issues list (25 per page) #768
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Co-authored-by: timothyfroehlich <[email protected]>
Co-authored-by: timothyfroehlich <[email protected]>
Co-authored-by: timothyfroehlich <[email protected]>
Co-authored-by: timothyfroehlich <[email protected]>
Co-authored-by: timothyfroehlich <[email protected]>
|
@copilot merge conflicts |
# Conflicts: # src/app/(app)/issues/page.tsx
Merge conflicts resolved in commit 2f8eb2b. The conflicts were in
Both changes are now integrated. The pagination query includes all the new reporter fields and the IssueRow component was auto-merged successfully with the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds server-side pagination to the issues list page to address performance and UX concerns with 197+ issues in production. The implementation follows a server-first architecture with progressive enhancement.
Changes:
- Server-side pagination logic with parallel queries for count and data using LIMIT/OFFSET
- Client-side pagination UI component with Previous/Next buttons and smart page number display
- Filter integration that resets to page 1 when filters change and preserves filters during pagination navigation
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/components/ui/pagination.tsx |
New shadcn/ui-style base pagination primitives with progressive enhancement using <a> links |
src/components/issues/IssuesPagination.tsx |
Client component implementing smart pagination controls with ellipsis logic and filter-aware URL building |
src/components/issues/IssueFilters.tsx |
Updated to reset to page 1 when any filter changes |
src/app/(app)/issues/page.tsx |
Server component with parallel queries for total count and paginated data, page parameter parsing and validation |
src/app/(app)/issues/pagination.test.ts |
Unit tests covering page parsing, offset calculation, and range display logic |
CHANGELOG.md |
Documents the new pagination feature for users |
Comments suppressed due to low confidence (2)
src/app/(app)/issues/page.tsx:179
- The pagination component is only shown when
issuesList.length > 0, but this means that if a user navigates to page 2 and applies filters that result in no matches, they won't see any pagination controls to navigate back. Consider always rendering the pagination component (or at least a simple message) whentotalCount > 0, even if the current page has no results due to filtering. This would happen if, for example, a user is on page 8 and then applies a filter that only returns 5 results total.
</p>
</div>
</div>
<div className="space-y-6">
{/* Filters */}
<div className="rounded-lg border border-border bg-card p-4 shadow-sm">
src/app/(app)/issues/page.tsx:179
- The pagination feature lacks E2E test coverage. Given that pagination is a critical user journey for navigating through 197+ issues in production, consider adding an E2E test in
e2e/full/that verifies: (1) pagination controls appear when there are multiple pages, (2) clicking Next/Previous navigation works correctly, (3) page numbers are correctly displayed and functional, and (4) filters are preserved when navigating between pages. Follow the patterns established ine2e/full/dashboard.spec.tsusing semantic selectors.
</p>
</div>
</div>
<div className="space-y-6">
{/* Filters */}
<div className="rounded-lg border border-border bg-card p-4 shadow-sm">
| import { describe, it, expect } from "vitest"; | ||
|
|
||
| /** | ||
| * Tests for pagination logic used in issues list | ||
| */ | ||
| describe("Issues Pagination Logic", () => { |
Copilot
AI
Jan 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The pagination test file is placed in src/app/(app)/issues/pagination.test.ts, but according to the repository's testing structure (seen in src/test/unit/), unit tests should be located in the src/test/unit/ directory, not co-located with the application code. Consider moving this test to src/test/unit/issues/pagination.test.ts to maintain consistency with the project's testing conventions.
| <PaginationPrevious | ||
| href="#" | ||
| className="pointer-events-none opacity-50" | ||
| aria-disabled="true" | ||
| /> | ||
| )} | ||
| </PaginationItem> | ||
|
|
||
| {/* Page numbers */} | ||
| {getPageNumbers().map((page, index) => ( | ||
| <PaginationItem | ||
| key={page === "ellipsis" ? `ellipsis-${index}` : `page-${page}`} | ||
| > | ||
| {page === "ellipsis" ? ( | ||
| <PaginationEllipsis /> | ||
| ) : ( | ||
| <PaginationLink | ||
| href={buildPageUrl(page)} | ||
| isActive={page === currentPage} | ||
| > | ||
| {page} | ||
| </PaginationLink> | ||
| )} | ||
| </PaginationItem> | ||
| ))} | ||
|
|
||
| {/* Next button */} | ||
| <PaginationItem> | ||
| {currentPage < totalPages ? ( | ||
| <PaginationNext href={buildPageUrl(currentPage + 1)} /> | ||
| ) : ( | ||
| <PaginationNext | ||
| href="#" | ||
| className="pointer-events-none opacity-50" | ||
| aria-disabled="true" | ||
| /> |
Copilot
AI
Jan 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The disabled Previous/Next buttons use href="#" with pointer-events-none and aria-disabled="true". However, aria-disabled is not a valid attribute for anchor elements (<a>). For proper accessibility, consider using aria-hidden="true" on the disabled links, or better yet, render them as <span> elements styled to look like buttons when disabled, rather than as links with href="#".
Pagination Implementation - Complete ✅
Overview
Implemented pagination on the issues list page to handle large datasets (197+ issues). Users can now navigate through issues 25 at a time with full filter preservation and URL state management.
Key Features Delivered
/issues?page=2)Files Changed (6 files)
src/app/(app)/issues/page.tsx- Server-side pagination logic + reporter fieldssrc/components/issues/IssuesPagination.tsx- Pagination controls (client component)src/components/ui/pagination.tsx- Base UI component (shadcn/ui style)src/components/issues/IssueFilters.tsx- Reset page on filter changesrc/app/(app)/issues/pagination.test.ts- 18 unit tests (all passing)CHANGELOG.md- Feature documentationMerge Resolution
Successfully merged changes from main branch (
8e3481a) which added:reporterNameandreporterEmailcolumns to issues queryinvitedReporterrelationresolveIssueReporter()helper function usage in IssueRowBoth pagination logic and reporter display enhancements now work together seamlessly.
Implementation Highlights
Architecture Alignment
✅ Server Components with direct Drizzle queries
✅ URL-based state (no Redux/Zustand needed)
✅ Progressive enhancement (links work without JS)
✅ Single-tenant design (no org scoping)
✅ Follows project patterns from docs/PATTERNS.md
Ready for merge!
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.