[TASK-15655] Feat: Summary separator rendering#1356
[TASK-15655] Feat: Summary separator rendering#1356Zishan-7 merged 3 commits intopeanut-wallet-devfrom
Conversation
Zishan-7
commented
Oct 23, 2025
Zishan-7
commented
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. WalkthroughAdds group-aware row rendering to TransactionDetailsReceipt: introduces row groups, computes last visible item per group, centralizes bottom-border visibility via shouldHideGroupBorder, applies group-aware border logic across many rows, and reorders transaction detail row keys in the utils file. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/TransactionDetails/TransactionDetailsReceipt.tsx (2)
478-481: Prevent double separators in txnDetails by hiding “Token amount” border“Token amount” isn’t part of txnDetails group, so it still renders its own separator. When followed by tokenAndNetwork/txId, this creates two separators within the section. Hide the border on “Token amount” to let the group control the single separator.
- {!isStableCoin(transaction.tokenSymbol ?? 'USDC') && ( - <PaymentInfoRow label="Token amount" value={transaction.amount} /> - )} + {!isStableCoin(transaction.tokenSymbol ?? 'USDC') && ( + <PaymentInfoRow label="Token amount" value={transaction.amount} hideBottomBorder /> + )}Also applies to: 524-525, 551-552
116-183: rowVisibilityConfig deps: add isPublic (used by mantecaDepositInfo)isPublic influences visibility but isn’t in the deps array. Add it so rows re-evaluate if the receipt toggles between public/private.
- }, [transaction, isPendingBankRequest]) + }, [transaction, isPendingBankRequest, isPublic])
🧹 Nitpick comments (2)
src/components/TransactionDetails/TransactionDetailsReceipt.tsx (1)
201-206: Group-aware border logic is correct; tighten typing and depsLogic matches the intended behavior. Minor hardening:
- Mark rowGroups as const to lock keys at the type level.
- Include rowGroups in lastVisibleInGroups deps (future-proof).
- const rowGroups = useMemo( - () => ({ - dateRows: ['createdAt', 'cancelled', 'claimed', 'completed'] as TransactionDetailsRowKey[], - txnDetails: ['tokenAndNetwork', 'txId'] as TransactionDetailsRowKey[], - fees: ['networkFee', 'peanutFee'] as TransactionDetailsRowKey[], - }), - [] - ) + const rowGroups = useMemo( + () => + ({ + dateRows: ['createdAt', 'cancelled', 'claimed', 'completed'], + txnDetails: ['tokenAndNetwork', 'txId'], + fees: ['networkFee', 'peanutFee'], + } as const), + [] + ) @@ - const lastVisibleInGroups = useMemo( + const lastVisibleInGroups = useMemo( () => ({ dateRows: getLastVisibleInGroup(rowGroups.dateRows), txnDetails: getLastVisibleInGroup(rowGroups.txnDetails), fees: getLastVisibleInGroup(rowGroups.fees), }), - [rowVisibilityConfig] + [rowVisibilityConfig, rowGroups] )Also applies to: 207-216, 217-225, 227-239
src/components/TransactionDetails/transaction-details.utils.ts (1)
28-33: Order verified; apply compile-time guard to prevent driftOrdering is correct and matches component render order. All keys are present in the union type with no duplicates or gaps. Contiguity checks pass:
- Date rows (createdAt, cancelled, claimed, completed) at indices 0–3
- Transaction details (tokenAndNetwork, txId) at indices 5–6
- Fees (networkFee, peanutFee) at indices 15–16
Apply the compile-time guard to prevent keys from drifting:
-export const transactionDetailsRowKeys: TransactionDetailsRowKey[] = [ +export const transactionDetailsRowKeys = [ 'createdAt', 'cancelled', 'claimed', 'completed', 'to', 'tokenAndNetwork', 'txId', 'fee', 'mantecaDepositInfo', 'exchangeRate', 'bankAccountDetails', 'transferId', 'depositInstructions', 'points', 'comment', 'networkFee', 'peanutFee', 'attachment', -] +] as const satisfies ReadonlyArray<TransactionDetailsRowKey>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/TransactionDetails/TransactionDetailsReceipt.tsx(9 hunks)src/components/TransactionDetails/transaction-details.utils.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/components/TransactionDetails/TransactionDetailsReceipt.tsx (2)
src/components/TransactionDetails/transaction-details.utils.ts (1)
TransactionDetailsRowKey(2-20)src/components/Payment/PaymentInfoRow.tsx (1)
PaymentInfoRow(18-87)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Deploy-Preview
🔇 Additional comments (4)
src/components/TransactionDetails/TransactionDetailsReceipt.tsx (4)
424-454: Date rows: migration to group-based borders looks goodCreated/Cancelled/Claimed/Completed correctly use shouldHideGroupBorder with dateRows.
Also applies to: 432-439, 440-447, 448-453
590-598: Exchange rate block: consistent separator handlingAlways-hiding the first row’s border to pair with the second row’s conditional border is consistent. LGTM.
703-704: Deposit instructions: explicit borders are consistentExplicit true/false flags look intentional for the collapsible block. LGTM.
Also applies to: 744-745, 853-854, 877-878
908-915: Fees group borders: correct usage; keep keys adjacent in order arrayUsing shouldHideGroupBorder for networkFee/peanutFee is correct. Ensure these keys remain adjacent in transactionDetailsRowKeys to avoid unexpected separators.
Validated by the array-order check suggested in transaction-details.utils.ts.
Also applies to: 917-922
Hugo0
left a comment
Hugo0
left a comment
There was a problem hiding this comment.
lgtm but mmight have merge conflicts with perk-rework branch. please DO NOT merge b4 prod release
| const isLastInGroup = rowKey === lastVisibleInGroups[groupName] | ||
| const isGlobalLast = shouldHideBorder(rowKey) | ||
|
|
||
| // if it's the last in its group, show border UNLESS it's also the global last |
…mary-separator-rendering
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/TransactionDetails/TransactionDetailsReceipt.tsx (1)
134-202: Missing dependency in rowVisibilityConfig useMemo.isPublic influences mantecaDepositInfo but is not in the deps array; the memo can go stale if isPublic changes.
-}, [transaction, isPendingBankRequest]) +}, [transaction, isPendingBankRequest, isPublic])
🧹 Nitpick comments (3)
src/components/TransactionDetails/transaction-details.utils.ts (1)
33-33: Consider a future “deposit” group for clarity.mantecaDepositInfo and depositInstructions conceptually belong together. A dedicated group would make border rules simpler if more deposit fields are added.
src/components/TransactionDetails/TransactionDetailsReceipt.tsx (2)
236-244: Memo deps: tie to visibleRows.If last-in-group uses visibleRows, memoize against visibleRows (not just rowVisibilityConfig) to avoid stale borders.
-const lastVisibleInGroups = useMemo( - () => ({ - dateRows: getLastVisibleInGroup(rowGroups.dateRows), - txnDetails: getLastVisibleInGroup(rowGroups.txnDetails), - fees: getLastVisibleInGroup(rowGroups.fees), - }), - [rowVisibilityConfig] -) +const lastVisibleInGroups = useMemo( + () => ({ + dateRows: getLastVisibleInGroup(rowGroups.dateRows), + txnDetails: getLastVisibleInGroup(rowGroups.txnDetails), + fees: getLastVisibleInGroup(rowGroups.fees), + }), + [visibleRows, rowGroups] +)
591-616: Group-aware border usage: good; add one tweak for “Token amount”.The switch to shouldHideGroupBorder for dateRows/txnDetails/fees looks right. To avoid a stray dashed separator between “Token amount” and “Token and network”, hide the border under “Token amount” when the latter is visible.
- {!isStableCoin(transaction.tokenSymbol ?? 'USDC') && ( - <PaymentInfoRow label="Token amount" value={transaction.amount} /> - )} + {!isStableCoin(transaction.tokenSymbol ?? 'USDC') && ( + <PaymentInfoRow + label="Token amount" + value={transaction.amount} + hideBottomBorder={rowVisibilityConfig.tokenAndNetwork} + /> + )}Also applies to: 699-701, 726-727, 1084-1090, 1092-1097
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/TransactionDetails/TransactionDetailsReceipt.tsx(9 hunks)src/components/TransactionDetails/transaction-details.utils.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/components/TransactionDetails/TransactionDetailsReceipt.tsx (2)
src/components/TransactionDetails/transaction-details.utils.ts (1)
TransactionDetailsRowKey(2-21)src/components/Payment/PaymentInfoRow.tsx (1)
PaymentInfoRow(18-87)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Deploy-Preview
🔇 Additional comments (2)
src/components/TransactionDetails/transaction-details.utils.ts (1)
29-31: Reorder looks aligned; double-check grouping side effects.Placing 'to', 'tokenAndNetwork', and 'txId' after 'completed' aligns with the component. 'mantecaDepositInfo' placement after 'fee' and 'peanutFee' after 'networkFee' keeps the fees contiguous.
To ensure grouping/border behavior stays correct, please confirm:
- The date rows remain contiguous (especially if you keep 'closed' at the end, which I don’t recommend).
- The fees group is contiguous (networkFee → peanutFee) in the array.
Also applies to: 33-33, 41-41
src/components/TransactionDetails/TransactionDetailsReceipt.tsx (1)
619-629: Closed row border: consistent with change above.If you remove 'closed' from dateRows (recommended), leaving shouldHideBorder('closed') here is correct. If you instead move 'closed' next to 'completed' in the keys array, consider using shouldHideGroupBorder('closed','dateRows') for consistency.
2 participants
