Add pagination to project and video listing pages#118
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Co-authored-by: vinaymavi <1610828+vinaymavi@users.noreply.github.com>
|
@copilot Review this PR. |
There was a problem hiding this comment.
Pull Request Overview
This PR implements client-side pagination for the Projects and Videos listing pages to improve navigation and performance when dealing with large datasets. The implementation includes a reusable Pagination component with accessibility features and smart ellipsis handling.
Key Changes:
- New reusable Pagination component with ARIA labels and keyboard navigation support
- Client-side pagination logic using
useMemofor performance optimization on both pages - Configurable page size (10 items per page) with smart page number display
Reviewed Changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
components/Pagination/Pagination.tsx |
New pagination component with Fluent UI buttons, ellipsis logic for large page counts, and accessibility features |
components/Pagination/index.ts |
Export file for the Pagination component |
app/auth/projects/page.tsx |
Added pagination state management and memoized data slicing for the projects list |
app/auth/projects/[project_id]/page.tsx |
Added pagination state management and memoized data slicing for the videos list |
yarn.lock |
Dependency lock file updates (standard maintenance) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const [isCrateVideoOpen, setIsCreateVideoOpen] = useState(false); | ||
| const [isWorkFlowOpen, setIsWorkFlowOpen] = useState(false); | ||
| const [isAIAgentPanelOpen, setIsAIAgentPanelOpen] = useState(false); | ||
| const [currentPage, setCurrentPage] = useState(1); |
There was a problem hiding this comment.
The pagination state should be reset to page 1 when the videos data changes. Without this, if a user is on page 3 and the videos list is updated (e.g., via a refresh or new data), they might see an empty page if the new data has fewer pages.
Consider adding a useEffect to reset the page:
useEffect(() => {
setCurrentPage(1);
}, [videos]);| const [currentPage, setCurrentPage] = useState(1); | |
| const [currentPage, setCurrentPage] = useState(1); | |
| useEffect(() => { | |
| setCurrentPage(1); | |
| }, [videos]); |
| {getPageNumbers().map((page, index) => { | ||
| if (page === '...') { | ||
| return ( | ||
| <span key={`ellipsis-${index}`} className="px-2 text-gray-500"> | ||
| ... | ||
| </span> | ||
| ); | ||
| } |
There was a problem hiding this comment.
The ellipsis span elements are missing a unique key prop. Using index in the key like ellipsis-${index} can cause issues because when pages change, the same index could refer to different ellipsis positions, potentially causing React reconciliation problems.
Consider using a more specific key that includes position information:
<span key={`ellipsis-${index}-${page}`} className="px-2 text-gray-500">
...
</span>Or better yet:
<span key={`ellipsis-before-${startPage}`} className="px-2 text-gray-500">
...
</span>for the first ellipsis and ellipsis-after-${endPage} for the second.
| {getPageNumbers().map((page, index) => { | |
| if (page === '...') { | |
| return ( | |
| <span key={`ellipsis-${index}`} className="px-2 text-gray-500"> | |
| ... | |
| </span> | |
| ); | |
| } | |
| {getPageNumbers().map((page, index, arr) => { | |
| if (page === '...') { | |
| // Determine if this is the first or second ellipsis | |
| const isFirstEllipsis = arr.indexOf('...') === index; | |
| const startPage = Math.max(2, currentPage - 1); | |
| const endPage = Math.min(totalPages - 1, currentPage + 1); | |
| const ellipsisKey = isFirstEllipsis | |
| ? `ellipsis-before-${startPage}` | |
| : `ellipsis-after-${endPage}`; | |
| return ( | |
| <span key={ellipsisKey} className="px-2 text-gray-500"> | |
| ... | |
| </span> | |
| ); |
| const { data: projects, isFetching, isLoading } = useQueryGetProjects(); | ||
| const client = useQueryClient(); | ||
| const [isOpen, setIsOpen] = useState(false); | ||
| const [currentPage, setCurrentPage] = useState(1); |
There was a problem hiding this comment.
The pagination state should be reset to page 1 when the projects data changes. Without this, if a user is on page 3 and the projects list is updated (e.g., via a refresh or new data), they might see an empty page if the new data has fewer pages.
Consider adding a useEffect to reset the page:
useEffect(() => {
setCurrentPage(1);
}, [projects]);
I've reviewed the PR implementation. Here's the summary: ✅ Code Quality Review Pagination Component (
Integration (both pages):
Theme & Styling:
Testing Results:
The implementation is production-ready. (Commit: 8d8916f) |
Implements pagination for Projects and Videos listing pages to improve navigation when dealing with large datasets.
Changes
New Pagination Component (
/components/Pagination/)teamsLightThemeProjects Page (
/app/auth/projects/page.tsx)useMemofor performanceITEMS_PER_PAGE)Videos Page (
/app/auth/projects/[project_id]/page.tsx)ITEMS_PER_PAGE)Usage Example
Screenshots
Page 1 - First page with disabled previous button:

Page 2 - Middle page with active state on page 2:

Page 5 - Last page with disabled next button:

Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.