Persist xDS list pagination in the URL so it survives navigation - #1348
Draft
minwoox wants to merge 1 commit into
Draft
Persist xDS list pagination in the URL so it survives navigation#1348minwoox wants to merge 1 commit into
minwoox wants to merge 1 commit into
Conversation
Motivation: The xDS group list (and the per-group resource and history lists) kept their pagination only in TanStack Table's in-memory state. Modifications: - Add a reusable useUrlPagination hook (webapp/src/dogma/common/components/table/useUrlPagination.ts) that mirrors pageIndex/pageSize into the URL query (`page`, 1-indexed, and `pageSize`) with shallow routing, omits both params while at their defaults, and reads them back through a router.isReady-gated effect so the state is restored on mount and on Back/Forward. The valid page sizes are held in a ref so an inline `pageSizes` array cannot turn the sync effect into a render loop. Result: - xDS list pagination now survives navigation.
📝 WalkthroughWalkthroughThe PR adds shared URL-backed pagination hooks, applies controlled pagination to the shared table and XDS views, defines common XDS page sizes, and adds tests for URL synchronization, restoration, filtering, and page-index clamping. ChangesURL-backed pagination
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant XDSView
participant useUrlPagination
participant NextRouter
participant DataTable
XDSView->>useUrlPagination: provide page sizes
useUrlPagination->>NextRouter: read page and pageSize
useUrlPagination->>DataTable: return controlled pagination
DataTable->>useUrlPagination: report pagination changes
useUrlPagination->>NextRouter: shallow-update query parameters
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
webapp/src/dogma/common/components/table/DataTableClientPagination.tsx (1)
15-51: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAdd
useClampPageIndexto prevent a stranded empty page after filtering.
autoResetPageIndexisfalseand pagination is now controlled throughuseUrlPagination. When a column filter narrows the row set,pageCountcan drop below the currentpageIndex. Nothing in this file corrects that, so the table can render an empty page.
useUrlPagination.ts's own documentation states thatautoResetPageIndex: falsemust be paired withuseClampPageIndex.GroupList.tsx,ResourceHistory.tsx, andResourceList.tsxall call it. This file does not.🐛 Proposed fix
-import { useUrlPagination } from 'dogma/common/components/table/useUrlPagination'; +import { useClampPageIndex, useUrlPagination } from 'dogma/common/components/table/useUrlPagination'; import { useState } from 'react';state: { sorting, columnFilters, pagination, }, }); + // Auto-reset is off, so keep the page index within bounds when a filter narrows the list to fewer pages. + useClampPageIndex(table); if (!router.isReady) { return null; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@webapp/src/dogma/common/components/table/DataTableClientPagination.tsx` around lines 15 - 51, Add and invoke useClampPageIndex in DataTableClientPagination alongside the controlled pagination returned by useUrlPagination, passing the table’s filtered row/page-count context as required by the hook. Ensure it runs with the existing table instance and preserves the current pagination state while clamping pageIndex after column filtering.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@webapp/src/dogma/common/components/table/DataTableClientPagination.tsx`:
- Around line 15-51: Add and invoke useClampPageIndex in
DataTableClientPagination alongside the controlled pagination returned by
useUrlPagination, passing the table’s filtered row/page-count context as
required by the hook. Ensure it runs with the existing table instance and
preserves the current pagination state while clamping pageIndex after column
filtering.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: d9fc06ec-2077-4805-9549-46b4c80bf5b5
📒 Files selected for processing (10)
webapp/src/dogma/common/components/table/DataTableClientPagination.tsxwebapp/src/dogma/common/components/table/useUrlPagination.tswebapp/src/dogma/features/xds/GroupList.tsxwebapp/src/dogma/features/xds/ResourceHistory.tsxwebapp/src/dogma/features/xds/ResourceList.tsxwebapp/src/dogma/features/xds/XdsTypes.tswebapp/tests/dogma/common/components/table/useUrlPagination.test.tsxwebapp/tests/dogma/features/xds/GroupList.test.tsxwebapp/tests/dogma/features/xds/ResourceHistory.test.tsxwebapp/tests/dogma/features/xds/ResourceList.test.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation:
The xDS group list (and the per-group resource and history lists) kept their pagination only in TanStack Table's in-memory state.
Modifications:
page, 1-indexed, andpageSize) with shallow routing, omits both params while at their defaults, and reads them back through a router.isReady-gated effect so the state is restored on mount and on Back/Forward. The valid page sizes are held in a ref so an inlinepageSizesarray cannot turn the sync effect into a render loop.Result: