fix: show usd value everywhere in payment flow #910
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
## Walkthrough
This change introduces logic to fetch, store, and display a USD equivalent amount for charges in the payment flow. It updates the Redux payment state, UI components, and related types to support and prioritize USD display when available, including new properties and conditional formatting in transaction and confirmation views.
## Changes
| Files/Paths | Change Summary |
|-----------------------------------------------------------------------------|------------------------------------------------------------------------------------------------------------------------------------------|
| src/app/[...recipient]/client.tsx | Adds logic to fetch/store USD amount for charges, updates hooks and memoization to use USD if available. |
| src/components/Global/TokenSelector/TokenSelector.tsx | Ensures chain logo URI is set consistently when token and chain info are present, regardless of wallet connection. |
| src/components/Payment/PaymentForm/index.tsx | Dispatches USD amount to payment state during payment initiation if input USD value is provided. |
| src/components/Payment/Views/Confirm.payment.view.tsx<br>src/components/Payment/Views/Status.payment.view.tsx | Adds and uses USD amount from payment store for display and transaction details, updates memoization dependencies. |
| src/components/TransactionDetails/TransactionDetailsDrawer.tsx | Prefers displaying transaction amount from USD currency field if present. |
| src/redux/slices/payment-slice.ts<br>src/redux/types/payment.types.ts | Adds `usdAmount` property to payment state and type, introduces `setUsdAmount` reducer. |
| src/services/services.types.ts | Adds `currencyAmount` and `currencyCode` to `TRequestChargeResponse` interface. |
| src/app/manifest.ts | Adds `orientation: 'portrait'` property to the manifest return object. |
## Possibly related PRs
- [peanutprotocol/peanut-ui#720](https://github.com/peanutprotocol/peanut-ui/pull/720): Both PRs modify payment form logic for handling USD amounts and token prices.
- [peanutprotocol/peanut-ui#852](https://github.com/peanutprotocol/peanut-ui/pull/852): Both PRs extend payment flow components to handle currency amounts, focusing on different currencies and approaches.
## Suggested reviewers
- jjramirezn📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
src/services/services.types.ts (1)
202-204: Consider grouping currency data into a dedicated object
currencyAmountandcurrencyCodeare added as loose, optional fields. We now have three separate ways to surface “amount” (tokenAmount,currencyAmount, and the nestedpayments[].paidAmountInRequestedToken). A small typed wrapper (e.g.currency?: { amount: string; code: string }) would:
- keep related information together
- avoid param-explosion if more currency attributes appear (symbol, rate, etc.)
- make downstream discriminated-union checks simpler (
if (charge.currency)…).Refactoring is optional for this PR but will pay off as multi-currency handling grows.
src/components/TransactionDetails/TransactionDetailsDrawer.tsx (1)
135-137: AvoidNumber()on unknown string — preferparseFloatwith fallback
Number(transaction.currency.amount)will returnNaNfor empty string/whitespace, producing"$ 0"afterformatAmount, which can silently hide bad data. Consider:- : transaction.currency?.amount - ? `$ ${formatAmount(Number(transaction.currency.amount))}` + : transaction.currency?.amount + ? `$ ${formatAmount(parseFloat(transaction.currency.amount) || 0)}`This still formats correctly but guards against non-numeric strings.
src/components/Payment/Views/Confirm.payment.view.tsx (2)
197-203: RedundantNumber()conversion
formatAmountalready acceptsstring | number. Wrapping withNumber(...)is unnecessary and can silently coerce''/nullinto0.- return `$ ${formatAmount(Number(usdAmount))}` + return `$ ${formatAmount(usdAmount)}`
205-209: Returnundefinedinstead of an empty string for optional symbolDown-stream components can treat
''as a valid value and attempt to render it.
Returningundefinedmakes the “no-symbol” intent explicit and avoids awkward paddings/spacings.- return '' + return undefinedsrc/components/Payment/Views/Status.payment.view.tsx (1)
78-84: SameNumber()coercion issue as in Confirm view- if (usdAmount) return `$ ${formatAmount(Number(usdAmount))}` + if (usdAmount) return `$ ${formatAmount(usdAmount)}`Keeps behaviour consistent and avoids silent
NaNon unexpected input.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/app/[...recipient]/client.tsx(7 hunks)src/components/Global/TokenSelector/TokenSelector.tsx(1 hunks)src/components/Payment/PaymentForm/index.tsx(1 hunks)src/components/Payment/Views/Confirm.payment.view.tsx(3 hunks)src/components/Payment/Views/Status.payment.view.tsx(5 hunks)src/components/TransactionDetails/TransactionDetailsDrawer.tsx(1 hunks)src/redux/slices/payment-slice.ts(2 hunks)src/redux/types/payment.types.ts(1 hunks)src/services/services.types.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
src/components/Payment/Views/Confirm.payment.view.tsx (2)
src/redux/hooks.ts (1)
usePaymentStore(12-12)src/utils/general.utils.ts (1)
formatAmount(300-340)
src/components/TransactionDetails/TransactionDetailsDrawer.tsx (1)
src/utils/general.utils.ts (1)
formatAmount(300-340)
src/components/Payment/Views/Status.payment.view.tsx (3)
src/redux/hooks.ts (1)
usePaymentStore(12-12)src/utils/general.utils.ts (1)
formatAmount(300-340)src/constants/zerodev.consts.ts (1)
PEANUT_WALLET_TOKEN_SYMBOL(21-21)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Deploy-Preview
🔇 Additional comments (5)
src/redux/types/payment.types.ts (1)
19-20: Double-check lifecycle ofusdAmount
usdAmountis persisted in global state but there is no corresponding reset inpaymentActions.reset()(or equivalent clear-flow). A stale USD value could leak into a next transaction if the user navigates back and starts another payment without refreshing.Action items:
- Ensure any “reset / clear” reducer wipes
usdAmount.- When the token amount is edited back to zero, dispatch
setUsdAmount(null).Small but prevents subtle display bugs.
src/components/Global/TokenSelector/TokenSelector.tsx (1)
246-247: LGTM – logo now shown for connected-wallet pathAssigning
buttonChainLogoURIinside the external-wallet branch fixes the missing chain badge. No issues spotted.src/redux/slices/payment-slice.ts (1)
24-25: Prefer a numeric type forusdAmountto avoid repeated parsing
usdAmountis stored as astring, yet every consumer immediately converts it usingNumber(...).
Keeping it asnumber | null:
- Eliminates the double-parsing overhead (
Number➜formatAmountconverts again).- Prevents accidental string concatenation bugs elsewhere in the slice.
Diff illustrating the change in this file:
- usdAmount: null, + usdAmount: null as number | null, ... - setUsdAmount: (state, action: PayloadAction<string | null>) => { + setUsdAmount: (state, action: PayloadAction<number | null>) => { state.usdAmount = action.payload },Types in
IPaymentStateand everydispatch(paymentActions.setUsdAmount(...))call would need the same update.
[ suggest_essential_refactor ]Also applies to: 64-66
src/components/Payment/Views/Status.payment.view.tsx (1)
98-102:amountshould mirror raw token amount, not the USD display value
TransactionDetails.amountis later used for analytics / maths.
Storing the fiat value there alters semantics. Consider:- amount: usdAmount ? Number(usdAmount) : parseFloat(amountValue), + amount: parseFloat(amountValue),…and rely on the newly-added
currencyfield for the USD representation.src/app/[...recipient]/client.tsx (1)
282-301:Number(chargeDetails.tokenAmount)multiplied by price loses precision for large token values
tokenAmountis a string that may already be in wei-scaled units depending on upstream service. Consider using a big-number library (e.g.ethers.js’sformatUnits) before the multiplication to avoid precision loss.(Request verification if the service already returns human-readable decimals).
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
src/app/[...recipient]/client.tsx (1)
115-178:⚠️ Potential issueNested
useEffectcauses a syntax error & invalid hook usage
useEffectis declared inside an existinguseEffectcallback (and even in the middle of a.get()call).
React hooks must be invoked at the top level of the component – never conditionally and never nested inside other callbacks.
Besides violating the Rules of Hooks, this leaves the outerchargesApi.get(chargeId)call unfinished, so the file no longer parses (see Biome / formatter failure).- if (chargeId) { - chargesApi - .get(chargeId) -useEffect(() => { // ❌ nested hook starts here - if (!chargeId) return +// 1️⃣ Delete the obsolete outer logic (lines 118-178) completely. + +// 2️⃣ Keep a single top-level effect: + +useEffect(() => { + if (!chargeId) return + let stale = false ;(async () => { try { const charge = await chargesApi.get(chargeId) dispatch(paymentActions.setChargeDetails(charge)) … if (charge.payments?.length) { const latest = charge.payments.at(-1) if (latest.status !== 'NEW') dispatch(paymentActions.setView('STATUS')) } } catch (err) { if (!stale) setError(getDefaultError(!!user)) } })() return () => { stale = true } -}, [chargeId, dispatch, user]) - // check latest payment status if payments exist - if (charge.payments && charge.payments.length > 0) { - … - }) - .catch((_err) => { - setError(getDefaultError(!!user)) - }) - } - }, [chargeId, dispatch, user]) +}, [chargeId, dispatch, user])This restores valid syntax, avoids double-fetching the charge, and still handles the “latest payment status” logic.
Failing to fix this will keep CI red and the page from rendering at all.🧰 Tools
🪛 Biome (1.9.4)
[error] 172-173: Expected a statement but instead found ')
.catch((_err) =>'.Expected a statement here.
(parse)
[error] 175-175: Expected a statement but instead found ')'.
Expected a statement here.
(parse)
[error] 177-177: expected
,but instead found}Remove }
(parse)
[error] 173-175: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
Safe fix: Remove redundant block.(lint/complexity/noUselessLoneBlockStatements)
🪛 GitHub Actions: Code Formatting
[error] 172-172: SyntaxError: Declaration or statement expected at line 172, column 18.
🧹 Nitpick comments (2)
src/app/[...recipient]/client.tsx (2)
130-152:toFixed(2)returns a string – store a number instead
setUsdAmountis fed the string result ofNumber(...).toFixed(2).
Down-stream consumers treatusdAmountas a numeric quantity (e.g., arithmetic or sorting) this will coerce each time and may bite later.- paymentActions.setUsdAmount(Number(charge.currencyAmount).toFixed(2)) + paymentActions.setUsdAmount(parseFloat(charge.currencyAmount.toString()))Same change applies to the branch using
usdValue.toFixed(2).
Prefer keeping raw numbers in state and format only at render time.
324-325: Minor:amountshould remain numeric incurrencypayload
currency: { amount: usdAmount, code: 'USD' }passes the (string)usdAmountstraight into the drawer component.
If you adopt the numeric advice above, wrap it here:amount: usdAmount.toFixed(2). Otherwise leave as is.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/app/[...recipient]/client.tsx(6 hunks)src/components/Payment/Views/Status.payment.view.tsx(4 hunks)src/services/services.types.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/services/services.types.ts
- src/components/Payment/Views/Status.payment.view.tsx
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/app/[...recipient]/client.tsx (4)
src/redux/hooks.ts (1)
usePaymentStore(12-12)src/services/charges.ts (1)
chargesApi(6-91)src/redux/slices/payment-slice.ts (1)
paymentActions(73-73)src/app/actions/tokens.ts (1)
fetchTokenPrice(59-108)
🪛 Biome (1.9.4)
src/app/[...recipient]/client.tsx
[error] 177-177: expected , but instead found }
Remove }
(parse)
[error] 177-180: Expected a statement but instead found ', [chargeId, dispatch, user])
// fetch requests for the recipient only when id is not available in the URL
useEffect(() =>'.
Expected a statement here.
(parse)
🪛 GitHub Actions: Code Formatting
src/app/[...recipient]/client.tsx
[error] 172-172: SyntaxError: Declaration or statement expected at line 172, column 18.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Deploy-Preview
contributes to TASK-11896 : now fetches usd value for a token and use it in the ui (i dont know what POL/ETH/blah...blah...blah is, show me dollars )
also fixes TASK-11613 : orientation for pwa