fix(web): clamp total pages count to minimum of 1 - #544
Conversation
) Clamps pageCount to a minimum of 1 in use-pagination hook, StreamsTable, and HistoryTable so empty streams/histories render 'Page 1 of 1' instead of 'Page 1 of 0'. Fixes Fundable-Protocol#422
|
@SMOGZ6783 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
📝 WalkthroughWalkthroughPagination now clamps page counts and sanitizes range inputs, while token balance fetching handles missing accounts and related hooks receive client-module and timeout-ref updates. ChangesPagination normalization
Token balance hook handling
Balance validation client lifecycle
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Possibly related PRs
🚥 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.
Actionable comments posted: 3
🤖 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.
Inline comments:
In `@apps/web/src/components/modules/payment-stream/StreamsTable.tsx`:
- Line 52: Update the pagination rendering in StreamsTable so the page indicator
renders for empty streams when pageCount is 1, showing “Page 1 of 1”. Move the
indicator outside the totalCount guard or relax that guard, while keeping
optional pagination controls hidden when totalCount is zero.
In `@apps/web/src/hooks/use-pagination.ts`:
- Around line 14-19: Update clampPageCount to treat every non-finite pageCount,
including positive Infinity, as invalid and return the minimum page count of 1;
preserve the existing ceiling behavior for finite positive values.
- Line 27: Update the safeSiblingCount normalization in the pagination hook to
replace non-finite siblingCount values with a finite fallback before applying
Math.floor, preserving the existing default for nullish values and ensuring
windowSize, left, and right receive valid numeric inputs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b5317df1-d280-4685-9877-cb6cb4cee2b9
📒 Files selected for processing (6)
apps/web/src/components/modules/history/HistoryTable.tsxapps/web/src/components/modules/payment-stream/StreamsTable.tsxapps/web/src/hooks/use-balance-validation.tsapps/web/src/hooks/use-pagination.test.tsapps/web/src/hooks/use-pagination.tsapps/web/src/hooks/use-token-balance.ts
| const router = useRouter(); | ||
| const searchParams = useSearchParams(); | ||
| const pageCount = Math.ceil(totalCount / limit); | ||
| const pageCount = clampPageCount(Math.ceil(totalCount / limit)); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Render the empty-stream pagination state.
Although pageCount is clamped to 1, the pagination block at Line 201 is still skipped when totalCount === 0. Empty streams therefore show no “Page 1 of 1” indicator. Render the page indicator outside that guard, or relax the guard while keeping optional controls hidden.
🤖 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 `@apps/web/src/components/modules/payment-stream/StreamsTable.tsx` at line 52,
Update the pagination rendering in StreamsTable so the page indicator renders
for empty streams when pageCount is 1, showing “Page 1 of 1”. Move the indicator
outside the totalCount guard or relax that guard, while keeping optional
pagination controls hidden when totalCount is zero.
| export const clampPageCount = (pageCount: number): number => { | ||
| if (!pageCount || Number.isNaN(pageCount) || pageCount < 1) { | ||
| return 1; | ||
| } | ||
| return Math.ceil(pageCount); | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reject positive Infinity in clampPageCount.
clampPageCount(Infinity) currently returns Infinity, allowing invalid table calculations such as Math.ceil(totalCount / 0) to produce an unusable page count and an Infinity page item. Treat all non-finite values as the minimum page count.
Proposed fix
export const clampPageCount = (pageCount: number): number => {
- if (!pageCount || Number.isNaN(pageCount) || pageCount < 1) {
+ if (!Number.isFinite(pageCount) || pageCount < 1) {
return 1;
}
return Math.ceil(pageCount);
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const clampPageCount = (pageCount: number): number => { | |
| if (!pageCount || Number.isNaN(pageCount) || pageCount < 1) { | |
| return 1; | |
| } | |
| return Math.ceil(pageCount); | |
| }; | |
| export const clampPageCount = (pageCount: number): number => { | |
| if (!Number.isFinite(pageCount) || pageCount < 1) { | |
| return 1; | |
| } | |
| return Math.ceil(pageCount); | |
| }; |
🤖 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 `@apps/web/src/hooks/use-pagination.ts` around lines 14 - 19, Update
clampPageCount to treat every non-finite pageCount, including positive Infinity,
as invalid and return the minimum page count of 1; preserve the existing ceiling
behavior for finite positive values.
| const safeCurrentPage = Math.min(Math.max(1, currentPage), safePageCount); | ||
| const windowSize = siblingCount * 2 + 1; | ||
| const safePageCount = clampPageCount(pageCount); | ||
| const safeSiblingCount = Math.max(0, Math.floor(siblingCount ?? 2)); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Normalize invalid sibling counts before arithmetic.
When siblingCount is NaN, safeSiblingCount, windowSize, left, and right all become NaN; the returned range then collapses to only the first and last pages. Use a finite fallback before flooring.
Proposed fix
- const safeSiblingCount = Math.max(0, Math.floor(siblingCount ?? 2));
+ const normalizedSiblingCount = siblingCount ?? 2;
+ const safeSiblingCount = Number.isFinite(normalizedSiblingCount)
+ ? Math.max(0, Math.floor(normalizedSiblingCount))
+ : 2;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const safeSiblingCount = Math.max(0, Math.floor(siblingCount ?? 2)); | |
| const normalizedSiblingCount = siblingCount ?? 2; | |
| const safeSiblingCount = Number.isFinite(normalizedSiblingCount) | |
| ? Math.max(0, Math.floor(normalizedSiblingCount)) | |
| : 2; |
🤖 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 `@apps/web/src/hooks/use-pagination.ts` at line 27, Update the safeSiblingCount
normalization in the pagination hook to replace non-finite siblingCount values
with a finite fallback before applying Math.floor, preserving the existing
default for nullish values and ensuring windowSize, left, and right receive
valid numeric inputs.
|
dont forget to offramp using https://stellar.fundable.finance/offramp its fast, free and p2p rates |
2 similar comments
|
dont forget to offramp using https://stellar.fundable.finance/offramp its fast, free and p2p rates |
|
dont forget to offramp using https://stellar.fundable.finance/offramp its fast, free and p2p rates |
2 participants
Summary
Fixes issue #422 where tables rendered 'Page 1 of 0' when data streams or transaction history were empty.
Changes Made
Fixes #422
Summary by CodeRabbit
Bug Fixes
Reliability