-
Notifications
You must be signed in to change notification settings - Fork 13
feat: withdraw using peanut wallet #870
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 Git ↗︎
|
WalkthroughThis update introduces a comprehensive withdrawal flow for the mobile UI, including new pages, context providers, and view components for handling add and withdraw money operations. It refactors existing add-money components to support both add and withdraw flows, restructures payment method data to be country-specific, and implements utility and UI improvements across multiple components. Several new modules are added, and existing ones are updated for consistency, extensibility, and better user experience. Changes
Suggested reviewers
✨ Finishing Touches
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 (
|
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.
AND WE ARE BACK TO USING CONTEXT, RIP REDUX HAHAHAHAHHA
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.
Love to see it
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: 12
🧹 Nitpick comments (14)
src/context/authContext.tsx (1)
176-176
: Remove console.log statementThere's a console.log statement that should be removed before production deployment.
- console.log({ user })
src/components/Global/Icons/check-circle.tsx (1)
1-10
: Well-structured icon component with minor improvement opportunityThe CheckCircleIcon component is well-implemented and follows the proper pattern for SVG icon components.
Consider making the fill color customizable (via props or CSS variables) rather than hardcoded to black, which would provide more flexibility and consistency with typical icon systems. Additionally, note that the SVG has slightly different width and height dimensions (12x13) which is uncommon for a circle icon - ensure this is intentional for visual alignment purposes.
- fill="black" + fill={props.fill || "currentColor"}src/components/Claim/Claim.tsx (1)
225-234
: Missing dependency & potential stale closure
openTransactionDetails
is called insideuseEffect
, but it is not listed in the dependency array. If the hook ever returns a new function instance (e.g. internal state update), this effect will not fire again, causing the drawer not to open.- }, [linkState, transactionForDrawer]) + }, [linkState, transactionForDrawer, openTransactionDetails])This is a tiny change, yet it keeps ESLint/react-hooks happy and prevents rare edge-cases.
src/components/AddMoney/components/DepositMethodList.tsx (1)
58-68
:position
calculation produces duplicate “first” cardsWhen the first item is
crypto
and the second item iscountry
, both index 0 and 1 return'first'
, breaking border-radius/spacing.A clearer approach:
- position={ - methods.length === 1 - ? 'single' - : index === 0 - ? 'first' - : method.type === 'country' && index === 1 - ? 'first' - : index === methods.length - 1 - ? 'last' - : 'middle' - } + position={ + methods.length === 1 + ? 'single' + : index === 0 + ? 'first' + : index === methods.length - 1 + ? 'last' + : 'middle' + }If you need extra spacing for the first
country
entry, consider using a CSS utility or the card’sclassName
prop instead of manipulating the “position” semantic.src/components/AddWithdraw/components/AddWithdrawCountriesList.tsx (1)
74-86
: Consider adding a loading state or empty state for methods.The component doesn't explicitly handle cases where methods exist for the country but are empty for the specific flow (add/withdraw). This could lead to a confusing UI with just a header and no content.
Consider adding an empty state component when
methods?.[flow]
exists but is empty:<div className="flex-1 overflow-y-auto"> {flow === 'add' && methods?.add && renderPaymentMethods('Add money via', methods.add)} {flow === 'withdraw' && methods?.withdraw && renderPaymentMethods('Choose withdrawing method', methods.withdraw)} + {methods && !methods[flow]?.length && ( + <EmptyState + title={`No ${flow === 'add' ? 'deposit' : 'withdrawal'} methods available`} + description="Please try a different country or check back later." + icon="info" + /> + )} </div>src/components/AddWithdraw/components/AddWithdrawRouterView.tsx (2)
32-40
: StabiliseuseMemo
to avoid silent re-computations
ALL_METHODS_DATA
is referenced inside the memo but isn’t listed in the dependency array.
If this constant is ever rebuilt (e.g., during hot-reload) the memo will not update, leaving UI out-of-sync.
Also,map
allocates a new array each render even when input hasn’t changed.- const allMethodsTransformed: DepositMethod[] = useMemo(() => { - return ALL_METHODS_DATA.map((method) => { + const allMethodsTransformed: DepositMethod[] = useMemo(() => { + return ALL_METHODS_DATA.map((method) => { … - }, [baseRoute]) + }, [baseRoute, ALL_METHODS_DATA])If
ALL_METHODS_DATA
truly is a compile-time constant, wrap the import withObject.freeze
or move the transformation out of render to avoid memo completely.
85-88
: Persist recent methods – the TODO is user-visibleThe “Recent methods” feature resets on every refresh because state is not persisted.
Consider:
- Serialising
recentMethods
tolocalStorage
when the array changes.- Hydrating on mount.
If you’d like, I can sketch a small hook (
useLocalStorageState
) to replace the currentuseState
.src/context/WithdrawFlowContext.tsx (1)
59-63
: Defaultname
should honour the declared type
name
is specified asstring | undefined
but initialised with an empty string.
Preferundefined
to clearly indicate “no value yet”:-const [recipient, setRecipient] = useState<RecipientState>({ address: '', name: '' }) +const [recipient, setRecipient] = useState<RecipientState>({ address: '', name: undefined })src/app/(mobile-ui)/withdraw/crypto/page.tsx (3)
54-63
: Effect dependency list includes unused setters → eslint-exhaustive-deps noise
setAmountToWithdraw
is listed but never used inside the effect.
This can confuse future maintainers and trigger linter warnings.-}, [amountToWithdraw, router, dispatch, setAmountToWithdraw, setCurrentView]) +}, [amountToWithdraw, router, dispatch, setCurrentView])
165-198
: Error handling path omits toast / user feedbackOn failure, only context state is updated.
If the modal is already closed the user receives no visible cue.
Recommend surfacing the error via a global toast/snackbar or the existingActionModal
with an “Okay” CTA.
239-251
: Remove redundant<> Fragment </>
Static analysis is correct – the fragment wraps a single
DirectSuccessView
.-<> - <DirectSuccessView … /> -</> +<DirectSuccessView … />🧰 Tools
🪛 Biome (1.9.4)
[error] 240-250: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment(lint/complexity/noUselessFragments)
src/components/AddMoney/consts/index.ts (3)
176-183
: Consider adding an explicitpath
for the default bank withdrawalDown-stream components might rely on
path
to navigate. Supplying a dummy (''
) or omitting it is fine for now but makes the contract hazy—please document or add it explicitly to avoidundefined
checks everywhere.
184-221
: Key on ISO country codes, not localized titles, for RTP lookup
rtpWithdrawMethodsData
is indexed by the human-readablecountry.title
.
Because titles can change (translation, diacritics, punctuation) this is brittle and already fails for"Côte d'Ivoire"
,"Åland"
, etc.
Storing/looking-up by the stable ISO-3166 code (country.id
) is safer and removes the need for title normalisation.Follow-up sketch:
-const rtpWithdrawMethodsData: Record<string, Array<{ title: string; description: string }>> = { - India: [ … ], +const rtpWithdrawMethodsData: Record<string, Array<{ title: string; description: string }>> = { + IN: [ … ],and later:
-const specificWithdrawals = rtpWithdrawMethodsData[countryTitle] +const specificWithdrawals = rtpWithdrawMethodsData[countryCode]
1983-2025
: Minor optimisation & readability inside the builder loop
countryData.forEach
mutates an external object; returning a new object viareduce
keeps the function pure (easier to unit-test).- The inner
if/else
that checksrtpWithdrawMethodsData['Germany']
just to gate “SEPA Instant” can be simplified to a constant since that key will always exist once the table is initialised.Not blocking, but consider refactoring:
-export const COUNTRY_SPECIFIC_METHODS: Record<string, CountrySpecificMethods> = {} - -countryData.forEach((country) => { - … - COUNTRY_SPECIFIC_METHODS[countryCode] = … -}) +export const COUNTRY_SPECIFIC_METHODS: Record<string, CountrySpecificMethods> = + countryData + .filter((c) => c.type === 'country') + .reduce<Record<string, CountrySpecificMethods>>((acc, country) => { + const withdrawList = buildWithdrawList(country) + acc[country.id] = { add: UPDATED_DEFAULT_ADD_MONEY_METHODS.map((m) => ({ ...m })), withdraw: withdrawList } + return acc + }, {})
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (46)
src/app/(mobile-ui)/add-money/[...country]/page.tsx
(1 hunks)src/app/(mobile-ui)/add-money/crypto/direct/page.tsx
(1 hunks)src/app/(mobile-ui)/add-money/crypto/page.tsx
(1 hunks)src/app/(mobile-ui)/add-money/layout.tsx
(1 hunks)src/app/(mobile-ui)/add-money/page.tsx
(1 hunks)src/app/(mobile-ui)/home/page.tsx
(1 hunks)src/app/(mobile-ui)/withdraw/[...country]/page.tsx
(1 hunks)src/app/(mobile-ui)/withdraw/crypto/page.tsx
(1 hunks)src/app/(mobile-ui)/withdraw/layout.tsx
(1 hunks)src/app/(mobile-ui)/withdraw/page.tsx
(1 hunks)src/app/[...recipient]/client.tsx
(1 hunks)src/app/[...recipient]/payment-layout-wrapper.tsx
(1 hunks)src/assets/index.ts
(1 hunks)src/components/0_Bruddle/PageContainer.tsx
(1 hunks)src/components/AddMoney/components/DepositMethodList.tsx
(3 hunks)src/components/AddMoney/consts/index.ts
(1 hunks)src/components/AddWithdraw/components/AddWithdrawCountriesList.tsx
(1 hunks)src/components/AddWithdraw/components/AddWithdrawRouterView.tsx
(4 hunks)src/components/Claim/Claim.tsx
(6 hunks)src/components/Claim/Generic/AlreadyClaimed.view.tsx
(0 hunks)src/components/Claim/Generic/SenderClaim.view.tsx
(0 hunks)src/components/Claim/Generic/index.ts
(0 hunks)src/components/Claim/Link/Initial.view.tsx
(2 hunks)src/components/Claim/Link/Onchain/Success.view.tsx
(4 hunks)src/components/Global/ActionModal/index.tsx
(2 hunks)src/components/Global/Card/index.tsx
(1 hunks)src/components/Global/Icons/Icon.tsx
(3 hunks)src/components/Global/Icons/check-circle.tsx
(1 hunks)src/components/Global/PeanutActionDetailsCard/index.tsx
(4 hunks)src/components/Global/ValidatedInput/index.tsx
(5 hunks)src/components/Payment/PaymentForm/index.tsx
(2 hunks)src/components/Payment/PaymentInfoRow.tsx
(1 hunks)src/components/Payment/Views/Status.payment.view.tsx
(5 hunks)src/components/Request/link/views/Create.request.link.view.tsx
(3 hunks)src/components/TransactionDetails/TransactionDetailsDrawer.tsx
(2 hunks)src/components/TransactionDetails/TransactionDetailsHeaderCard.tsx
(2 hunks)src/components/TransactionDetails/transactionTransformer.ts
(1 hunks)src/components/Withdraw/views/Confirm.withdraw.view.tsx
(1 hunks)src/components/Withdraw/views/Initial.withdraw.view.tsx
(1 hunks)src/context/WithdrawFlowContext.tsx
(1 hunks)src/context/authContext.tsx
(3 hunks)src/context/contextProvider.tsx
(2 hunks)src/hooks/useRecentUsers.ts
(1 hunks)src/hooks/useWebSocket.ts
(1 hunks)src/services/services.types.ts
(1 hunks)src/styles/globals.css
(1 hunks)
💤 Files with no reviewable changes (3)
- src/components/Claim/Generic/SenderClaim.view.tsx
- src/components/Claim/Generic/index.ts
- src/components/Claim/Generic/AlreadyClaimed.view.tsx
🧰 Additional context used
🧠 Learnings (1)
src/hooks/useWebSocket.ts (1)
Learnt from: jjramirezn
PR: peanutprotocol/peanut-ui#857
File: src/hooks/useWebSocket.ts:77-82
Timestamp: 2025-05-15T14:47:26.891Z
Learning: The useWebSocket hook in src/hooks/useWebSocket.ts is designed to provide raw history entries, while the components using it (such as HomeHistory.tsx) are responsible for implementing deduplication logic based on UUID to prevent duplicate entries when combining WebSocket data with other data sources.
🧬 Code Graph Analysis (12)
src/context/contextProvider.tsx (1)
src/context/WithdrawFlowContext.tsx (1)
WithdrawFlowContextProvider
(51-95)
src/app/(mobile-ui)/add-money/page.tsx (1)
src/components/AddWithdraw/components/AddWithdrawRouterView.tsx (1)
AddWithdrawRouterView
(19-131)
src/components/Claim/Link/Onchain/Success.view.tsx (3)
src/context/authContext.tsx (1)
useAuth
(198-204)src/utils/general.utils.ts (2)
getTokenDetails
(894-913)printableAddress
(57-60)src/components/0_Bruddle/Button.tsx (1)
Button
(67-117)
src/components/Request/link/views/Create.request.link.view.tsx (3)
src/services/charges.ts (1)
chargesApi
(6-81)src/constants/general.consts.ts (1)
BASE_URL
(42-42)src/constants/query.consts.ts (1)
TRANSACTIONS
(2-2)
src/components/Global/ActionModal/index.tsx (2)
src/components/0_Bruddle/Button.tsx (1)
Button
(67-117)src/components/Global/Icons/Icon.tsx (1)
Icon
(149-158)
src/components/AddWithdraw/components/AddWithdrawCountriesList.tsx (3)
src/components/AddMoney/consts/index.ts (3)
countryData
(223-1981)COUNTRY_SPECIFIC_METHODS
(1983-1983)SpecificPaymentMethod
(117-124)src/components/Global/EmptyStates/EmptyState.tsx (1)
EmptyState
(13-28)src/components/SearchUsers/SearchResultCard.tsx (1)
SearchResultCard
(18-62)
src/components/AddWithdraw/components/AddWithdrawRouterView.tsx (2)
src/components/AddMoney/components/DepositMethodList.tsx (2)
DepositMethod
(8-16)DepositMethodList
(23-74)src/components/SearchUsers/SearchInput.tsx (1)
SearchInput
(14-46)
src/app/(mobile-ui)/withdraw/crypto/page.tsx (9)
src/redux/hooks.ts (2)
useAppDispatch
(5-5)usePaymentStore
(12-12)src/context/WithdrawFlowContext.tsx (2)
useWithdrawFlow
(97-103)WithdrawData
(9-14)src/hooks/usePaymentInitiator.ts (2)
usePaymentInitiator
(61-945)InitiatePaymentPayload
(35-49)src/redux/slices/payment-slice.ts (1)
paymentActions
(69-69)src/utils/token.utils.ts (1)
NATIVE_TOKEN_ADDRESS
(27-27)src/services/requests.ts (1)
requestsApi
(5-55)src/services/charges.ts (1)
chargesApi
(6-81)src/components/Global/PeanutLoading/index.tsx (1)
PeanutLoading
(4-19)src/utils/general.utils.ts (1)
printableAddress
(57-60)
src/context/WithdrawFlowContext.tsx (1)
src/interfaces/interfaces.ts (1)
ITokenPriceData
(80-83)
src/components/Global/Icons/Icon.tsx (1)
src/components/Global/Icons/check-circle.tsx (1)
CheckCircleIcon
(3-10)
src/components/AddMoney/components/DepositMethodList.tsx (1)
src/components/SearchUsers/SearchResultCard.tsx (1)
SearchResultCard
(18-62)
src/components/Global/ValidatedInput/index.tsx (1)
src/components/Global/Icons/Icon.tsx (1)
Icon
(149-158)
🪛 Biome (1.9.4)
src/app/(mobile-ui)/withdraw/crypto/page.tsx
[error] 240-250: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
🔇 Additional comments (69)
src/components/Global/Card/index.tsx (1)
49-49
: Class update improves content display flexibilityThe removal of the
overflow-hidden
class from the Card component allows content to potentially extend beyond the card boundaries, which is important for components like dropdowns, tooltips, or overlays that might need to break out of the container.src/app/(mobile-ui)/home/page.tsx (1)
83-83
: Fixed typo in withdraw navigation pathThe path has been corrected from
/witdraw
to/withdraw
, ensuring users can properly navigate to the withdrawal flow.src/app/(mobile-ui)/add-money/crypto/page.tsx (1)
124-124
: Simplified icon implementationThe change from JSX element to string value for the icon prop aligns with updates to the ActionModal component, which now handles icon rendering internally.
src/app/[...recipient]/payment-layout-wrapper.tsx (1)
34-36
: Responsive layout adjustment aligns with broader UI refinements.The removal of the
h-full
class while maintaining themin-h-[calc(100dvh-160px)]
constraint improves the layout's responsiveness. This change is consistent with other layout refinements across the application to enhance responsive behavior.src/components/Payment/PaymentInfoRow.tsx (1)
32-33
: Z-index improvement prevents tooltip layering issues.Adding the
z-40
class ensures the info icon and tooltip trigger appear above other elements, preventing potential UI layering issues. This works well with the tooltip'sz-50
class on line 49.src/services/services.types.ts (1)
101-107
:✅ Verification successful
Making requestProps optional supports flexible charge creation.
Making
requestProps
optional enhances the flexibility of theCreateChargeRequest
interface to support various charge creation scenarios in the new withdrawal flow.
🏁 Script executed:
#!/bin/bash # Search for usages of CreateChargeRequest to verify the change is used correctly echo "Searching for usages of CreateChargeRequest where requestProps might be undefined:" rg "CreateChargeRequest" -A 5 -B 5Length of output: 5298
Approve Optional
requestProps
in CreateChargeRequest
All current implementations ofCreateChargeRequest
(e.g., inusePaymentInitiator.ts
andwithdraw/crypto/page.tsx
) explicitly supplyrequestProps
. Making it optional won’t break existing flows and supports new withdrawal scenarios.src/assets/index.ts (1)
3-3
: Added exchanges export supports new withdrawal functionality.The addition of the exchanges export makes the exchange-related assets available for the new withdrawal flow. This supports the implementation of country-specific payment methods and crypto withdrawals.
src/app/(mobile-ui)/withdraw/[...country]/page.tsx (1)
1-9
: Well-structured component for country-specific withdraw flowThe component properly implements a country-specific withdrawal page by reusing the shared
AddWithdrawCountriesList
component with the appropriate "withdraw" flow. This follows good practices for code reusability between add-money and withdraw flows.src/components/Global/Icons/Icon.tsx (1)
12-12
: Added support for check-circle iconThe implementation correctly adds the new check-circle icon to the icon system by:
- Importing the component
- Adding it to the IconName type
- Mapping it in the iconComponents record
This follows the established pattern for icon integration.
Also applies to: 60-60, 146-146
src/context/authContext.tsx (1)
11-11
: Improved logout flow with query cache invalidationAdding React Query's cache invalidation during logout is an excellent practice that prevents stale data from being displayed if another user logs in. This ensures all components using React Query will fetch fresh data after a user switch.
Also applies to: 51-51, 154-154
src/app/(mobile-ui)/add-money/[...country]/page.tsx (2)
1-3
: Good addition of client-side component directiveThe 'use client' directive correctly marks this component as a client component, which is necessary since you're now using client-side features with the AddWithdrawCountriesList component.
6-6
: Clean implementation of unified flow patternThe component now renders AddWithdrawCountriesList with flow="add", which aligns with the unified approach to handling both add-money and withdrawal flows. This creates a consistent user experience and reduces code duplication.
src/components/0_Bruddle/PageContainer.tsx (1)
7-10
: Improved responsive layout with max-widthReplacing
md:*:w-2/5
withmd:*:max-w-xl
is a good improvement for the container styling. Using max-width instead of a fixed fractional width provides better control over the UI at various viewport sizes and helps maintain consistency across the application.src/hooks/useWebSocket.ts (1)
84-87
: Effective filtering of pending direct send requestsThe conditional check to ignore certain pending requests with type 'DIRECT_SEND', status 'NEW', and no senderAccount is appropriate. This prevents unwanted entries from being added to the historyEntries state, which aligns with the retrieved learning that this hook provides raw entries while consumers implement their own deduplication logic.
This change helps maintain more accurate transaction history state, especially important for the new withdrawal flow integration.
src/components/Claim/Link/Initial.view.tsx (1)
479-486
: Important improvement to redirect flowAdding
saveRedirectUrl()
before navigation is a key improvement. This ensures that after authentication, users will be correctly redirected back to the claim page they were attempting to access, creating a smoother user experience.src/components/TransactionDetails/TransactionDetailsDrawer.tsx (2)
43-50
: Improved precision in pending transaction detectionThe addition of the
transaction.status === 'pending'
check in theisPendingRequestee
condition correctly ensures this flag only applies to transactions that are actually in a pending state. This prevents potential UI issues where non-pending transactions might incorrectly trigger requestee-specific actions.
52-59
: Consistent status check for pending requester stateSimilar to the previous change, adding the explicit
transaction.status === 'pending'
check here ensures accurate identification of pending requester transactions. This consistent approach prevents potential UI rendering issues and aligns with the component's purpose of showing appropriate action buttons only for truly pending transactions.src/hooks/useRecentUsers.ts (1)
17-20
: Enhanced duplicate user detectionThe improved duplicate detection now checks for matching usernames case-insensitively in addition to matching user IDs. This prevents duplicate entries in the recent users list when the same user appears with different case variations in their username, improving the user experience.
src/components/Request/link/views/Create.request.link.view.tsx (2)
12-14
: Added necessary imports and query client for charge creationThe added imports and query client setup properly support the new charge creation functionality being implemented in this component.
Also applies to: 20-20, 26-26, 43-43
152-164
:✅ Verification successful
Added charge creation for request links
This addition creates a charge in the system after successfully creating a request link, which is essential for tracking and processing the payment. The invalidation of the transactions query ensures the UI is updated with the latest transaction data.
🏁 Script executed:
#!/bin/bash # Check if the TRANSACTIONS constant is used consistently across the codebase echo "Checking TRANSACTIONS constant usage in query invalidations..." rg -A 3 "invalidateQueries.*TRANSACTIONS" --type tsLength of output: 2402
Charge creation & TRANSACTIONS invalidation consistent with existing views
The new code correctly creates a charge for request links and invalidates theTRANSACTIONS
query, matching the pattern used in other components (Claim and Payment views) to keep the UI in sync.src/app/(mobile-ui)/add-money/layout.tsx (1)
5-5
: Improved responsive layout with centered contentAdding the
md:self-center
class ensures the container is centered on medium and larger screens, creating a more consistent layout with the withdrawal flow and improving the overall UI appearance.src/app/(mobile-ui)/withdraw/layout.tsx (1)
1-6
: Clean layout implementation with consistent stylingThe WithdrawLayout component is well-structured and follows the same pattern as other layout components in the application, particularly matching the styling used in the add-money flow with the responsive
md:self-center
class. This maintains UI consistency across the application.src/context/contextProvider.tsx (2)
3-7
: Well-organized import structureThe rearrangement of imports maintains a clean, logical ordering while adding the new WithdrawFlowContext import.
16-18
: Proper integration of WithdrawFlowContextProviderThe WithdrawFlowContextProvider is correctly nested within the LoadingStateContextProvider, which allows the withdrawal flow to access loading state information while providing withdrawal-specific state to child components.
src/app/(mobile-ui)/add-money/crypto/direct/page.tsx (2)
25-25
: Improved component structure by removing redundant containerSimplifying the loading state render by directly returning the PeanutLoading component without the PageContainer wrapper reduces unnecessary nesting.
30-30
: Streamlined component renderingDirectly returning the PaymentPage component improves the structure by removing redundant container wrapping. This is consistent with the layout restructuring throughout the application where container styling is now handled at the layout level.
src/app/(mobile-ui)/add-money/page.tsx (1)
1-18
: Good implementation of client-side component for Add Money flow.The component correctly uses the shared
AddWithdrawRouterView
component for the "add" flow, with appropriate props for page title, heading, and navigation. This aligns well with the refactoring to unify add-money and withdrawal flows using a common component structure.src/components/Payment/PaymentForm/index.tsx (1)
477-488
: Improved navigation control with conditional routing.The replacement of static text with a
NavHeader
component enhances user experience by providing proper back navigation based on the flow type. The routing logic correctly directs users to the appropriate route based on context:/add-money
for add flow,/withdraw
for withdraw flow, and/home
as default fallback.src/components/TransactionDetails/TransactionDetailsHeaderCard.tsx (1)
33-58
: More nuanced transaction title display for improved UX.The changes improve title handling logic by:
- Only using generic link transaction phrases when
userName
is falsy- Simplifying title display for pending/cancelled send transactions by showing only the display name rather than prefixing with "Sent to" or "Sending to"
These changes provide a more appropriate user experience based on the transaction status context.
src/components/Global/PeanutActionDetailsCard/index.tsx (4)
11-11
: Added 'WITHDRAW' to transaction types.The transaction type has been properly extended to include the new withdrawal flow.
45-45
: Appropriate icon selection for withdrawal transactions.Using the 'arrow-up' icon for withdrawal transactions maintains consistency with the overall design system where upward arrows indicate money leaving the account.
59-59
: Clear title text for withdrawal transactions.The title "You're withdrawing" is concise and accurately describes the action being performed.
74-77
: Consistent icon display for wallet-related transactions.The logic properly includes the new 'WITHDRAW' transaction type alongside 'ADD_MONEY' and non-username recipient types for displaying the wallet-outline icon, maintaining UI consistency.
src/components/Global/ValidatedInput/index.tsx (5)
6-6
: Import path changed from absolute to relative.The import statement for the
Icon
component has been updated from an absolute path ('@/components/Global/Icon'
) to a relative path ('../Icons/Icon'
). This aligns with common practices for importing nearby components.
154-156
: Added consistent white background to input container.The root container now always includes a
bg-white
class, ensuring a consistent white background regardless of validation state.
174-176
: Improved input styling with mobile touch feedback.The input element's class list was updated to add
tap-highlight-color-white
and consolidate thebg-white
class, which improves the mobile touch experience by removing the default tap highlight color.
197-197
: Improved background consistency when not validating.The container wrapping the validation icon now has a white background (
bg-white
) instead of transparent when not validating, which maintains visual consistency with the rest of the input.
212-212
: Updated icon name from "close" to "cancel".The icon used for clearing the input was renamed from
"close"
to"cancel"
while keeping the same styling and placement. This suggests a standardization of icon naming across the application.src/components/Claim/Link/Onchain/Success.view.tsx (5)
7-8
: Updated imports for better token formatting and auth integration.Import changes:
- Replaced
formatAmount
withgetTokenDetails
and addedformatUnits
from viem for more precise token amount formatting- Added
Hash
type from viem for proper typing- Added
useAuth
hook for user data refresh functionality- Added
useMemo
for performance optimizationThese changes support the improved token amount formatting and user experience enhancements in this component.
Also applies to: 10-10, 12-14
23-23
: Added user data refresh capability.The component now destructures
fetchUser
from the auth context, which will be used to refresh user data after a successful claim.
64-73
: Added token details retrieval for accurate amount formatting.A new
useMemo
hook properly retrieves token details usinggetTokenDetails
with the token address and chain ID. This optimizes performance by caching the token details and only recalculating whenclaimLinkData
changes.
92-92
: Improved token amount formatting with proper decimals.The amount displayed in
PeanutActionDetailsCard
is now formatted usingformatUnits
with the token decimals fromtokenDetails
(defaulting to 6 if not available). This is more accurate than the previousformatAmount
method, especially for tokens with non-standard decimal places.
97-104
: Enhanced navigation with user data refresh.The "Back to home" button now calls
fetchUser()
before navigating to/home
, ensuring that user data is refreshed after a successful claim transaction. This provides a more up-to-date experience for the user when they return to the home screen.src/components/Global/ActionModal/index.tsx (2)
78-78
: Enhanced icon color inheritance.Added
fill="currentColor"
to theIcon
component when the icon is specified as a string. This ensures the icon inherits the current text color, improving visual consistency and accessibility.
169-203
: Improved button icon positioning logic.The rendering of call-to-action buttons was refactored to:
- Use a block body arrow function for better readability
- Introduce a
currentIconPosition
variable that defaults to'left'
if an icon exists but no position is provided- Pass this position to the Button component
- Conditionally render the icon on the left or right side based on position
This approach improves code clarity and ensures consistent icon placement in buttons throughout the modal.
src/components/TransactionDetails/transactionTransformer.ts (3)
107-107
: Updated sender role fallback text for better user context.For
SEND_LINK
transactions where the user role isSENDER
, the fallback recipient name text was updated from'Sent via Link'
to"You're sending via link"
. This change provides more clear context to the user about their role in the transaction.
109-109
: Added explicit link transaction tracking for sender role.A new boolean flag
isLinkTx
is now set totrue
if the peer is not a user. This allows the UI to properly distinguish between direct user-to-user transactions and link-based transactions.
115-115
: Added explicit link transaction tracking for recipient role.Similar to the sender role change, the
isLinkTx
flag is now set based on the sender's user status for recipient roles. This maintains consistency in how link transactions are tracked across all user roles.src/app/(mobile-ui)/withdraw/page.tsx (1)
55-60
: 🛠️ Refactor suggestionButton disabled logic can mis-disable when
rawTokenAmount
is non-numeric
parseFloat('abc')
returnsNaN
, which is falsy, so the button will not be disabled and clicking it will trigger the invalid-amount branch.disabled={ - !parseFloat(rawTokenAmount) || parseFloat(rawTokenAmount) <= 0 + !rawTokenAmount || isNaN(Number(rawTokenAmount)) || Number(rawTokenAmount) <= 0 }This pairs nicely with the improved validation above.
Likely an incorrect or invalid review comment.
src/components/Payment/Views/Status.payment.view.tsx (5)
33-33
: Prop addition for withdraw flow support looks good.The addition of
isWithdrawFlow
prop follows the existing pattern established withisAddMoneyFlow
, maintaining consistent component API design.
157-162
: Great refactoring with thegetTitle
helper function.This helper function elegantly replaces what would have been a nested ternary expression in the JSX, making the code more readable and maintainable. The function clearly handles all flow types: add money, withdraw, send, and request.
191-194
: Clean conditional rendering implementation.Using the new
getTitle
function and updating the conditional logic to exclude both add money and withdraw flows from displaying recipient names is appropriate, as recipient details aren't relevant for these flow types.
204-208
: Appropriate message prefixing for different flows.The message now correctly uses "to" prefix for withdraw flows and "for" prefix for other flows, making the UI text more natural and contextually appropriate based on the flow type.
216-216
: Consistent UI behavior for similar flows.Hiding the "See receipt" button for withdraw flows maintains consistency with how add money flows are handled, keeping the user experience coherent across similar transaction types.
src/components/Withdraw/views/Confirm.withdraw.view.tsx (6)
16-27
: Well-structured props interface for the confirmation view.The
WithdrawConfirmViewProps
interface is comprehensive and properly typed, providing all necessary parameters for a withdrawal confirmation screen including amount, token details, destination address, fees, and appropriate callback handlers.
41-45
: Good use of the token chain icons hook.The component correctly uses the
useTokenChainIcons
hook to retrieve token and chain icon URLs and resolved names, ensuring consistent display of assets across the application.
52-59
: Appropriate reuse of PeanutActionDetailsCard.The component leverages the existing
PeanutActionDetailsCard
with the new 'WITHDRAW' transaction type, maintaining UI consistency while extending functionality for the withdrawal flow.
94-104
: Well-structured payment information display.The information rows for recipient address, network fee, and platform fee are clearly organized and provide appropriate tooltips for additional context. The consistent structure helps users understand the withdrawal details before confirming.
106-115
: Proper button state management.The primary action button correctly handles loading and disabled states based on the
isProcessing
flag, providing clear feedback to the user about the withdrawal status.
117-117
: Appropriate error handling.The component conditionally renders an error alert when an error message is provided, ensuring users are informed of any issues with their withdrawal request.
src/components/Withdraw/views/Initial.withdraw.view.tsx (4)
28-47
: Good use of context for state management.The component appropriately leverages multiple contexts (token selector and withdraw flow) to access and manage token selection, chain selection, and withdrawal flow state, following React best practices for complex state management.
49-64
: Robust validation in handleReview function.The function properly checks that token, chain, and recipient address are all selected and valid before proceeding with the review, displaying appropriate error messaging when validation fails.
91-92
: Clear commenting for future implementation.The comment explaining why the token selector is currently disabled provides good context for future developers who might work on implementing cross-chain support.
109-126
: Comprehensive button state management.The button's disabled state is thoroughly controlled based on multiple validation factors, ensuring users can only proceed when all required information is valid and complete.
src/components/AddWithdraw/components/AddWithdrawCountriesList.tsx (3)
15-32
: Good error handling for invalid country cases.The component appropriately handles cases where the country slug doesn't match any known country by displaying an empty state with a clear message, preventing potential runtime errors.
36-72
: Well-structured helper function for rendering payment methods.The
renderPaymentMethods
function is well-designed to handle the rendering of payment methods based on the provided title and methods array. It correctly handles empty lists and applies appropriate styling based on item position.
16-24
:✅ Verification successful
Verify the typing of params.country.
The component assumes
params.country
could be either a string or an array of strings. This assumption should be verified against the Next.js router implementation to ensure compatibility.
🏁 Script executed:
#!/bin/bash # Check how country parameters are handled in other components rg "const.*params.*country.*Array.isArray" --type tsxLength of output: 83
🏁 Script executed:
#!/bin/bash # Inspect the import and usage of useParams in the target file rg "import.*useParams" -n src/components/AddWithdraw/components/AddWithdrawCountriesList.tsx rg "useParams\(\)" -n src/components/AddWithdraw/components/AddWithdrawCountriesList.tsxLength of output: 276
🌐 Web query:
What is the TypeScript type definition of useParams from next/navigation in Next.js?
💡 Result:
The
useParams
hook in Next.js's App Router allows you to read dynamic route parameters from the current URL in Client Components. Let's explore its TypeScript type definition and how to use it properly.Type Definition and Usage
In Next.js,
useParams
returns an object containing the current route's dynamic parameters. The basic type definition looks like this:function useParams<T extends Record<string, string | string[]>>(): TWhen using
useParams
with TypeScript, you should provide a generic type that defines the structure of your route parameters:const params = useParams<{ slug: string }>() // params will be typed as { slug: string }For routes with multiple parameters, you would define all of them in the type:
const params = useParams<{ categoryId: string; itemId: string }>() // Access with params.categoryId and params.itemIdParameter Types Based on Route Structure
The type you should use for
useParams
depends on your route structure:
Route Pattern TypeScript Type app/blog/[slug]/page.js
{ slug: string }
app/shop/[...slug]/page.js
{ slug: string[] }
app/shop/[[...slug]]/page.js
{ slug?: string[] }
app/[categoryId]/[itemId]/page.js
{ categoryId: string, itemId: string }
Example in Client Components
Here's a complete example of using
useParams
with TypeScript in a client component:'use client' import { useParams } from 'next/navigation' export default function ExampleClientComponent() { // Define the expected parameters based on your route const params = useParams<{ tag: string; item: string }>() // Route -> /shop/[tag]/[item] // URL -> /shop/shoes/nike-air-max-97 // `params` -> { tag: 'shoes', item: 'nike-air-max-97' } return ( <div> <h1>Tag: {params.tag}</h1> <h2>Item: {params.item}</h2> </div> ) }Important Notes
useParams
is a Client Component hook and cannot be used directly in Server Components[1].- If your route has no dynamic parameters,
useParams
returns an empty object ({}
)[1].- The hook does not take any parameters[1].
- For optional catch-all routes (
[[...slug]]
), you should use a type with optional parameters ({ slug?: string[] }
)[7].- Unlike
useSearchParams
,useParams
values are always strings or string arrays, nevernull
orundefined
for defined route parameters[7].By properly typing your params, you get full TypeScript validation, which helps prevent errors when accessing route parameters in your components.
Citations:
- 1: https://nextjs.org/docs/app/api-reference/functions/use-params
- 2: vercel/next.js#46131
- 3: https://dev.to/javila35/react-router-hook-useparam-now-w-typescript-m93
- 4: https://nextjs.org/docs/app/api-reference/config/typescript
- 5: https://nextjs.org/docs/app/api-reference/functions/use-search-params
- 6: https://reactnavigation.org/docs/typescript/
- 7: https://nextjs.org/docs/app/building-your-application/routing/dynamic-routes
- 8: https://www.flightcontrol.dev/blog/fix-nextjs-routing-to-have-full-type-safety
- 9: https://dev.to/peterlidee/using-searchparams-usesearchparams-and-userouter-in-next-13-4623
- 10: remix-run/react-router#8200
params.country typing verified
The Next.js App Router
useParams
hook is typed asRecord<string, string | string[]>
, soparams.country
may indeed be astring
orstring[]
. Your runtime check withArray.isArray
correctly handles both cases. No changes required.src/components/AddWithdraw/components/AddWithdrawRouterView.tsx (1)
55-65
: 🛠️ Refactor suggestionSearch results can mutate original data – may break future look-ups
By overwriting
title
anddescription
for crypto items you lose the original values, which means:
- Consecutive searches in the same render cycle now use the mutated string – “Crypto Deposit” will never match a subsequent search for the real token/currency name.
- Down-stream consumers that rely on the canonical
title
no longer receive it.Instead, return a new object for the override:
- if (method.type === 'crypto') { - return { - ...method, - title: …, - description: …, - } - } - return method + if (method.type === 'crypto') + return { + ...method, + title: flow === 'add' ? 'Crypto Deposit' : 'Crypto', + description: + flow === 'add' + ? 'Use an exchange or your wallet' + : 'Withdraw to a wallet or exchange', + } + return { ...method }This keeps the original array immutable.
Likely an incorrect or invalid review comment.
src/components/AddMoney/consts/index.ts (1)
1851-1854
: Slug contains dots – verify routing behaviour
path: 'u.s.-minor-outlying-islands'
includes two dots.
Next.js treats.
literally, but some tooling (and browsers when used as file paths) might misinterpret it as a file extension separator. Consider usingus-minor-outlying-islands
or similar.Double-check that all slugs conform to your routing conventions.
} else if (entry.userRole === EHistoryUserRole.RECIPIENT) { | ||
direction = 'receive' | ||
transactionCardType = 'add' | ||
transactionCardType = 'receive' |
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.
Fixed transaction card type for recipient role.
For SEND_LINK
transactions where the user role is RECIPIENT
, the transaction card type was corrected from 'add'
to 'receive'
. This ensures the transaction is displayed with the correct visual treatment in the UI.
This is an important fix as it ensures the transaction is correctly categorized and displayed to the user, improving the clarity of their transaction history.
🤖 Prompt for AI Agents
In src/components/TransactionDetails/transactionTransformer.ts at line 112,
update the transactionCardType assignment for SEND_LINK transactions when the
user role is RECIPIENT from 'add' to 'receive'. This change correctly
categorizes the transaction for proper UI display by setting transactionCardType
to 'receive' instead of 'add'.
isOpen={isDrawerOpen && selectedTransaction?.id === transactionForDrawer?.id} | ||
onClose={() => { | ||
router.push('/home') | ||
}} | ||
transaction={selectedTransaction} | ||
/> |
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.
🛠️ Refactor suggestion
Drawer close handler doesn’t clear drawer state
onClose
only does router.push('/home')
; it never invokes closeTransactionDetails
, leaving the drawer marked as open in context. This can surface later as a ghost-open drawer when the user re-enters the view.
onClose={() => {
+ closeTransactionDetails()
router.push('/home')
}}
Call the context close helper first (or in useEffect
on route change) to keep the global drawer state consistent.
📝 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.
isOpen={isDrawerOpen && selectedTransaction?.id === transactionForDrawer?.id} | |
onClose={() => { | |
router.push('/home') | |
}} | |
transaction={selectedTransaction} | |
/> | |
isOpen={isDrawerOpen && selectedTransaction?.id === transactionForDrawer?.id} | |
onClose={() => { | |
closeTransactionDetails() | |
router.push('/home') | |
}} | |
transaction={selectedTransaction} | |
/> |
🤖 Prompt for AI Agents
In src/components/Claim/Claim.tsx around lines 281 to 286, the onClose handler
for the drawer only triggers router.push('/home') but does not call
closeTransactionDetails to clear the drawer state. Update the onClose handler to
first invoke closeTransactionDetails to reset the drawer state before
navigating, ensuring the drawer state remains consistent and preventing
ghost-open drawer issues.
let status: StatusType | ||
switch (claimLinkData.status) { | ||
case ESendLinkStatus.creating: | ||
case ESendLinkStatus.completed: | ||
status = 'pending' | ||
break | ||
case ESendLinkStatus.CLAIMING: | ||
status = 'processing' | ||
break | ||
case ESendLinkStatus.CLAIMED: | ||
status = 'completed' | ||
break | ||
case ESendLinkStatus.CANCELLED: | ||
status = 'cancelled' | ||
break | ||
case ESendLinkStatus.FAILED: | ||
status = 'failed' | ||
break | ||
default: | ||
status = 'pending' | ||
break | ||
} |
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.
Incorrect status mapping for completed
links
ESendLinkStatus.completed
is currently mapped to 'pending'
, which will mis-label completed transactions in the drawer UI. This will confuse users and may break any logic that relies on the completed
badge.
- switch (claimLinkData.status) {
- case ESendLinkStatus.creating:
- case ESendLinkStatus.completed:
- status = 'pending'
+ switch (claimLinkData.status) {
+ case ESendLinkStatus.creating:
+ status = 'pending'
+ break
+ case ESendLinkStatus.completed:
+ status = 'completed'
While you are here, double-check whether ESendLinkStatus.creating
should be 'processing'
instead of 'pending'
to give better feedback.
📝 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.
let status: StatusType | |
switch (claimLinkData.status) { | |
case ESendLinkStatus.creating: | |
case ESendLinkStatus.completed: | |
status = 'pending' | |
break | |
case ESendLinkStatus.CLAIMING: | |
status = 'processing' | |
break | |
case ESendLinkStatus.CLAIMED: | |
status = 'completed' | |
break | |
case ESendLinkStatus.CANCELLED: | |
status = 'cancelled' | |
break | |
case ESendLinkStatus.FAILED: | |
status = 'failed' | |
break | |
default: | |
status = 'pending' | |
break | |
} | |
let status: StatusType | |
switch (claimLinkData.status) { | |
case ESendLinkStatus.creating: | |
status = 'pending' | |
break | |
case ESendLinkStatus.completed: | |
status = 'completed' | |
break | |
case ESendLinkStatus.CLAIMING: | |
status = 'processing' | |
break | |
case ESendLinkStatus.CLAIMED: | |
status = 'completed' | |
break | |
case ESendLinkStatus.CANCELLED: | |
status = 'cancelled' | |
break | |
case ESendLinkStatus.FAILED: | |
status = 'failed' | |
break | |
default: | |
status = 'pending' | |
break | |
} |
🤖 Prompt for AI Agents
In src/components/Claim/Claim.tsx between lines 71 and 92, the status mapping
incorrectly assigns ESendLinkStatus.completed to 'pending', which mislabels
completed transactions. Change the mapping for ESendLinkStatus.completed to
'completed' to reflect the correct status. Also, update ESendLinkStatus.creating
to map to 'processing' instead of 'pending' to provide more accurate user
feedback.
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: 3
♻️ Duplicate comments (1)
src/components/AddMoney/components/DepositMethodList.tsx (1)
67-67
: Improved key generation with stable identifiersThe composite key using both type and ID is a good improvement over using array indices, ensuring stable React keys even if the methods array is reordered.
🧹 Nitpick comments (8)
src/app/(mobile-ui)/withdraw/page.tsx (4)
69-69
: Consider aligning button disable logic with validation logicThe button disable condition only checks if the amount is positive, while your validation function also checks if the amount is finite and within balance limits. For consistency, consider using the same validation logic:
- disabled={!parseFloat(rawTokenAmount) || parseFloat(rawTokenAmount) <= 0} + disabled={!Number.isFinite(Number(rawTokenAmount)) || Number(rawTokenAmount) <= 0 || Number(rawTokenAmount) > Number(formatUnits(balance, PEANUT_WALLET_TOKEN_DECIMALS))}Alternatively, you could extract the validation into a separate function to reuse in both places.
35-43
: Consider validating decimal precisionWhile your validation checks for valid numbers and balance limits, it doesn't validate that the input respects token decimal precision. Consider adding this check to prevent potential issues with precision loss.
if (Number.isFinite(amount) && amount > 0 && amount <= max) { + // Check decimal precision + const decimalPart = rawTokenAmount.includes('.') ? rawTokenAmount.split('.')[1] : ''; + if (decimalPart.length > PEANUT_WALLET_TOKEN_DECIMALS) { + setError({ + showError: true, + errorMessage: `Please enter an amount with up to ${PEANUT_WALLET_TOKEN_DECIMALS} decimal places.`, + }); + return; + } setAmountToWithdraw(rawTokenAmount) setStep('selectMethod') } else {
74-74
: Redundant error message checkThe condition
error.showError && !!error.errorMessage
contains a redundant check. If your error state management ensures thaterror.errorMessage
is always set whenerror.showError
is true, you can simplify this to:- {error.showError && !!error.errorMessage && <ErrorAlert description={error.errorMessage} />} + {error.showError && <ErrorAlert description={error.errorMessage} />}
58-75
: Consider adding accessibility attributes for error statesTo improve accessibility, add ARIA attributes to notify screen readers when errors occur:
<div className="my-auto flex flex-grow flex-col justify-center gap-4 md:my-0"> <div className="text-sm font-bold">Amount to withdraw</div> <TokenAmountInput tokenValue={rawTokenAmount} setTokenValue={handleTokenAmountChange} walletBalance={peanutWalletBalance} + aria-invalid={error.showError} + aria-describedby={error.showError ? "withdraw-error" : undefined} /> <Button variant="purple" shadowSize="4" onClick={handleAmountContinue} disabled={!parseFloat(rawTokenAmount) || parseFloat(rawTokenAmount) <= 0} className="w-full" > Continue </Button> - {error.showError && !!error.errorMessage && <ErrorAlert description={error.errorMessage} />} + {error.showError && !!error.errorMessage && <ErrorAlert id="withdraw-error" role="alert" description={error.errorMessage} />} </div>src/components/AddMoney/consts/index.ts (1)
2007-2014
: RedundantcountrySpecificWithdrawMethods['Germany']
check
countrySpecificWithdrawMethods['Germany']
is constant and always truthy as long as the lookup table remains unchanged; the extra condition just obscures intent.- if (country.currency === 'EUR' && countrySpecificWithdrawMethods['Germany']) { + if (country.currency === 'EUR') {Simplifying the condition makes the fallback easier to read and maintain.
src/components/AddMoney/components/DepositMethodList.tsx (3)
38-59
: Consider extracting position determination logicThe card positioning logic is quite complex and would benefit from being extracted into a helper function for better readability and maintainability.
+ const getCardPosition = (method: DepositMethod, index: number, methods: DepositMethod[]): CardPosition => { + const isFirstOverall = index === 0 + const isLastOverall = index === methods.length - 1 + const isSingleOverall = methods.length === 1 + const isCryptoAtSlot0 = methods[0]?.type === 'crypto' + const isCurrentMethodCountry = method.type === 'country' + + if (isSingleOverall) { + return 'single' + } else if (isFirstOverall) { + return 'first' + } else if (isCryptoAtSlot0 && isCurrentMethodCountry && index === 1) { + // if crypto card is at methods[0], and this is the country card at methods[1], + // treat this country card as 'first' in its own group. + return 'first' + } else if (isLastOverall) { + return 'last' + } else { + return 'middle' + } + } {methods.map((method, index) => { - let determinedPosition: CardPosition - const isFirstOverall = index === 0 - const isLastOverall = index === methods.length - 1 - const isSingleOverall = methods.length === 1 - - const isCryptoAtSlot0 = methods[0]?.type === 'crypto' - const isCurrentMethodCountry = method.type === 'country' - - if (isSingleOverall) { - determinedPosition = 'single' - } else if (isFirstOverall) { - determinedPosition = 'first' - } else if (isCryptoAtSlot0 && isCurrentMethodCountry && index === 1) { - // if crypto card is at methods[0], and this is the country card at methods[1], - // treat this country card as 'first' in its own group. - determinedPosition = 'first' - } else if (isLastOverall) { - determinedPosition = 'last' - } else { - determinedPosition = 'middle' - } + const determinedPosition = getCardPosition(method, index, methods)
60-63
: Simplify class name handling with twMergeThe current implementation with array and join is unnecessary since twMerge can directly handle conditional classes.
- const classNames = [] - if (method.type === 'crypto') { - classNames.push('mb-2') - } - return ( <SearchResultCard key={`${method.type}-${method.id}`} title={method.title} description={method.description || method.currency} leftIcon={/* ... */} onClick={() => handleMethodClick(method)} position={determinedPosition} - className={twMerge(classNames.join(' '))} + className={twMerge(method.type === 'crypto' ? 'mb-2' : '')}
71-85
: Consider extracting icon rendering to a helper functionThe nested conditional rendering for the leftIcon is quite complex and would be more readable as a separate helper function.
+ const renderMethodIcon = (method: DepositMethod) => { + if (method.type === 'crypto') { + return <AvatarWithBadge icon="wallet-outline" size="extra-small" className="bg-yellow-1" /> + } + + if (method.type === 'country') { + return ( + <Image + src={`https://flagcdn.com/w320/${method.id.toLowerCase()}.png`} + alt={`${method.title} flag`} + width={32} + height={32} + className="min-h-8 min-w-8 rounded-full object-fill object-center shadow-sm" + loading="lazy" + /> + ) + } + + return <AvatarWithBadge name={method.title} size="extra-small" className="bg-yellow-1" /> + } <SearchResultCard key={`${method.type}-${method.id}`} title={method.title} description={method.description || method.currency} - leftIcon={ - method.type === 'crypto' ? ( - <AvatarWithBadge icon="wallet-outline" size="extra-small" className="bg-yellow-1" /> - ) : method.type === 'country' ? ( - <Image - src={`https://flagcdn.com/w320/${method.id.toLowerCase()}.png`} - alt={`${method.title} flag`} - width={32} - height={32} - className="min-h-8 min-w-8 rounded-full object-fill object-center shadow-sm" - loading="lazy" - /> - ) : ( - <AvatarWithBadge name={method.title} size="extra-small" className="bg-yellow-1" /> - ) - } + leftIcon={renderMethodIcon(method)}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
src/assets/payment-apps/apple-pay.svg
is excluded by!**/*.svg
src/assets/payment-apps/google-pay.svg
is excluded by!**/*.svg
src/assets/payment-apps/mercado-pago.svg
is excluded by!**/*.svg
src/assets/payment-apps/paypal.svg
is excluded by!**/*.svg
src/assets/payment-apps/satispay.svg
is excluded by!**/*.svg
📒 Files selected for processing (22)
src/app/(mobile-ui)/add-money/crypto/direct/page.tsx
(1 hunks)src/app/(mobile-ui)/add-money/crypto/page.tsx
(2 hunks)src/app/(mobile-ui)/withdraw/layout.tsx
(1 hunks)src/app/(mobile-ui)/withdraw/page.tsx
(1 hunks)src/app/[...recipient]/client.tsx
(2 hunks)src/assets/index.ts
(1 hunks)src/assets/payment-apps/index.ts
(1 hunks)src/assets/peanut/index.ts
(0 hunks)src/components/0_Bruddle/Button.tsx
(1 hunks)src/components/AddMoney/components/CryptoSourceListCard.tsx
(2 hunks)src/components/AddMoney/components/DepositMethodList.tsx
(2 hunks)src/components/AddMoney/consts/index.ts
(2 hunks)src/components/AddMoney/views/CryptoDepositQR.view.tsx
(1 hunks)src/components/AddMoney/views/NetworkSelection.view.tsx
(0 hunks)src/components/AddMoney/views/TokenSelection.view.tsx
(2 hunks)src/components/AddWithdraw/components/AddWithdrawCountriesList.tsx
(1 hunks)src/components/Global/Card/index.tsx
(2 hunks)src/components/Home/HomeHistory.tsx
(3 hunks)src/components/Payment/PaymentForm/index.tsx
(4 hunks)src/components/SearchUsers/SearchResultCard.tsx
(3 hunks)src/components/Withdraw/views/Initial.withdraw.view.tsx
(1 hunks)src/context/WithdrawFlowContext.tsx
(1 hunks)
💤 Files with no reviewable changes (2)
- src/assets/peanut/index.ts
- src/components/AddMoney/views/NetworkSelection.view.tsx
✅ Files skipped from review due to trivial changes (3)
- src/app/[...recipient]/client.tsx
- src/components/AddMoney/views/TokenSelection.view.tsx
- src/assets/payment-apps/index.ts
🚧 Files skipped from review as they are similar to previous changes (9)
- src/assets/index.ts
- src/app/(mobile-ui)/add-money/crypto/direct/page.tsx
- src/app/(mobile-ui)/withdraw/layout.tsx
- src/components/Payment/PaymentForm/index.tsx
- src/components/Global/Card/index.tsx
- src/app/(mobile-ui)/add-money/crypto/page.tsx
- src/components/Withdraw/views/Initial.withdraw.view.tsx
- src/components/AddWithdraw/components/AddWithdrawCountriesList.tsx
- src/context/WithdrawFlowContext.tsx
🧰 Additional context used
🧬 Code Graph Analysis (3)
src/components/Home/HomeHistory.tsx (1)
src/components/Global/Card/index.tsx (1)
getCardPosition
(14-19)
src/components/AddMoney/consts/index.ts (1)
src/components/Global/Icons/Icon.tsx (1)
IconName
(49-94)
src/components/AddMoney/components/DepositMethodList.tsx (2)
src/components/Global/Card/index.tsx (1)
CardPosition
(4-4)src/components/SearchUsers/SearchResultCard.tsx (1)
SearchResultCard
(19-66)
🔇 Additional comments (16)
src/components/AddMoney/views/CryptoDepositQR.view.tsx (1)
23-24
: Great improvement on timeout cleanup!This change properly manages component lifecycle by returning a cleanup function that clears the timeout when the component unmounts or when dependencies change. This prevents potential memory leaks and stale state updates that could occur if the component unmounts before the timeout completes.
src/components/0_Bruddle/Button.tsx (1)
79-79
: Improved default behavior for icon positioning.Setting a default value for
iconPosition
ensures consistent icon placement when the prop is not explicitly provided. This change aligns with similar updates in other components likeActionModal
, creating a standardized UI behavior across the application.src/components/Home/HomeHistory.tsx (3)
11-12
: Good modularization by importing the utility function.The new import of
getCardPosition
from the Card component will help standardize card position determination across the codebase, improving consistency and maintainability.
100-100
: Clean refactoring using the utility function.Good replacement of manual position logic with the centralized
getCardPosition
utility function, which makes the code more maintainable and consistent with other components.
140-140
: Consistent implementation of card position logic.Excellent use of the
getCardPosition
utility function here as well, ensuring consistency throughout the component and aligning with the same pattern used in other components across the codebase.src/components/AddMoney/components/CryptoSourceListCard.tsx (3)
12-18
: Good component renaming for better semantic clarity.Renaming from
Icon
toGenericIcon
improves code readability by clearly indicating this component's specific purpose in rendering generic wallet/exchange icons. This aligns well with the broader refactoring of icon handling across the codebase.
38-38
: Correctly updated component reference.The usage of
GenericIcon
here properly reflects the component renaming, maintaining consistency throughout the file.
40-40
: Improved visual consistency with unified styling.Changing the background color from gray to yellow (
bg-yellow-1
) creates visual consistency between the generic and non-generic icon representations. This subtle UI improvement helps maintain a cohesive look across different icon states.src/app/(mobile-ui)/withdraw/page.tsx (3)
31-44
: Great improvement on validation logic!You've addressed the previous review feedback by implementing proper validation that checks for finite numbers, positive values, and balance limits. The error handling has also been improved by replacing
alert()
with theErrorAlert
component.
46-52
: Good practice using useCallback for event handlersUsing
useCallback
for the token amount change handler is a good practice for optimization, especially for functions passed to child components.
80-90
: Good component reuse across flowsUsing the
AddWithdrawRouterView
component for both add and withdraw flows with different props is a great example of component reuse.src/components/AddMoney/consts/index.ts (1)
2023-2026
: Per-country.map()
cloning works – nice defensive copyGood job addressing the earlier cross-country mutation issue by cloning
UPDATED_DEFAULT_ADD_MONEY_METHODS
for every country. 👍src/components/SearchUsers/SearchResultCard.tsx (2)
16-16
: Good addition of custom styling flexibilityAdding the
descriptionClassName
prop enhances the component's versatility by allowing consumers to customize the description's appearance while preserving default styling.
45-47
: Well-implemented styling with twMergeUsing
twMerge
to combine default classes with custom ones is the correct approach to prevent Tailwind class conflicts. This implementation maintains the base styling while allowing for customization.src/components/AddMoney/components/DepositMethodList.tsx (2)
21-21
: Good addition of country-specific click handlerAdding the optional
onCountryClick
callback enhances component flexibility by allowing custom handling of country selections while maintaining backward compatibility.
27-33
: Well-structured method click handlerThe updated click handler correctly distinguishes between country and non-country methods, providing a clean separation of concerns for different navigation paths.
withdrawList.push({ | ||
id: `${countryCode.toLowerCase()}-${method.title.toLowerCase()}-withdraw`, | ||
icon: method.icon as IconName, | ||
title: method.title, | ||
description: method.description, | ||
isSoon: true, | ||
}) |
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.
🛠️ Refactor suggestion
Generated IDs contain spaces → fragile for look-ups & URL slugs
"${countryCode.toLowerCase()}-${method.title.toLowerCase()}-withdraw"
keeps the spaces from method.title
(e.g. "us-cash app-withdraw"
).
If these IDs are later used as DOM ids, query keys, or parts of URLs they will break.
Quick fix:
- id: `${countryCode.toLowerCase()}-${method.title.toLowerCase()}-withdraw`,
+ id: `${countryCode.toLowerCase()}-${method.title.toLowerCase().replace(/\s+/g, '-')}-withdraw`,
Consider extracting a small slugify()
helper so the same logic is reused everywhere.
📝 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.
withdrawList.push({ | |
id: `${countryCode.toLowerCase()}-${method.title.toLowerCase()}-withdraw`, | |
icon: method.icon as IconName, | |
title: method.title, | |
description: method.description, | |
isSoon: true, | |
}) | |
withdrawList.push({ | |
- id: `${countryCode.toLowerCase()}-${method.title.toLowerCase()}-withdraw`, | |
+ id: `${countryCode.toLowerCase()}-${method.title.toLowerCase().replace(/\s+/g, '-')}-withdraw`, | |
icon: method.icon as IconName, | |
title: method.title, | |
description: method.description, | |
isSoon: true, | |
}) |
🤖 Prompt for AI Agents
In src/components/AddMoney/consts/index.ts around lines 1998 to 2004, the
generated IDs include spaces from method.title, which can cause issues in DOM
ids, query keys, or URLs. Fix this by creating a slugify() helper function that
converts strings to lowercase, replaces spaces and special characters with
hyphens or removes them, and use this helper to transform method.title before
concatenating it into the ID. This ensures consistent, safe IDs without spaces
or invalid characters.
id: string | ||
icon: IconName | string | ||
title: string | ||
description: string | ||
isSoon?: boolean | ||
path?: string | ||
} | ||
|
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.
icon
union type still too narrow – will break when using imported image assets
SpecificPaymentMethod.icon
was widened from string
to IconName | string
, but the default lists below pass StaticImageData
values (APPLE_PAY
, GOOGLE_PAY
, etc.).
Because StaticImageData
is not assignable to IconName | string
, the file will fail to compile (TS2322
).
-export interface SpecificPaymentMethod {
- id: string
- icon: IconName | string
+export type IconSrc = IconName | StaticImageData | string
+
+export interface SpecificPaymentMethod {
+ id: string
+ /** Accepts an inline IconName, a Next-Image StaticImageData, or a remote URL */
+ icon: IconSrc
This also brings the type in line with CryptoToken
/ CryptoSource
, eliminating the need for as
-casts scattered elsewhere.
📝 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 interface SpecificPaymentMethod { | |
id: string | |
icon: IconName | string | |
title: string | |
description: string | |
isSoon?: boolean | |
path?: string | |
} | |
// add this alias at the top of the file (near other shared types/imports) | |
export type IconSrc = IconName | StaticImageData | string | |
export interface SpecificPaymentMethod { | |
id: string | |
/** Accepts an inline IconName, a Next-Image StaticImageData, or a remote URL */ | |
icon: IconSrc | |
title: string | |
description: string | |
isSoon?: boolean | |
path?: string | |
} |
🤖 Prompt for AI Agents
In src/components/AddMoney/consts/index.ts around lines 118 to 125, the type of
the `icon` property in the `SpecificPaymentMethod` interface is currently
`IconName | string`, which causes TypeScript errors when assigning
`StaticImageData` values. To fix this, expand the union type of `icon` to
include `StaticImageData` so it can accept imported image assets without type
errors. This change will also align the type with similar interfaces like
`CryptoToken` and `CryptoSource`, reducing the need for type assertions
elsewhere.
{ | ||
id: 'bank-transfer-add', | ||
icon: 'bank' as IconName, | ||
title: 'From Bank', | ||
description: '1 to 3 working days - KYC required', | ||
isSoon: true, | ||
}, | ||
{ | ||
id: 'mercado-pago-add', | ||
icon: MERCADO_PAGO, | ||
title: 'Mercado Pago', | ||
description: 'Popular in LATAM', | ||
isSoon: true, | ||
}, | ||
{ | ||
id: 'apple-pay-add', | ||
icon: APPLE_PAY, | ||
title: 'Apple Pay', | ||
description: 'Usually arrives instantly', | ||
isSoon: true, | ||
}, | ||
{ | ||
id: 'google-pay-add', | ||
icon: GOOGLE_PAY, | ||
title: 'Google Pay', | ||
description: 'Usually arrives instantly', | ||
isSoon: true, | ||
}, | ||
] | ||
|
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.
🛠️ Refactor suggestion
Assignments here violate the interface & require unsafe casts
Until the interface above is fixed, every icon: APPLE_PAY
/ icon: GOOGLE_PAY
etc. will need a type assertion, polluting the code.
After adopting the broader IconSrc
union, all the as IconName
workarounds can be removed:
- icon: MERCADO_PAGO,
+ icon: MERCADO_PAGO,
(no cast required).
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/components/AddMoney/consts/index.ts between lines 145 and 174, the icon
properties for APPLE_PAY, GOOGLE_PAY, and similar are assigned without proper
type assertions, violating the interface and requiring unsafe casts. To fix
this, update the interface to use the broader IconSrc union type for the icon
property, which will allow these assignments without needing type assertions.
After this interface change, remove all 'as IconName' casts from the icon
assignments in this file.
<div className="space-y-4"> | ||
<ActionButtonGroup> | ||
<ActionButtonWithHref label="Add money" action="add" href="/add-money" size="small" /> | ||
<ActionButtonWithHref label="Withdraw" action="withdraw" href="/witdraw" size="small" /> |
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.
oof I missed that
Merging into feature branch |
contributes to TASK-11025