-
Notifications
You must be signed in to change notification settings - Fork 13
[TASK-14738] Fix/layout copy bugs #1223
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
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughMoves withdraw flow reset from unmount to DirectSuccessView completion, adds optional onComplete prop to DirectSuccessView, extends TransactionDetails with fullName and populates it in the transformer, updates TransactionCard to use fullName for avatars, and centers text in VerifiedUserLabel. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests
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 |
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.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/Payment/Views/Status.payment.view.tsx (1)
107-141
: Missing fullName in TransactionDetails built here can break consumers that now expect it.
TransactionDetails.fullName
is newly required; this object doesn’t set it. SetfullName
torecipientName
(already resolved to full/username/address) to avoid runtime gaps.let details: Partial<TransactionDetails> = { id: paymentDetails?.payerTransactionHash, txHash: paymentDetails?.payerTransactionHash, status: 'completed' as StatusPillType, amount: parseFloat(amountValue), createdAt: new Date(paymentDetails?.createdAt ?? chargeDetails.createdAt), completedAt: new Date(), tokenSymbol: chargeDetails.tokenSymbol, direction: 'send', // only showing receipt for send txns initials: getInitialsFromName(recipientName), + fullName: recipientName, extraDataForDrawer: { isLinkTransaction: false, originalType: EHistoryEntryType.DIRECT_SEND, originalUserRole: EHistoryUserRole.SENDER, link: receiptLink, }, userName: user?.username || parsedPaymentData?.recipient?.identifier,
🧹 Nitpick comments (8)
src/components/UserHeader/index.tsx (1)
71-73
: Centering label may misalign/truncate in list rows; prefer responsive centering.In list contexts (e.g., TransactionCard) this label sits inside a truncate container; center text can look off and reduce readability for long names/addresses. Suggest centering only on larger breakpoints.
-<div className={twMerge('text-center text-sm font-semibold md:text-base', className)}> +<div className={twMerge('text-sm font-semibold md:text-base md:text-center', className)}>src/components/Payment/Views/Status.payment.view.tsx (1)
143-159
: Nit: remove imported function from useMemo deps.
getInitialsFromName
is a module import (stable). Keeping it in deps causes unnecessary re-computation noise.- }, [ - chargeDetails, - type, - amountValue, - recipientName, - parsedPaymentData, - message, - user, - getInitialsFromName, - tokenIconUrl, - chainIconUrl, - resolvedChainName, - resolvedTokenSymbol, - paymentDetails, - usdAmount, - ]) + }, [ + chargeDetails, + type, + amountValue, + recipientName, + parsedPaymentData, + message, + user, + tokenIconUrl, + chainIconUrl, + resolvedChainName, + resolvedTokenSymbol, + paymentDetails, + usdAmount, + ])src/app/(mobile-ui)/withdraw/crypto/page.tsx (2)
281-287
: Effect deps include an unused dependency; remove to avoid needless re-subscribes.
resetWithdrawFlow
isn’t referenced in the effect; also per learnings,resetTokenContextProvider
is stable and doesn’t need to be in deps.- }, [resetWithdrawFlow, resetPaymentInitiator]) + }, [resetPaymentInitiator])
354-372
: Edge-case: ensure no stale state if user leaves STATUS without pressing buttons.If the user hard-refreshes or route-replaces from STATUS,
onComplete
won’t run and unmount no longer resets. Consider a guarded fallback (e.g., effect in STATUS that resets on unmount only when view==='STATUS').src/components/TransactionDetails/transactionTransformer.ts (4)
159-167
: Normalize fullName assignment (prefer fullName || username || identifier).Some branches set
fullName
tousername
, skippingaccount.fullName
when available (e.g., Lines 179, 195, 207). Apply a consistent pattern to avoid regressions in avatar names.Example adjustments:
- fullName = entry.recipientAccount?.username ?? '' + fullName = entry.recipientAccount?.fullName ?? entry.recipientAccount?.username ?? entry.recipientAccount?.identifier ?? ''Repeat similarly for sender/recipient branches shown above.
Also applies to: 174-207, 210-244, 274-278
166-167
: Avoid comma operator; split for clarity.The current expression is terse and easy to miss in reviews.
-;((fullName = entry.senderAccount?.fullName ?? ''), (isLinkTx = !entry.senderAccount)) // If the sender is not an user then it's a public link +fullName = entry.senderAccount?.fullName ?? '' +isLinkTx = !entry.senderAccount // If the sender is not a user then it's a public link
437-441
: Fallback LGTM; can be simplified.-if (!fullName || fullName === '') { - fullName = nameForDetails -} +fullName ||= nameForDetails
452-453
: Initials should align with the name used for the avatar.Use
fullName
(fallback tonameForDetails
) to keep initials consistent withTransactionCard
’suserNameForAvatar
.- initials: getInitialsFromName(nameForDetails), + initials: getInitialsFromName(fullName || nameForDetails),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/app/(mobile-ui)/withdraw/crypto/page.tsx
(1 hunks)src/components/Payment/Views/Status.payment.view.tsx
(4 hunks)src/components/TransactionDetails/TransactionCard.tsx
(1 hunks)src/components/TransactionDetails/transactionTransformer.ts
(9 hunks)src/components/UserHeader/index.tsx
(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-08-22T07:28:32.281Z
Learnt from: Zishan-7
PR: peanutprotocol/peanut-ui#1104
File: src/components/Payment/PaymentForm/index.tsx:522-545
Timestamp: 2025-08-22T07:28:32.281Z
Learning: In `src/components/Payment/PaymentForm/index.tsx`, the `handleCompleteDaimoPayment` function is only for updating payment status in the backend after a successful Daimo payment. Payment success/failure is handled by Daimo itself, so try/catch error handling and error display are not needed for backend sync failures - users shouldn't see errors if payment succeeded but database update failed.
Applied to files:
src/components/Payment/Views/Status.payment.view.tsx
src/app/(mobile-ui)/withdraw/crypto/page.tsx
📚 Learning: 2025-05-22T15:38:48.586Z
Learnt from: kushagrasarathe
PR: peanutprotocol/peanut-ui#869
File: src/app/(mobile-ui)/withdraw/page.tsx:82-88
Timestamp: 2025-05-22T15:38:48.586Z
Learning: The country-specific withdrawal route exists at src/app/(mobile-ui)/withdraw/[...country]/page.tsx and renders the AddWithdrawCountriesList component with flow="withdraw".
Applied to files:
src/app/(mobile-ui)/withdraw/crypto/page.tsx
📚 Learning: 2024-10-25T11:33:46.776Z
Learnt from: jjramirezn
PR: peanutprotocol/peanut-ui#484
File: src/components/Cashout/Components/Initial.view.tsx:273-274
Timestamp: 2024-10-25T11:33:46.776Z
Learning: In the `InitialCashoutView` component (`src/components/Cashout/Components/Initial.view.tsx`), linked bank accounts should not generate error states, and the `ValidatedInput` component will clear any error messages if needed. Therefore, it's unnecessary to manually clear the error state when selecting or clearing linked bank accounts.
Applied to files:
src/app/(mobile-ui)/withdraw/crypto/page.tsx
📚 Learning: 2024-10-08T20:13:42.967Z
Learnt from: jjramirezn
PR: peanutprotocol/peanut-ui#413
File: src/context/tokenSelector.context.tsx:118-123
Timestamp: 2024-10-08T20:13:42.967Z
Learning: In the `TokenContextProvider` component within `src/context/tokenSelector.context.tsx`, in the TypeScript React application, when data changes and before calling `fetchAndSetTokenPrice`, it is necessary to reset `selectedTokenData`, `selectedTokenPrice`, `selectedTokenDecimals`, and `inputDenomination` to discard stale data.
Applied to files:
src/app/(mobile-ui)/withdraw/crypto/page.tsx
📚 Learning: 2025-01-16T13:14:40.363Z
Learnt from: jjramirezn
PR: peanutprotocol/peanut-ui#631
File: src/components/Create/Create.tsx:108-112
Timestamp: 2025-01-16T13:14:40.363Z
Learning: In the Peanut UI codebase, the `resetTokenContextProvider` function from `tokenSelectorContext` is a stable function reference that doesn't change, so it doesn't need to be included in useEffect dependencies.
Applied to files:
src/app/(mobile-ui)/withdraw/crypto/page.tsx
⏰ 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 (7)
src/components/Payment/Views/Status.payment.view.tsx (4)
39-40
: Prop addition LGTM.Optional
onComplete
keeps backward compatibility.
53-54
: Destructuring LGTM.No behavior change when
onComplete
is undefined.
166-175
: Invoke onComplete before navigation/reset — good; ensure idempotence.Calling it first is correct for upstream cleanup (e.g., withdraw flow). Please confirm
onComplete
is idempotent and safe to call multiple times.
189-196
: Back/cancel also triggering onComplete — good consistency.This keeps success-exit paths aligned on mobile.
src/app/(mobile-ui)/withdraw/crypto/page.tsx (1)
369-371
: Resetting withdraw flow via onComplete — matches new lifecycle.Good move from unmount to explicit completion.
src/components/TransactionDetails/transactionTransformer.ts (1)
150-151
: Introduction of fullName accumulator — good.Keeps avatar naming separate from handles/identifiers.
src/components/TransactionDetails/TransactionCard.tsx (1)
68-70
: Avatar name fallback LGTM.
transaction.fullName || transaction.userName
is the right order given the transformer changes.Please verify all TransactionDetails producers now set
fullName
(see transformer comment); the fallback here protects the UI, but consistency helps.
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.
lgtm
also contributes to-
TASK-14694
TASK-14698