Conversation
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: Kushagra Sarathe <76868364+kushagrasarathe@users.noreply.github.com>
fix: set default currency in input to local for manteca countries
feat: contacts frontend updates
fix: use server action for contacts api and clean types
Add header to OnrampConfirmationModal
…/peanut-ui into peanut-wallet-dev
fix: always redirect to add-money flow for bank/mepa and pix request fulfillments
[TASK-16583] feat: sign user operation and send it for QR payments instead of sending
…-chain Feat: Add warning modal for cross-chain deposits
…d updating CardProps ref type
Update points card and add empty state
feat: offline caching
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThis PR introduces significant infrastructure improvements and new features including service worker integration with offline support, a new contacts management system, enhanced payment flow resilience with DevConnect intent persistence, network retry strategies, and performance optimizations through lazy loading and improved caching. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas requiring extra attention:
Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review for possible bugs or unexpected behaviour |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 273661 🏁 Script executed: Length of output: 2293 🏁 Script executed: Length of output: 9998 🏁 Script executed: Length of output: 7026 @Hugo0 here’s a focused prod-readiness sweep of PR #1482 (ran shell checks on the branch). I’ve grouped the findings by severity and included minimal diffs you can drop in. Must-fix before prod
File: src/components/Global/Card/index.tsx import React, { forwardRef } from 'react'
import { twMerge } from 'tailwind-merge'
type CardPosition = 'single' | 'first' | 'middle' | 'last'
interface CardProps {
children: React.ReactNode
position?: CardPosition
className?: string
onClick?: () => void
border?: boolean
}
export function getCardPosition(index: number, totalItems: number): CardPosition {
if (totalItems === 1) return 'single'
if (index === 0) return 'first'
if (index === totalItems - 1) return 'last'
return 'middle'
}
const Card = forwardRef<HTMLDivElement, CardProps>(function Card(
{ children, position = 'single', className = '', onClick, border = true },
ref
) {
const getBorderRadius = () => {
switch (position) {
case 'single':
return 'rounded-2xl'
case 'first':
return 'rounded-t-2xl'
case 'middle':
return ''
case 'last':
return 'rounded-b-2xl'
}
}
const getBorder = () => (border ? 'border border-black' : '')
return (
<div
ref={ref}
className={twMerge('w-full bg-white px-4 py-2', getBorderRadius(), getBorder(), className)}
onClick={onClick}
>
{children}
</div>
)
})
export default Card
File: src/app/layout.tsx (replace the updatefound handler) {process.env.NODE_ENV !== 'development' && (
<Script id="sw-registration" strategy="beforeInteractive">
{`
if ('serviceWorker' in navigator) {
window.__blockSwReload = false; // flip to true during critical ops
window.addEventListener('load', async () => {
try {
const registration = await navigator.serviceWorker.register('/sw.js', {
scope: '/',
updateViaCache: 'none'
});
registration.addEventListener('updatefound', () => {
const newWorker = registration.installing;
if (newWorker) {
newWorker.addEventListener('statechange', () => {
const shouldReload = newWorker.state === 'installed' && navigator.serviceWorker.controller && !window.__blockSwReload;
if (shouldReload) {
console.log('New SW available, reloading...');
window.location.reload();
} else if (newWorker.state === 'installed') {
// Defer reload until the app flips the flag back
console.log('New SW available, deferring reload until idle.');
}
});
}
});
} catch (error) {
console.error('SW registration failed:', error);
}
});
}
`}
</Script>
)}
File: src/app/(mobile-ui)/qr-pay/page.tsx (add near other refs) const payingTimeoutRef = useRef<number | null>(null)
useEffect(() => () => { if (payingTimeoutRef.current) clearTimeout(payingTimeoutRef.current) }, [])Replace scheduling + cleanup: // before sending to backend
if (payingTimeoutRef.current) clearTimeout(payingTimeoutRef.current)
payingTimeoutRef.current = window.setTimeout(() => setLoadingState('Paying'), 3000)
try {
// ...
} catch (error) {
// ...
} finally {
if (payingTimeoutRef.current) {
clearTimeout(payingTimeoutRef.current)
payingTimeoutRef.current = null
}
setLoadingState('Idle')
}
File: src/app/(mobile-ui)/qr-pay/page.tsx (QrPayPageLoading component) <Image
src={PeanutGuyGIF}
alt="Peanut Man"
fill
style={{ objectFit: 'contain' }}
className="absolute z-0 h-32 w-32 -translate-y-20"
/>File: src/components/Global/PeanutFactsLoading/index.tsx <Image src={PeanutGuyGIF} alt="Peanut Man" fill style={{ objectFit: 'contain' }} />
File: next.config.js value: 'camera=(self), microphone=(self), clipboard-read=(self), clipboard-write=(self)',
Should-fix soon (low risk, prevents regressions)
Option A (quick): reuse constants everywhere import { MIN_MANTECA_QR_PAYMENT_AMOUNT as MIN_QR_PAYMENT_AMOUNT } from '@/constants/payment.consts'
// remove local const MIN_QR_PAYMENT_AMOUNTOption B (better): add export const MIN_QR_PAYMENT_AMOUNT = 0.1 to payment.consts.ts and import it in the page.
File: src/components/Setup/Views/Landing.tsx import { useAuth } from '@/context/authContext'
import { getRedirectUrl, sanitizeRedirectURL, clearRedirectUrl } from '@/utils'
import { useRouter, useSearchParams } from 'next/navigation'
import { useEffect } from 'react'
const { user } = useAuth()
const router = useRouter()
const searchParams = useSearchParams()
useEffect(() => {
if (!user?.user) return
const queryRedirect = searchParams.get('redirect_uri')
const localRedirect = getRedirectUrl()
const target = queryRedirect ?? localRedirect ?? '/home'
const safe = sanitizeRedirectURL(target) || '/home'
if (localRedirect) clearRedirectUrl()
router.replace(safe)
}, [user, router, searchParams])Nice-to-have polish
What I checked (traceable)
Want me to open follow‑up issues for the non-blockers (6–8) and leave the must‑fix items as blocking comments? I can also push a PR with the forwardRef + timeouts + next/image + header fixes if you prefer. 🧠 Learnings used |
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
src/components/Profile/components/ProfileHeader.tsx (1)
32-32: Remove unused state variable.The
isDrawerOpenstate is no longer used after the Drawer removal.Apply this diff:
- const [isDrawerOpen, setIsDrawerOpen] = useState(false)src/components/Setup/Views/InstallPWA.tsx (1)
45-62: Add error handling around the API call.While the existence check on line 56 prevents calling an undefined method, the
getInstalledRelatedApps()call itself can still throw errors (e.g., permission denied, network issues). Since this is an async function without error handling, any such errors would fail silently, potentially leavingisPWAInstalledin an incorrect state.Apply this diff to add error handling:
const checkInstallation = async () => { const pwaInstalledByMedia = window.matchMedia('(display-mode: standalone)').matches if (pwaInstalledByMedia) { setIsPWAInstalled(true) return } const _navigator = window.navigator as Navigator & { getInstalledRelatedApps: () => Promise< { platform: string; url?: string; id?: string; version?: string }[] > } - const installedApps = _navigator.getInstalledRelatedApps ? await _navigator.getInstalledRelatedApps() : [] - if (installedApps.length > 0) { - setIsPWAInstalled(true) - } else { - setIsPWAInstalled(false) + try { + const installedApps = _navigator.getInstalledRelatedApps ? await _navigator.getInstalledRelatedApps() : [] + if (installedApps.length > 0) { + setIsPWAInstalled(true) + } else { + setIsPWAInstalled(false) + } + } catch (error) { + console.error('Error checking installed apps:', error) + captureException(error) + setIsPWAInstalled(false) } }src/app/manifest.ts (1)
27-60: Update service worker icon reference to beta version.The comment on lines 3-5 explicitly requests keeping icons consistent across files. While manifest.ts and actions.ts have been updated to use beta icons, sw.ts still references the old non-beta icon at line 35:
icon: '/icons/icon-192x192.png',Change this to:
icon: '/icons/icon-192x192-beta.png',src/app/[...recipient]/client.tsx (1)
625-630: Fix typo in default error messageMinor copy issue:
- message: 'They payment you are trying to access is invalid. Please check the URL and try again.', + message: 'The payment you are trying to access is invalid. Please check the URL and try again.',src/components/Payment/PaymentForm/index.tsx (1)
169-221: External ADDRESS/ENS flows: default token may never be set when chain is missingIn this
useEffect, for external recipients with nochainin the URL you:
- Set
selectedChainIDtoPEANUT_WALLET_CHAIN.idin theelse if (isExternalRecipient && !selectedChainID)branch.- Then rely on
selectedChainIDin the laterelse if (isExternalRecipient && !selectedTokenAddress && selectedChainID)branch to default the token to USDC.Because React state updates are async,
selectedChainIDis still empty during that same effect run. At the end you setinitialSetupDone(true), so on the next render the early return short‑circuits the effect and the default token is never applied. Users in this case see Arbitrum selected but no token, and must manually pick USDC.A minimal, safe fix is to extract the “default token for external recipients” into a separate effect that doesn’t depend on
initialSetupDone:useEffect(() => { // ...existing logic up to setInitialSetupDone(true) setInitialSetupDone(true) -}, [ - chain, - token, - amount, - initialSetupDone, - requestDetails, - showRequestPotInitialView, - isRequestPotLink, - recipient?.recipientType, - selectedChainID, - selectedTokenAddress, - supportedSquidChainsAndTokens, -]) +}, [ + chain, + token, + amount, + initialSetupDone, + requestDetails, + showRequestPotInitialView, + isRequestPotLink, + recipient?.recipientType, +]) + +// Ensure external ADDRESS/ENS recipients always get a default USDC token once chain is known +useEffect(() => { + const isExternalRecipient = + recipient?.recipientType === 'ADDRESS' || recipient?.recipientType === 'ENS' + + if (!isExternalRecipient || !selectedChainID || selectedTokenAddress) return + + const chainData = supportedSquidChainsAndTokens[selectedChainID] + const defaultToken = chainData?.tokens.find((t) => t.symbol.toLowerCase() === 'usdc') + if (defaultToken) { + setSelectedTokenAddress(defaultToken.address) + } +}, [recipient?.recipientType, selectedChainID, selectedTokenAddress, supportedSquidChainsAndTokens])This keeps initial setup semantics while guaranteeing a USDC default once the chain is resolved.
src/app/(mobile-ui)/qr-pay/page.tsx (1)
616-704: Delayed “Paying” loading state can override success and leave a stuck spinnerIn
handleMantecaPaymentyou schedule a delayed loading state:setTimeout(() => setLoadingState('Paying'), 3000)while also:
- Setting
setLoadingState('Preparing transaction')earlier.- Setting
setLoadingState('Idle')in thefinallyblock.- Having a separate effect that sets
loadingStateto'Idle'wheneverisSuccessbecomes true.If the backend call resolves quickly (success or failure) before the 3‑second timeout:
setLoadingState('Idle')runs from the try/finally and/or theisSuccesseffect.- After 3 seconds, the timeout fires and sets
loadingState('Paying')again.- The render short‑circuits at the
if (isLoadingPaymentData || isLoadingKycState || loadingState.toLowerCase() === 'paying')branch and shows<PeanutLoading ... />, even though the payment might have completed successfully.Because
isSuccessdoesn’t change again, the “set Idle on success” effect doesn’t re‑run to recover from this.A simpler, more predictable pattern is:
- Set
'Paying'immediately before callingcompleteQrPaymentWithSignedTx.- Clear it deterministically in
finally.- Drop the delayed timer entirely.
For example:
- setLoadingState('Preparing transaction') + setLoadingState('Preparing transaction') let signedUserOpData try { signedUserOpData = await signTransferUserOp(MANTECA_DEPOSIT_ADDRESS, finalPaymentLock.paymentAgainstAmount) } catch (error) { // ... setLoadingState('Idle') return } - - // Send signed UserOp to backend for coordinated execution - // Backend will: 1) Complete Manteca payment, 2) Broadcast UserOp only if Manteca succeeds - setTimeout(() => setLoadingState('Paying'), 3000) + // Send signed UserOp to backend for coordinated execution + // Backend will: 1) Complete Manteca payment, 2) Broadcast UserOp only if Manteca succeeds + setLoadingState('Paying') try { const qrPayment = await mantecaApi.completeQrPaymentWithSignedTx({ ... }) setQrPayment(qrPayment) setIsSuccess(true) } catch (error) { // ... } finally { setLoadingState('Idle') }This avoids race conditions between timers and success/failure flows and keeps the loading UI aligned with the actual network operation.
Also applies to: 253-258, 1161-1167
src/services/manteca.ts (1)
1-1: MovegetPrices/PEANUT_API_KEYusage to a server‑only module
getPricesstill pullsPEANUT_API_KEYfrom@/constantsin a shared service that’s imported by client code, which risks exposing the key in the client bundle. Even though this isn’t introduced by this PR, it would be safer to movegetPricesinto a server‑only module (or server action / API route) and have the client call that instead.Based on learnings.
Also applies to: 207-221
🧹 Nitpick comments (25)
src/components/Profile/components/ProfileHeader.tsx (1)
64-74: Add user feedback and improve error handling.The share functionality has several concerns:
- No user feedback when the URL is copied to clipboard in the fallback case.
navigator.clipboard.writeText()can fail (permissions, context) but lacks error handling.- User cancellations of the share dialog are logged as errors.
Consider applying these improvements:
onClick={() => { if (navigator.share) { navigator .share({ url: profileUrl, }) .catch((error) => { - console.error('Error sharing:', error) + // DOMException with name 'AbortError' means user cancelled + if (error.name !== 'AbortError') { + console.error('Error sharing:', error) + } }) } else { - navigator.clipboard.writeText(profileUrl) + navigator.clipboard.writeText(profileUrl) + .then(() => { + // TODO: Show success toast/notification + }) + .catch((error) => { + console.error('Failed to copy to clipboard:', error) + // TODO: Show error toast/notification + }) } }}src/components/UserHeader/index.tsx (1)
114-114: Nice UX improvement with dynamic name interpolation.The tooltip now displays the specific user's name, making the message more personalized and informative.
For consistency, consider updating the inviter tooltip on the same line to also include the name:
- content={isInviter ? 'You were invited by this user.' : `You've invited ${name}`} + content={isInviter ? `You were invited by ${name}` : `You've invited ${name}`}This would make both messages consistent, though the current implementation is perfectly fine.
src/components/Invites/InvitesPage.tsx (2)
20-35: Invite-code normalization and campaign mapping – confirm backend case sensitivityNormalizing the invite code to lowercase and mapping it via
INVITE_CODE_TO_CAMPAIGN_MAPis a clean way to attach campaign tags automatically. However, becauseinviteCodeis now lowercased at the source, that lowercased value is what gets passed into:
invitesApi.validateInviteCode(inviteCode!)setupActions.setInviteCode(inviteCode)saveToCookie('inviteCode', inviteCode)If the backend treats invite codes as case-insensitive this is fine, but if codes are case-sensitive this could cause previously valid links with mixed/upper case codes to fail validation.
Also, to make the normalization a bit more robust, consider trimming whitespace before lowercasing.
Example diff for trimming (optional):
- const inviteCode = searchParams.get('code')?.toLowerCase() + const inviteCode = searchParams.get('code')?.trim().toLowerCase()Please confirm that invite-code handling on the backend is case-insensitive before merging this change.
55-78: Content-gating effect correctly avoids flicker; consider minor simplificationThe
useEffectthat drivesshouldShowContent:
- Hides content while user data or invite validation is loading.
- Keeps content hidden while an auto-redirect is expected (user already has app access and invite is valid).
- Otherwise shows either the error view or the invite screen.
This should effectively prevent “flash” of the invite UI for logged-in users who are about to be redirected.
If you want to tighten readability, you could condense the branching into a single derived flag, e.g.:
- useEffect(() => { - // if still fetching user, don't show content yet - if (isFetchingUser) { - setShouldShowContent(false) - return - } - - // if invite validation is still loading, don't show content yet - if (isLoading) { - setShouldShowContent(false) - return - } - - // if user has app access AND invite is valid, they'll be redirected - // don't show content in this case (show loading instead) - if (!redirectUri && user?.user?.hasAppAccess && inviteCodeData?.success) { - setShouldShowContent(false) - return - } - - // otherwise, safe to show content (either error view or invite screen) - setShouldShowContent(true) - }, [user, isFetchingUser, redirectUri, inviteCodeData, isLoading]) + useEffect(() => { + const shouldHideContent = + isFetchingUser || + isLoading || + (!redirectUri && user?.user?.hasAppAccess && inviteCodeData?.success) + + setShouldShowContent(!shouldHideContent) + }, [user, isFetchingUser, redirectUri, inviteCodeData, isLoading])Not required, but it may make the gating logic easier to scan.
src/app/(mobile-ui)/home/page.tsx (1)
270-283: Re-check KYC modalonClosebehavior vs prior fire‑and‑forget patternThe new
onClosehandler forKycCompletedModalisasyncand awaits bothupdateUserByIdandfetchUser:onClose={async () => { setShowKycModal(false) if (user?.user.userId) { await updateUserById({ ... }) await fetchUser() } }}Although you optimistically close the modal first (
setShowKycModal(false)), makingonCloseitselfasyncand awaited can still change user‑perceived behavior ifKycCompletedModalinternally does something likeawait onClose?.()before finishing its own close animation or unmount.Earlier learning for this file explicitly called out that
updateUserByIdwas intentionally not awaited on KYC modal close to keep the UX snappy and treat the backend sync as fire‑and‑forget. Based on learnings.To stay closer to that intent while keeping your new logic, you could decouple the async work from the callback’s completion:
- <KycCompletedModal - isOpen={showKycModal} - onClose={async () => { - setShowKycModal(false) - if (user?.user.userId) { - await updateUserById({ - userId: user.user.userId, - showKycCompletedModal: false, - }) - await fetchUser() - } - }} - /> + <KycCompletedModal + isOpen={showKycModal} + onClose={() => { + setShowKycModal(false) + if (user?.user.userId) { + // fire-and-forget background sync + void (async () => { + await updateUserById({ + userId: user.user.userId, + showKycCompletedModal: false, + }) + await fetchUser() + })() + } + }} + />This preserves the instant close UX while still ensuring the user record is updated and refetched. Please verify against how
KycCompletedModalcurrently consumesonClose.src/components/Global/Card/index.tsx (1)
12-12: Consider using React.forwardRef for proper ref forwarding.While passing
refas a regular prop may work in some scenarios, the idiomatic React pattern for ref forwarding is to useReact.forwardRef. This ensures proper ref behavior and follows React best practices.Apply this diff to use the standard ref forwarding pattern:
-interface CardProps { +export interface CardProps { children: React.ReactNode position?: CardPosition className?: string onClick?: () => void border?: boolean - ref?: React.Ref<HTMLDivElement> } -const Card: React.FC<CardProps> = ({ children, position = 'single', className = '', onClick, border = true, ref }) => { +const Card = React.forwardRef<HTMLDivElement, CardProps>( + ({ children, position = 'single', className = '', onClick, border = true }, ref) => { const getBorderRadius = () => { switch (position) { case 'single': return 'rounded-sm' case 'first': return 'rounded-t-sm' case 'last': return 'rounded-b-sm' case 'middle': return '' default: return 'rounded-sm' } } const getBorder = () => { if (!border) return '' switch (position) { case 'single': return 'border border-black' case 'first': return 'border border-black' case 'middle': return 'border border-black border-t-0' case 'last': return 'border border-black border-t-0' default: return 'border border-black' } } return ( <div ref={ref} className={twMerge('w-full bg-white px-4 py-2', getBorderRadius(), getBorder(), className)} onClick={onClick} > {children} </div> ) -} +}) + +Card.displayName = 'Card'Also applies to: 22-22, 57-57
src/components/Global/DirectSendQR/index.tsx (1)
8-8: Remove unused import.
PeanutLoadingis imported but never used in this file.Apply this diff:
-import PeanutLoading from '@/components/Global/PeanutLoading'src/lib/url-parser/parser.ts (1)
162-169: Consider using a constant for the 'base' chain identifier.While the code is marked for post-DevConnect deletion, using a constant (e.g.,
DEVCONNECT_CHAIN_NAME = 'base') would make the special-casing more explicit and easier to find when removing this logic.Example:
+ const DEVCONNECT_CHAIN_NAME = 'base' + // 6. Determine if this is a DevConnect flow // @dev: note, this needs to be deleted post devconnect // devconnect flow: external address + base chain specified in URL const isDevConnectFlow = recipientDetails.recipientType === 'ADDRESS' && chainId !== undefined && - chainId.toLowerCase() === 'base' && + chainId.toLowerCase() === DEVCONNECT_CHAIN_NAME && chainDetails !== undefinedsrc/components/Global/PeanutLoading/index.tsx (1)
25-25: Conditionally render the message to avoid empty div.The message div will render with margin even when
messageis undefined, creating unnecessary whitespace.Apply this diff to conditionally render the message:
</div> - <div className="mt-6 self-center text-center text-center font-medium">{message}</div> + {message && <div className="mt-6 self-center text-center font-medium">{message}</div>} </div>src/hooks/useHomeCarouselCTAs.tsx (1)
11-13: DevConnect intent lifecycle and CTA wiring look consistent; minor optional refinements only
- Reading intents from
getUserPreferences, pruning byDEVCONNECT_INTENT_EXPIRY_MS, and selecting the newest pending intent is coherent and matches the way intents are stored insaveDevConnectIntent.- Using a dedicated
pendingDevConnectIntentstate and wiring it intogenerateCarouselCTAs(including it in the dependencies) keeps the CTA generation predictable.- Clearing
devConnectIntentsinonCloseensures the CTA won’t reappear on future mounts once dismissed; the current session behavior is handled by the carousel itself.Optional refinements you could consider (non‑blocking):
- Use a non‑mutating sort for clarity, e.g.
const [mostRecentIntent] = [...recentIntents].sort(...), so the name “recentIntents” clearly refers to the unmodified array used for persistence.- In
onClose, you might alsosetPendingDevConnectIntent(undefined)to keep local state aligned immediately with preferences, though functionally things still work because the CTA instance is already being closed.Also applies to: 40-82, 111-137, 179-180
src/components/AddMoney/components/MantecaAddMoney.tsx (1)
18-21: Manteca limits + DevConnect intent persistence are wired correctly; only small cleanups possible
- Reusing
MIN_MANTECA_DEPOSIT_AMOUNT/MAX_MANTECA_DEPOSIT_AMOUNTin theusdAmountvalidation keeps limits centralized and consistent with other flows.- The
parseUnitscomparison against those constants looks correct givenPEANUT_WALLET_TOKEN_DECIMALS.- Calling
saveDevConnectIntent(user?.user?.userId, parsedPaymentData, usdAmount, depositData.data?.externalId)right after a successfulmantecaApi.depositaligns with the persistence logic insaveDevConnectIntent(it will no‑op for non‑DevConnect flows and is tolerant of missingonrampId).Potential non‑blocking tweaks:
- If
useSearchParamsis not used, consider removing it to avoid lint warnings.- If this intent logging is meant to be strictly best‑effort, you might optionally wrap just the
saveDevConnectIntentcall in a try/catch so any unexpected error there cannot affect the deposit flow (though the helper already swallows its own internal errors).Also applies to: 31-31, 41-41, 64-77, 107-123
src/app/(mobile-ui)/add-money/crypto/direct/page.tsx (1)
27-44: Confirmation modal flow before Daimo deposit is sound; consider DRYing min/max and guarding edge parsing
- The new flow (
validateAmount→showModal→ slideronAccepted→ DaimoonClick) nicely forces users through the “You MUST” checklist before opening the Daimo widget, and ties validation + error display into the existingErrorAlert.- Passing
minAmount,maxAmount, andonValidationError={setError}intoDaimoPayButtonkeeps the SDK‑side validation in sync with your local checks.Two optional improvements:
- Avoid repeated magic numbers
Min/max values (0.1and30_000) are duplicated invalidateAmountand asminAmount/maxAmountprops onDaimoPayButton. Consider extracting them into file‑level constants so updating the limits in future is one change instead of two:+const MIN_CRYPTO_DEPOSIT = 0.1 +const MAX_CRYPTO_DEPOSIT = 30_000 ... -if (formattedAmount < 0.1) { - setError('Minimum deposit using crypto is $0.1.') +if (formattedAmount < MIN_CRYPTO_DEPOSIT) { + setError(`Minimum deposit using crypto is $${MIN_CRYPTO_DEPOSIT}.`) ... -if (formattedAmount > 30_000) { - setError('Maximum deposit using crypto is $30,000.') +if (formattedAmount > MAX_CRYPTO_DEPOSIT) { + setError(`Maximum deposit using crypto is $${MAX_CRYPTO_DEPOSIT.toLocaleString()}.`) ... -minAmount={0.1} -maxAmount={30_000} +minAmount={MIN_CRYPTO_DEPOSIT} +maxAmount={MAX_CRYPTO_DEPOSIT}- Optional NaN guard
IfTokenAmountInputever produced a non‑numeric string (e.g., transient states),parseFloatcould returnNaNand slip past both comparisons. You could short‑circuit onNumber.isNaN(formattedAmount)and surface a generic validation error before opening the modal.Also applies to: 111-125, 131-201
src/components/Send/views/Contacts.view.tsx (1)
54-58: Avoid usingwindow.location.pathnamedirectly for navigation
redirectToSendByLinkbuilds the URL with${window.location.pathname}?view=link. In a Next.js app this works, but it’s a bit brittle (e.g., base paths, locale prefixes, or existing query params are ignored) and makes the function harder to test.You could instead derive the path from
usePathname()or push a known route, e.g.router.push('/send?view=link'), or manipulateuseSearchParams()to preserve other query parameters.src/hooks/usePullToRefresh.ts (1)
35-64: Hook setup/teardown for pull‑to‑refresh is correct; optionally guard against double initThe effect correctly skips SSR/disabled cases, initializes
PullToRefreshonce perrouter/enabledcombination, and destroys instances in the cleanup. Using a ref to holdshouldPullToRefreshmeans you don’t re‑init just because the callback identity changes, which is nice.If you ever find this hook being mounted/unmounted frequently, you could defensively call
PullToRefresh.destroyAll()just beforePullToRefresh.init({...})to guarantee there’s never more than one instance, but given the current dependency setup this is more of a precaution than a necessity.src/context/authContext.tsx (1)
127-167: Cache clearing implementation is solid; optional concurrency guard improvement remains validThe
USER_DATA_CACHE_PATTERNSapproach correctly targets user-specific caches (user API, transactions, KYC merchant), properly guarded by feature detection ('caches' in window) and wrapped in try/catch to prevent cache clearing failures from blocking logout.The
isLoggingOutguard works as intended for most use cases. However, verification reveals a few UI call sites lack properdisabledattributes:
src/components/Global/NavHeader/index.tsx(line 65–69): Hasloading={isLoggingOut}but nodisabledpropsrc/app/(mobile-ui)/settings/page.tsx(line 14–17): Lacks any state guardWhile these could theoretically allow rapid clicks to bypass the guard before React flushes the state update, the impact is low since the guard engages once
setIsLoggingOut(true)propagates. The optionaluseRefapproach suggested would eliminate this edge case but remains a refactor rather than a fix.src/components/Global/PeanutFactsLoading/index.tsx (2)
33-36: Passmessagethrough toPeanutLoadingfor consistent loading textRight now
PeanutLoadingis rendered without amessageprop while you also render{message}in the<p>. This means the spinner’s sr-only text defaults to “Loading…” while the visible text says “Processing…” (or whatever is passed), which is slightly inconsistent for screen readers.You can align both by passing the same message down:
- <div className="flex items-center justify-center gap-2 pt-6"> - <div> - <PeanutLoading /> - </div> - <p className="text-center text-sm font-semibold text-gray-900"> {message} </p> - </div> + <div className="flex items-center justify-center gap-2 pt-6"> + <div> + <PeanutLoading message={message} /> + </div> + <p className="text-center text-sm font-semibold text-gray-900">{message}</p> + </div>Also applies to: 57-62
52-54: Consider updatingnext/imageusage to the modernfillAPIUsing
layout="fill"andobjectFit="contain"still works but is the older API. In newer Next.js versions the recommended pattern is:<Image src={PeanutGuyGIF} alt="Peanut Man" fill className="object-contain" />Not urgent, but it will avoid future deprecation warnings.
src/app/[...recipient]/client.tsx (2)
177-237: DevConnect intent persistence: watch in-place sort and login timingThe overall persistence flow (deterministic
intentId, capped list, amount updates) looks solid, but there are a few details to consider:
existingIntents.sort(...)mutates the array returned fromgetUserPreferences. Safer to clone before sorting so you never accidentally mutate shared state:- const sortedIntents = existingIntents.sort((a, b) => b.createdAt - a.createdAt) + const sortedIntents = [...existingIntents].sort((a, b) => b.createdAt - a.createdAt)
The
createDeterministicIdhelper works, buthash = hash & hashis a non-obvious way to force a 32‑bit int.hash |= 0(or a comment) would make the intent clearer to future readers.This block only runs when
parsePaymentURLresolves anduser?.user?.userIdis already set. If a user opens a DevConnect link while logged out and then logs in,isUrlParsedis alreadytrue, so the effect won’t re-run and the intent won’t be stored. If that scenario should be supported, you may need a follow-up effect keyed onuser?.user.userIdthat replays the persistence usingparsedPaymentData.
478-479: ConfirmshowActionListbehavior fordirect_payflows
showActionListis now:const showActionList = flow !== 'direct_pay' || (flow === 'direct_pay' && !user)So for
direct_pay:
- Logged-in users: ActionList is hidden.
- Guest / unknown users: ActionList is shown.
Combined with the new
usdAmountprop passed intoActionList, this changes when upsell/actions are visible compared to prior behavior.If the intent is “only show ActionList for direct-pay when the user is not logged in”, this is correct; otherwise, you might want a tighter condition such as explicitly checking
isExternalWalletFloworisDirectPay. Please double-check with the product expectations for direct pay.Also applies to: 567-576
src/interfaces/interfaces.ts (1)
339-355: Consider tightening Contact typing for KYC status and relationshipsThe new
Contact/ContactsResponseshapes look good, but you can make them more type-safe:
bridgeKycStatusis currentlystring | nulleven thoughBridgeKycStatusis already imported:-export interface Contact { +export interface Contact { userId: string username: string fullName: string | null - bridgeKycStatus: string | null + bridgeKycStatus: BridgeKycStatus | null
- If
relationshipTypesis reused elsewhere, consider extracting the union to a named type (e.g.type ContactRelationshipType = 'inviter' | 'invitee' | 'sent_money' | 'received_money') to avoid future drift.These are small improvements but will help keep the API and backend payloads aligned.
src/app/(mobile-ui)/withdraw/manteca/page.tsx (1)
78-85: Manteca flows now always default to local-currency input—verify this is desiredThe new logic:
- Derives
isMantecaCountryfromselectedCountry.id in MANTECA_COUNTRIES_CONFIG.- Sets
isInitialInputUsdto:
falseifswap-currency === 'true'(explicit local toggle for non‑Manteca).falsefor any Manteca country, regardless ofswap-currency.trueonly for non‑Manteca countries without theswap-currency=trueflag.Net effect: Manteca withdrawals always start with local-currency input; the
swap-currencyquery param no longer allows deep-linking into a USD-default state for those countries.If that matches product/UX requirements, this is fine. If you still want deep links that start in USD for Manteca, you might want to respect a second explicit flag (or a different param) instead of forcing local currency unconditionally.
Also, since an
isMantecaCountry(countryPath)helper already exists inmanteca.consts.ts, consider reusing it here to avoid duplicating the condition.Also applies to: 91-99, 107-124
src/constants/payment.consts.ts (1)
1-32: validateMinimumAmount: consider failing unknown methods and hoisting the mapCurrent behavior treats unknown
methodIdas having a 0 minimum, which can silently accept misconfigured or new methods, and theminimumsmap is rebuilt on every call.You might want unknown methods to fail validation instead, and keep the map at module scope:
-export const validateMinimumAmount = (amount: number, methodId: string): boolean => { - const minimums: Record<string, number> = { - bank: MIN_BANK_TRANSFER_AMOUNT, - mercadopago: MIN_MERCADOPAGO_AMOUNT, - pix: MIN_PIX_AMOUNT, - } - return amount >= (minimums[methodId] ?? 0) -} +const MINIMUMS_BY_METHOD: Record<string, number> = { + bank: MIN_BANK_TRANSFER_AMOUNT, + mercadopago: MIN_MERCADOPAGO_AMOUNT, + pix: MIN_PIX_AMOUNT, +} + +export const validateMinimumAmount = (amount: number, methodId: string): boolean => { + const min = MINIMUMS_BY_METHOD[methodId] + if (min === undefined) return false + return amount >= min +}This makes misconfigurations more visible and avoids unnecessary allocations.
src/components/Send/views/SendRouter.view.tsx (1)
18-35: Contacts avatars: logic is good; consider more robust keysThe new
useContacts({ limit: 3 })+recentContactsAvatarInitialsflow is a nice way to:
- Show a small, geo‑filtered avatar preview for “Peanut contacts”.
- Fall back to
['PE', 'AN', 'UT']when there’s no contact data.- Display a lightweight skeleton while
isFetchingContactsis true.One small nit: using
key={initial}in the avatar map can collide when multiple contacts share initials. You can keep the behavior but make keys stable with the index:- {recentContactsAvatarInitials().map((initial, index) => { - return ( - <div key={initial} style={{ zIndex: index }}> + {recentContactsAvatarInitials().map((initial, index) => { + return ( + <div key={`${initial}-${index}`} style={{ zIndex: index }}> <AvatarWithBadge name={initial} size="tiny" /> </div> ) })}Functionality is fine as-is; this just avoids potential React key warnings.
Also applies to: 36-78
src/components/Common/ActionList.tsx (1)
115-116: Daimo ref + balance modal interplay is soundStoring the Daimo button’s click handler in
daimoButtonClickRefand invoking it from the “Continue” CTA viasetTimeoutafter updating modal state avoids double balance prompts and respects the user’s original method choice. This keeps the exchange‑or‑wallet flow aligned with the Peanut‑balance modal without entangling Daimo’s internal UI.You might later consider also disabling methods while a payment is in progress (using
loadingStep) to prevent accidental double inits, but it’s not blocking for this PR.Also applies to: 321-341, 411-461
src/utils/general.utils.ts (1)
452-469: DevConnect intent persistence is robust; a couple of small refactors are possibleThe
devConnectIntentsshape andsaveDevConnectIntentlogic (deriving flow data fromParsedURLor the most recent pending intent, deterministic id from address+chain, pruning toMAX_DEVCONNECT_INTENTS) are well-structured and safely wrapped in try/catch so they never block the main flow.Two minor refinements you could consider (non-blocking):
- Avoid mutating
existingIntentswhen sorting, e.g.const sortedIntents = [...existingIntents].sort(...).- Prefer a top-level import for
MAX_DEVCONNECT_INTENTSinstead ofrequire('@/constants/payment.consts')inside the function, if it doesn’t introduce circular dependencies; that would keep everything typed and easier to tree-shake.Also applies to: 952-1047
| import { useDisconnect } from '@reown/appkit/react' | ||
| import Link from 'next/link' | ||
| import { useEffect, useMemo, useState, useCallback } from 'react' | ||
| import { useEffect, useMemo, useState, useCallback, lazy, Suspense } from 'react' |
There was a problem hiding this comment.
🧩 Analysis chain
Most lazy-loaded modals are still mounted unconditionally; lazy chunks will load on first render
You’ve switched the heavy modals to React.lazy + Suspense + LazyLoadErrorBoundary, which is great. However, only SetupNotificationsModal is actually conditionally rendered:
SetupNotificationsModal– wrapped in{showPermissionModal && (…)}✅IOSInstallPWAModal,NoMoreJailModal,EarlyUserModal,KycCompletedModal,BalanceWarningModal– the components are always rendered, with visibility controlled only by props likevisible/isOpen❌
For lazy components, rendering them at all (even with visible={false}) will cause React to evaluate the lazy component and trigger the dynamic import(), so the associated chunks are still fetched on the initial home render. That contradicts the intent expressed in the comments on Lines 38–40 (“Components are only loaded when user triggers them”).
To actually defer network/parse cost until the user action or condition hits, the lazy component usage should be gated by the same booleans you already maintain.
Example pattern for the modals currently rendered unconditionally:
- {/* iOS PWA Install Modal */}
- <LazyLoadErrorBoundary>
- <Suspense fallback={null}>
- <IOSInstallPWAModal
- visible={showIOSPWAInstallModal}
- onClose={() => setShowIOSPWAInstallModal(false)}
- />
- </Suspense>
- </LazyLoadErrorBoundary>
+ {/* iOS PWA Install Modal */}
+ {showIOSPWAInstallModal && (
+ <LazyLoadErrorBoundary>
+ <Suspense fallback={null}>
+ <IOSInstallPWAModal
+ visible
+ onClose={() => setShowIOSPWAInstallModal(false)}
+ />
+ </Suspense>
+ </LazyLoadErrorBoundary>
+ )}The same idea applies to:
NoMoreJailModalEarlyUserModalKycCompletedModalBalanceWarningModal(gated byshowBalanceWarningModal)
This will better match the stated goal of loading these chunks only when actually needed. Please verify against your bundle analysis and any existing modal patterns elsewhere in the app.
Also applies to: 38-47, 226-232, 243-251, 256-261, 262-267, 268-287, 290-305
🏁 Script executed:
#!/bin/bash
# Inspect the home page file to verify lazy-loading and modal rendering patterns
cat src/app/\(mobile-ui\)/home/page.tsx | head -50Length of output: 3085
🏁 Script executed:
#!/bin/bash
# Check modal rendering sections mentioned in review (lines 226-287, 290-305)
sed -n '226,305p' src/app/\(mobile-ui\)/home/page.tsxLength of output: 3344
🏁 Script executed:
#!/bin/bash
# Extract KycCompletedModal onClose implementation
sed -n '268,287p' src/app/\(mobile-ui\)/home/page.tsxLength of output: 968
🏁 Script executed:
#!/bin/bash
# Check for conditional rendering gates for each modal
rg -n "showIOSPWAInstallModal|showNoMoreJail|showEarlyUser|showKycCompleted|showBalanceWarning" src/app/\(mobile-ui\)/home/page.tsx -A2 -B2Length of output: 2087
🏁 Script executed:
#!/bin/bash
# Check NoMoreJailModal and EarlyUserModal component definitions to see their props
fd -e tsx -e ts -path '*/components/Global/*' | grep -E 'NoMoreJailModal|EarlyUserModal'Length of output: 168
🏁 Script executed:
#!/bin/bash
# Inspect NoMoreJailModal and EarlyUserModal for visible/isOpen/similar props
grep -r "export.*NoMoreJailModal" src/components/Global/ -A 20 | head -40Length of output: 78
🏁 Script executed:
#!/bin/bash
# Check EarlyUserModal component
grep -r "export.*EarlyUserModal" src/components/Global/ -A 20 | head -40Length of output: 76
---
🏁 Script executed:
```shell
#!/bin/bash
# Inspect full NoMoreJailModal component to see function signature and props
head -50 src/components/Global/NoMoreJailModal/index.tsx
Length of output: 1938
🏁 Script executed:
#!/bin/bash
# Inspect full EarlyUserModal component to see function signature and props
head -50 src/components/Global/EarlyUserModal/index.tsxLength of output: 1899
🏁 Script executed:
#!/bin/bash
# Get more context on NoMoreJailModal including props interface if any
cat src/components/Global/NoMoreJailModal/index.tsxLength of output: 2858
🏁 Script executed:
#!/bin/bash
# Get more context on EarlyUserModal including props interface if any
cat src/components/Global/EarlyUserModal/index.tsxLength of output: 2105
Lazy-loaded modals are still rendering unconditionally; chunks load on initial render
The lazy imports are beneficial, but only SetupNotificationsModal is actually gated. The other five modals (IOSInstallPWAModal, NoMoreJailModal, EarlyUserModal, KycCompletedModal, BalanceWarningModal) are always rendered, which triggers their lazy chunks on the initial page load regardless of whether the user will interact with them.
NoMoreJailModalandEarlyUserModalmanage visibility internally (viasessionStorageand user data) but rendering them at all still causes the import to fire.IOSInstallPWAModal,KycCompletedModal, andBalanceWarningModaldepend on parent-controlled props, but they're rendered unconditionally.
For true lazy-loading, wrap these in conditional gates using the same booleans you already maintain:
- <LazyLoadErrorBoundary>
- <Suspense fallback={null}>
- <IOSInstallPWAModal
- visible={showIOSPWAInstallModal}
- onClose={() => setShowIOSPWAInstallModal(false)}
- />
- </Suspense>
- </LazyLoadErrorBoundary>
+ {showIOSPWAInstallModal && (
+ <LazyLoadErrorBoundary>
+ <Suspense fallback={null}>
+ <IOSInstallPWAModal
+ visible
+ onClose={() => setShowIOSPWAInstallModal(false)}
+ />
+ </Suspense>
+ </LazyLoadErrorBoundary>
+ )}Apply the same pattern to NoMoreJailModal, EarlyUserModal, KycCompletedModal, and BalanceWarningModal (gated by showBalanceWarningModal).
Also applies to: 244–248, 250–254, 255–259, 270–287, 290–296
🤖 Prompt for AI Agents
In src/app/(mobile-ui)/home/page.tsx around lines 18 and also applying to ranges
244–248, 250–254, 255–259, 270–287, 290–296, the lazy-imported modal components
are being rendered unconditionally which forces their chunks to load on initial
render; update the JSX to only render each lazy modal when its corresponding
boolean gate is true (e.g., wrap IOSInstallPWAModal with its
showIOSInstallPwaModal flag, NoMoreJailModal with its showNoMoreJailModal flag,
EarlyUserModal with showEarlyUserModal, KycCompletedModal with
showKycCompletedModal, and BalanceWarningModal with showBalanceWarningModal),
keep Suspense around the conditional renders if needed, and remove unconditional
mounts so the lazy chunks only load when the modal will actually be shown.
| const { | ||
| data: fetchedPaymentLock, | ||
| isLoading: isLoadingPaymentLock, | ||
| error: paymentLockError, | ||
| failureCount, | ||
| } = useQuery({ | ||
| queryKey: ['manteca-payment-lock', qrCode, timestamp], | ||
| queryFn: async ({ queryKey }) => { | ||
| if (paymentProcessor !== 'MANTECA' || !qrCode || !isPaymentProcessorQR(qrCode)) { | ||
| return null | ||
| } | ||
| return mantecaApi.initiateQrPayment({ qrCode }) | ||
| }, | ||
| enabled: paymentProcessor === 'MANTECA' && !!qrCode && isPaymentProcessorQR(qrCode) && !paymentLock, | ||
| retry: (failureCount, error: any) => { | ||
| // Don't retry provider-specific errors | ||
| if (error?.message?.includes("provider can't decode it")) { | ||
| return false | ||
| } | ||
| // Retry network/timeout errors up to 2 times (3 total attempts) | ||
| return failureCount < 2 | ||
| }, | ||
| retryDelay: (attemptIndex) => { | ||
| const delayMs = Math.min(1000 * 2 ** attemptIndex, 2000) // 1s, 2s exponential backoff | ||
| const MAX_RETRIES = 2 | ||
| const attemptNumber = attemptIndex + 1 // attemptIndex is 0-based, display as 1-based | ||
| console.log( | ||
| `Payment lock fetch failed, retrying in ${delayMs}ms... (attempt ${attemptNumber}/${MAX_RETRIES})` | ||
| ) | ||
| return delayMs | ||
| }, | ||
| staleTime: 0, // Always fetch fresh data | ||
| gcTime: 0, // Don't cache for garbage collection | ||
| }) | ||
|
|
There was a problem hiding this comment.
Manteca “waiting for merchant amount” can get stuck with no retries
When mantecaApi.initiateQrPayment fails with "provider can't decode it" for non‑PIX QR types:
setWaitingForMerchantAmount(true)is called.- The
waitingForMerchantAmountbranch renders<QrPayPageLoading ... />. - The retry loop in the
useEffectat lines 960‑972 depends onshouldRetry, butshouldRetryis only toggled by the SimpleFi retry effect (paymentProcessor === 'SIMPLEFI').
For MANTECA flows, paymentProcessor !== 'SIMPLEFI', so:
shouldRetrynever flips fromtruetofalsefor MANTECA.- The
waitingForMerchantAmount && !shouldRetrycondition never passes. - You end up stuck indefinitely on the “Waiting for the merchant to set the amount” loading state, with no retries and no transition to the “We couldn't get the amount” empty state.
A straightforward fix is to decouple MANTECA from the SimpleFi retry flag and explicitly refetch the Manteca payment lock:
- useEffect(() => {
- if (waitingForMerchantAmount && !shouldRetry) {
- if (retryCount.current < 3) {
- retryCount.current++
- setTimeout(() => {
- setShouldRetry(true)
- }, 3000)
- } else {
- setWaitingForMerchantAmount(false)
- setShowOrderNotReadyModal(true)
- }
- }
- }, [waitingForMerchantAmount, shouldRetry])
+ useEffect(() => {
+ if (!waitingForMerchantAmount || paymentProcessor !== 'MANTECA') return
+
+ if (retryCount.current < 3) {
+ retryCount.current++
+ const timeoutId = setTimeout(() => {
+ queryClient.invalidateQueries({ queryKey: ['manteca-payment-lock', qrCode, timestamp] })
+ }, 3000)
+ return () => clearTimeout(timeoutId)
+ }
+
+ setWaitingForMerchantAmount(false)
+ setShowOrderNotReadyModal(true)
+ }, [waitingForMerchantAmount, paymentProcessor, queryClient, qrCode, timestamp])This keeps the existing 3‑attempt behavior but guarantees progress (either success or an explicit “order not ready” state) for MANTECA as well.
Also applies to: 497-540, 960-972, 1123-1125
🤖 Prompt for AI Agents
In src/app/(mobile-ui)/qr-pay/page.tsx around lines 461-495 (and related
references at 497-540, 960-972, 1123-1125): the MANTECA flow can get stuck
waiting for merchant amount because it never participates in the SimpleFi-driven
shouldRetry toggle; update the MANTECA branch so that when initiateQrPayment
returns the "provider can't decode it" condition you explicitly trigger
refetching the Manteca payment lock (or flip a local retry flag used only for
MANTECA) to perform up to the existing 3 attempts and then stop and show the
"order not ready" empty state; ensure the refetch/retry respects the same retry
count/backoff logic as the useQuery (or call queryClient/refetch with attempt
tracking) and do not rely on the SimpleFi shouldRetry flag for MANTECA.
| registration.addEventListener('updatefound', () => { | ||
| const newWorker = registration.installing; | ||
| if (newWorker) { | ||
| newWorker.addEventListener('statechange', () => { | ||
| if (newWorker.state === 'installed' && navigator.serviceWorker.controller) { | ||
| // New SW installed, reload to activate | ||
| console.log('New SW available, reloading...'); | ||
| window.location.reload(); | ||
| } | ||
| }); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Auto-reload on service worker update may disrupt user flows.
The implementation immediately reloads the page when a new service worker is installed (line 106). This could interrupt users in the middle of transactions or form submissions.
Consider:
- Showing a notification/toast asking users to refresh when convenient
- Delaying reload if there's an active transaction
- Using
skipWaiting()+clients.claim()without forced reload for non-critical updates
Example improvement:
- if (newWorker.state === 'installed' && navigator.serviceWorker.controller) {
- // New SW installed, reload to activate
- console.log('New SW available, reloading...');
- window.location.reload();
- }
+ if (newWorker.state === 'installed' && navigator.serviceWorker.controller) {
+ // New SW available, notify user instead of forcing reload
+ console.log('New SW available');
+ // TODO: Show toast notification with "Update Available - Click to refresh"
+ // Only auto-reload if user is idle or consents
+ }📝 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.
| registration.addEventListener('updatefound', () => { | |
| const newWorker = registration.installing; | |
| if (newWorker) { | |
| newWorker.addEventListener('statechange', () => { | |
| if (newWorker.state === 'installed' && navigator.serviceWorker.controller) { | |
| // New SW installed, reload to activate | |
| console.log('New SW available, reloading...'); | |
| window.location.reload(); | |
| } | |
| }); | |
| } | |
| }); | |
| registration.addEventListener('updatefound', () => { | |
| const newWorker = registration.installing; | |
| if (newWorker) { | |
| newWorker.addEventListener('statechange', () => { | |
| if (newWorker.state === 'installed' && navigator.serviceWorker.controller) { | |
| // New SW available, notify user instead of forcing reload | |
| console.log('New SW available'); | |
| // TODO: Show toast notification with "Update Available - Click to refresh" | |
| // Only auto-reload if user is idle or consents | |
| } | |
| }); | |
| } | |
| }); |
🤖 Prompt for AI Agents
In src/app/layout.tsx around lines 99 to 110, the current service-worker handler
force-reloads the page immediately when a new worker is installed which can
disrupt users; instead, replace the immediate window.location.reload() with
logic that (1) notifies the user (toast/modal) that an update is available and
offers a “Refresh now” action, (2) optionally delay or suppress reload when
there’s an active transaction or dirty form (hook into your app state or check
for unsaved changes before reloading), and (3) for non-critical updates call
postMessage/skipWaiting() from the SW and use clients.claim() to activate
without forcing navigation; implement the notification UI and an explicit
user-initiated reload (or graceful background activation) rather than
auto-reloading.
| // check if amount is valid for request flow | ||
| const currentRequestAmount = usdAmountValue ?? usdAmount | ||
| const requestAmountValue = currentRequestAmount ? parseFloat(currentRequestAmount) : 0 | ||
| const isAmountEntered = flow === 'request' ? !!currentRequestAmount && requestAmountValue > 0 : true | ||
|
|
||
| const handleMethodClick = async (method: PaymentMethod, bypassBalanceModal = false) => { | ||
| // validate minimum amount for bank/mercado pago/pix in request flow | ||
| if (flow === 'request' && requestLinkData) { | ||
| // check minimum amount for bank/mercado pago/pix | ||
| if ( | ||
| ['bank', 'mercadopago', 'pix'].includes(method.id) && | ||
| !validateMinimumAmount(requestAmountValue, method.id) | ||
| ) { | ||
| setShowMinAmountError(true) | ||
| return | ||
| } | ||
| } |
There was a problem hiding this comment.
Minimum-amount modal shows the wrong value for Mercadopago/PIX
You correctly use validateMinimumAmount for bank, mercadopago, and pix, but the error modal always interpolates MIN_BANK_TRANSFER_AMOUNT:
- For Mercadopago/PIX, the enforced minimums (
MIN_MERCADOPAGO_AMOUNT,MIN_PIX_AMOUNT) can differ from bank’s, yet the copy always mentions the bank limit. - This will confuse users when they’re blocked at, say, the PIX minimum but see a bank threshold in the message.
Consider tracking which method triggered the validation failure and computing the correct minimum per method, e.g.:
- const [showMinAmountError, setShowMinAmountError] = useState(false)
+ const [showMinAmountError, setShowMinAmountError] = useState(false)
+ const [minAmountMethodId, setMinAmountMethodId] = useState<string | null>(null)
...
- if (
- ['bank', 'mercadopago', 'pix'].includes(method.id) &&
- !validateMinimumAmount(requestAmountValue, method.id)
- ) {
- setShowMinAmountError(true)
- return
- }
+ if (
+ ['bank', 'mercadopago', 'pix'].includes(method.id) &&
+ !validateMinimumAmount(requestAmountValue, method.id)
+ ) {
+ setMinAmountMethodId(method.id)
+ setShowMinAmountError(true)
+ return
+ }
...
- if (method.id === 'bank' && !validateMinimumAmount(amountInUsd, method.id)) {
- setShowMinAmountError(true)
- return
- }
+ if (method.id === 'bank' && !validateMinimumAmount(amountInUsd, method.id)) {
+ setMinAmountMethodId(method.id)
+ setShowMinAmountError(true)
+ return
+ }
...
- <ActionModal
- visible={showMinAmountError}
- onClose={() => setShowMinAmountError(false)}
- title="Minimum Amount"
- description={`The minimum amount for this payment method is $${MIN_BANK_TRANSFER_AMOUNT}. Please enter a higher amount or try a different method.`}
+ const getMinAmountForMethod = (methodId: string | null) => {
+ switch (methodId) {
+ case 'bank':
+ return MIN_BANK_TRANSFER_AMOUNT
+ case 'mercadopago':
+ return MIN_MERCADOPAGO_AMOUNT
+ case 'pix':
+ return MIN_PIX_AMOUNT
+ default:
+ return MIN_BANK_TRANSFER_AMOUNT
+ }
+ }
+ ...
+ <ActionModal
+ visible={showMinAmountError}
+ onClose={() => {
+ setShowMinAmountError(false)
+ setMinAmountMethodId(null)
+ }}
+ title="Minimum Amount"
+ description={`The minimum amount for this payment method is $${getMinAmountForMethod(minAmountMethodId)}. Please enter a higher amount or try a different method.`}(adjust imports for MIN_MERCADOPAGO_AMOUNT and MIN_PIX_AMOUNT from your constants module).
Also applies to: 171-177, 371-377
🤖 Prompt for AI Agents
In src/components/Common/ActionList.tsx around lines 145-161 (also apply the
same change at 171-177 and 371-377): the min-amount error modal always shows
MIN_BANK_TRANSFER_AMOUNT even when mercadopago or pix triggered validation.
Update the handler to record which method failed (e.g., store the method id or
the actual minimum amount in state) before calling setShowMinAmountError, import
the proper constants (MIN_BANK_TRANSFER_AMOUNT, MIN_MERCADOPAGO_AMOUNT,
MIN_PIX_AMOUNT) and map method.id to the correct minimum when rendering the
modal so the message interpolates the correct threshold; include a sensible
fallback default if an unknown method is encountered.
| if (chargeDetails?.requestLink.recipientAccount.type === 'peanut-wallet') { | ||
| return ( | ||
| chargeDetails?.requestLink.recipientAccount.user.username ?? | ||
| printableAddress(chargeDetails?.requestLink.recipientAddress as string) | ||
| ) | ||
| } else { | ||
| return printableAddress(chargeDetails?.requestLink.recipientAccount.identifier as string) | ||
| } |
There was a problem hiding this comment.
Fix null handling and avoid using printableAddress for non-address identifiers.
This code has critical issues:
-
Null handling bug: If
chargeDetailsisundefined, the condition on line 112 evaluates tofalse, falling through to line 118 whereprintableAddress(undefined as string)is called. TheprintableAddressfunction expects a string and will receiveundefined, causing incorrect behavior. -
Type assertion masks errors: The
as stringassertions on lines 115 and 118 bypass TypeScript's type checking and hide potential runtime type mismatches. -
Semantic mismatch: For non-peanut-wallet account types,
recipientAccount.identifiermay not be an Ethereum address (could be email, phone number, etc.). UsingprintableAddress()on non-address identifiers is incorrect since that function is designed to format addresses.
Apply this diff to fix the null handling and semantic issues:
case 'request':
- if (chargeDetails?.requestLink.recipientAccount.type === 'peanut-wallet') {
- return (
- chargeDetails?.requestLink.recipientAccount.user.username ??
- printableAddress(chargeDetails?.requestLink.recipientAddress as string)
- )
- } else {
- return printableAddress(chargeDetails?.requestLink.recipientAccount.identifier as string)
- }
+ if (!chargeDetails?.requestLink.recipientAccount) {
+ return ''
+ }
+ if (chargeDetails.requestLink.recipientAccount.type === 'peanut-wallet') {
+ return (
+ chargeDetails.requestLink.recipientAccount.user.username ??
+ printableAddress(chargeDetails.requestLink.recipientAddress)
+ )
+ } else {
+ // For non-peanut-wallet accounts, identifier may not be an address
+ return chargeDetails.requestLink.recipientAccount.identifier
+ }🤖 Prompt for AI Agents
In src/components/Common/CountryListRouter.tsx around lines 112 to 119, the
branch assumes chargeDetails and recipientAccount exist and always calls
printableAddress with forced string casts; instead, first guard that
chargeDetails and chargeDetails.requestLink and
chargeDetails.requestLink.recipientAccount exist, then handle peanut-wallet by
returning recipientAccount.user.username if present or
printableAddress(recipientAddress) only when recipientAddress is a defined
string; for non-peanut types return the raw recipientAccount.identifier (no
printableAddress) and remove the `as string` assertions; if you must format an
identifier as an address, only call printableAddress after validating the
identifier is a proper address (use existing isValidAddress/address-check util)
and fall back to a safe default (empty string or '-') when values are missing.
| // inline peanut icon svg to ensure it works offline without needing to fetch external assets | ||
| const PeanutIcon = ({ className }: { className?: string }) => ( | ||
| <svg | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| viewBox="0 0 291 389" | ||
| fill="none" | ||
| aria-label="Peanut Logo" | ||
| className={className} | ||
| > | ||
| {/* <!-- Peanut shape --> */} | ||
| <path | ||
| d="M60.3258 45.632C64.7897 43.0841 70.8696 42.4485 77.6753 42.1648L77.6751 42.1639C86.6738 41.7919 95.9563 42.9122 105.073 44.8494C131.211 50.4032 159.276 64.4612 173.241 88.947L173.241 88.948C182.385 105.004 187.299 122.974 187.679 140.59L187.68 140.615L187.681 140.639C188.214 158.799 197.656 175.377 213.007 185.103L213.027 185.115L213.048 185.129C227.987 194.435 240.944 207.825 250.088 223.88L250.089 223.881C264.205 248.652 262.114 279.714 253.648 304.817C253.251 305.963 252.866 307.057 252.469 308.126L252.46 308.151L252.45 308.178C252.436 308.216 252.422 308.255 252.408 308.294C252.395 308.33 252.381 308.367 252.367 308.403C246.631 323.792 238.741 335.81 232.382 341.201C232.326 341.246 232.276 341.285 232.239 341.315C232.158 341.378 232.121 341.409 232.087 341.434L232.052 341.462L232.017 341.489C231.506 341.893 231.256 342.093 231.002 342.275C230.703 342.487 230.41 342.68 230.129 342.856L229.759 343.068C226.058 345.176 218.929 346.766 209.112 346.794C199.522 346.822 188.125 345.356 176.457 342.08C153.35 335.592 130.193 322.32 117.448 300.794L116.849 299.762C107.705 283.706 102.79 265.736 102.41 248.12L102.409 248.096L102.409 248.072C101.876 229.912 92.433 213.335 77.0818 203.609L77.0617 203.595L77.0418 203.583L75.6472 202.699C61.7596 193.736 49.6638 181.222 40.8698 166.328L40.0013 164.831L39.4191 163.79C27.402 141.848 27.7929 115.163 33.9934 91.9808C37.1244 80.275 41.6741 69.7248 46.5873 61.491C51.6171 53.0618 56.6207 47.7423 60.3214 45.6342L60.3258 45.632Z" | ||
| fill="#FFC900" | ||
| stroke="black" | ||
| stroke-width="12.6195" | ||
| /> | ||
|
|
||
| {/* <!-- Eye lines --> */} | ||
| <path d="M106.78 163.414L112.666 153.471" stroke="black" stroke-width="8.41298" stroke-linecap="round" /> | ||
|
|
||
| {/* <!-- Left eye --> */} | ||
| <path | ||
| d="M85.1709 145.907C98.5727 145.757 109.316 134.772 109.167 121.37C109.017 107.968 98.0318 97.2252 84.63 97.3746C71.2282 97.524 60.485 108.509 60.6344 121.911C60.7838 135.313 71.7691 146.056 85.1709 145.907Z" | ||
| fill="white" | ||
| stroke="black" | ||
| stroke-width="8.41298" | ||
| /> | ||
|
|
||
| {/* <!-- Right eye --> */} | ||
| <path | ||
| d="M127.511 122.531C140.913 122.382 151.656 111.396 151.507 97.9945C151.357 84.5927 140.372 73.8495 126.97 73.9989C113.569 74.1482 102.825 85.1336 102.975 98.5354C103.124 111.937 114.109 122.68 127.511 122.531Z" | ||
| fill="white" | ||
| stroke="black" | ||
| stroke-width="8.41298" | ||
| /> | ||
|
|
||
| {/* <!-- Right pupil --> */} | ||
| <path | ||
| d="M124.817 75.0371C119.025 82.8635 118.786 93.8654 124.943 102.046C130.198 109.028 138.726 112.102 146.794 110.725C152.587 102.898 152.825 91.8966 146.669 83.7158C141.413 76.7341 132.886 73.66 124.817 75.0371Z" | ||
| fill="black" | ||
| /> | ||
|
|
||
| {/* <!-- Left pupil --> */} | ||
| <path | ||
| d="M83.7629 98.3403C78.1936 106.086 78.0308 116.853 84.0684 124.879C89.0469 131.494 96.9795 134.564 104.643 133.65C110.213 125.904 110.376 115.137 104.338 107.111C99.3594 100.497 91.4268 97.426 83.7629 98.3403Z" | ||
| fill="black" | ||
| /> | ||
|
|
||
| {/* <!-- Smile --> */} | ||
| <path | ||
| d="M114.128 159.598C119.673 161.038 126.095 160.674 131.417 158.723C136.777 156.752 143.396 151.325 146.285 146.32" | ||
| stroke="black" | ||
| stroke-width="8.41298" | ||
| stroke-linecap="round" | ||
| /> | ||
|
|
||
| {/* <!-- Decorative marks --> */} | ||
| <path | ||
| d="M167.532 301.188C165.712 303.217 165.711 306.678 167.731 308.499C175.837 315.761 184.838 322.018 193.382 326.574C194.974 327.639 196.695 327.161 198.187 326.316C198.933 325.888 199.46 325.083 200.007 324.287C201.41 321.512 200.276 318.677 198.088 317.453C190.131 313.066 181.925 307.356 174.625 300.611C172.596 298.81 169.333 299.178 167.532 301.188Z" | ||
| fill="black" | ||
| /> | ||
| <path | ||
| d="M153.183 290.974L153.56 290.755C155.958 288.886 156.395 286.16 154.904 283.544C152.815 280.769 151.114 277.775 149.414 274.79C146.221 269.18 143.406 263.361 140.791 257.919C139.677 255.085 136.743 254.288 133.898 255.402C131.063 256.516 130.267 259.451 131.381 262.295C133.828 268.323 137.239 274.312 140.641 280.29C142.55 283.652 144.261 286.646 146.558 289.8C148.19 291.81 150.915 292.247 153.163 290.974L153.183 290.974Z" | ||
| fill="black" | ||
| /> | ||
| <path | ||
| d="M127.138 237.548L128.262 236.911C130.132 235.847 131.147 233.29 130.829 231.002C128.91 224.169 126.046 217.385 122.425 211.028C120.724 208.034 119.014 205.04 116.935 202.275C115.066 199.877 111.972 199.648 109.943 201.309C107.545 203.179 107.317 206.272 108.977 208.302C110.847 210.699 112.339 213.315 113.831 215.941C117.023 221.551 119.48 227.58 121.33 233.449C121.697 236.701 124.691 238.462 127.128 237.548L127.138 237.548Z" | ||
| fill="black" | ||
| /> | ||
| <path | ||
| d="M227.981 310.352C229.155 310.67 229.891 310.252 230.637 309.825C232.129 308.979 233.204 307.378 233.313 305.826C233.752 296.178 232.967 285.227 230.849 274.545C230.312 271.869 227.696 269.9 224.662 270.645C221.987 271.182 220.017 273.798 220.763 276.832C222.831 286.54 223.566 296.526 223.287 305.607C223.227 308.113 225.097 310.51 227.981 310.361L227.981 310.352Z" | ||
| fill="black" | ||
| /> | ||
| <path | ||
| d="M168.556 168.157C165.721 169.271 164.924 172.205 165.82 174.662C167.143 177.875 168.853 180.869 170.544 183.843C174.164 190.2 178.53 196.119 183.444 201.252C184.886 202.903 187.601 203.331 189.84 202.058L190.964 201.421C192.784 199.392 192.993 196.299 191.342 194.279C186.876 189.902 183.296 184.511 180.103 178.9C178.612 176.284 177.13 173.668 176.006 170.823C174.136 168.426 171.371 167.023 168.546 168.147L168.556 168.157Z" | ||
| fill="black" | ||
| /> | ||
| <path | ||
| d="M204.901 216.7C202.504 218.57 202.275 221.664 203.558 223.912C207.278 228.716 210.47 234.327 213.673 239.927C215.374 242.921 217.084 245.915 218.397 249.109C219.511 251.944 222.445 252.74 225.29 251.626L225.668 251.407C227.916 250.124 228.722 247.19 227.807 244.733C226.484 241.52 224.565 238.168 222.645 234.796C219.234 228.807 215.454 223.048 211.894 217.656C210.252 215.627 206.94 215.039 204.911 216.7L204.901 216.7Z" | ||
| fill="black" | ||
| /> | ||
| <path | ||
| d="M171.449 241.537C174.066 240.045 174.871 237.111 173.38 234.505L157.189 206.087C155.697 203.471 152.763 202.665 150.157 204.157C147.551 205.648 146.735 208.583 148.227 211.189L164.417 239.607C165.89 242.233 168.824 243.029 171.449 241.537Z" | ||
| fill="black" | ||
| /> | ||
| <path | ||
| d="M203.832 298.364C206.448 296.872 207.254 293.938 205.763 291.332L189.572 262.914C188.08 260.298 185.146 259.492 182.54 260.984C179.934 262.475 179.118 265.41 180.61 268.016L196.8 296.434C198.273 299.06 201.207 299.856 203.832 298.364Z" | ||
| fill="black" | ||
| /> | ||
|
|
||
| {/* <!-- Banner background --> */} | ||
| <path | ||
| d="M265.72 210.174H58.1795C56.3957 210.174 54.9497 211.498 54.9497 213.13V301.218C54.9497 302.851 56.3957 304.174 58.1795 304.174H265.72C267.504 304.174 268.95 302.851 268.95 301.218V213.13C268.95 211.498 267.504 210.174 265.72 210.174Z" | ||
| fill="black" | ||
| /> | ||
| <path | ||
| d="M259.901 216.174H63.9984C62.3146 216.174 60.9497 217.329 60.9497 218.753V295.596C60.9497 297.02 62.3146 298.174 63.9984 298.174H259.901C261.585 298.174 262.95 297.02 262.95 295.596V218.753C262.95 217.329 261.585 216.174 259.901 216.174Z" | ||
| fill="#F9F4F0" | ||
| /> | ||
|
|
||
| {/* <!-- BETA text --> */} | ||
| <path | ||
| d="M93.5 275.154V235.823H108.813C113.288 235.823 116.783 236.67 119.3 238.362C121.816 240.055 123.074 242.666 123.074 246.196C123.074 248.357 122.589 250.14 121.618 251.545C120.665 252.949 119.479 253.904 118.059 254.408V254.516C119.803 255.02 121.259 256.056 122.427 257.622C123.595 259.189 124.179 261.26 124.179 263.835C124.179 267.419 122.84 270.201 120.162 272.182C117.484 274.163 113.962 275.154 109.594 275.154H93.5ZM102.828 268.698H109.217C111.122 268.698 112.524 268.211 113.423 267.239C114.321 266.248 114.77 264.907 114.77 263.214C114.77 261.557 114.321 260.243 113.423 259.27C112.524 258.28 111.122 257.784 109.217 257.784H102.828V268.698ZM102.828 251.815H108.462C110.349 251.815 111.706 251.4 112.533 250.572C113.378 249.744 113.8 248.573 113.8 247.06C113.8 245.548 113.378 244.386 112.533 243.576C111.706 242.747 110.349 242.333 108.462 242.333H102.828V251.815Z" | ||
| fill="black" | ||
| /> | ||
| <path | ||
| d="M130.326 275.154V235.823H157.365V242.9H139.653V251.491H154.939V258.406H139.653V268.13H157.662V275.154H130.326Z" | ||
| fill="black" | ||
| /> | ||
| <path d="M160.466 242.9V235.823H192.493V242.9H181.197V275.154H171.761V242.9H160.466Z" fill="black" /> | ||
| <path | ||
| d="M189.958 275.154L204.759 235.661H214.922L230.046 275.154H219.91L210.178 248.114C209.962 247.502 209.791 246.916 209.665 246.358C209.539 245.8 209.432 245.251 209.342 244.71H209.126C209.036 245.251 208.928 245.809 208.803 246.385C208.677 246.943 208.506 247.52 208.29 248.114L198.639 275.154H189.958ZM199.044 267.077L201.038 260.27H218.076L220.098 267.077H199.044Z" | ||
| fill="black" | ||
| /> | ||
| </svg> |
There was a problem hiding this comment.
🧩 Analysis chain
Fix SVG attribute casing for TSX/React compatibility
The inline PeanutIcon SVG uses kebab‑case SVG attributes like stroke-width and stroke-linecap. In TSX/React these should be camelCased (strokeWidth, strokeLinecap, etc.), otherwise you’ll hit type errors and potentially lose some attribute behavior.
Example of the change you should apply throughout the SVG:
-<path
- d="M60.3258 45.632..."
- fill="#FFC900"
- stroke="black"
- stroke-width="12.6195"
-/>
+<path
+ d="M60.3258 45.632..."
+ fill="#FFC900"
+ stroke="black"
+ strokeWidth="12.6195"
+/>
...
-<path d="M106.78 163.414L112.666 153.471" stroke="black" stroke-width="8.41298" stroke-linecap="round" />
+<path d="M106.78 163.414L112.666 153.471" stroke="black" strokeWidth="8.41298" strokeLinecap="round" />Search for all stroke-width, stroke-linecap, and similar SVG attributes and convert them to their camelCase JSX equivalents.
I'll verify this review comment by checking the actual file and confirming the SVG attribute requirements.
🏁 Script executed:
#!/bin/bash
# Find and examine the OfflineScreen file
find . -type f -name "index.tsx" | grep -i offlinescreen | head -5Length of output: 47
🏁 Script executed:
#!/bin/bash
# Check file content for kebab-case SVG attributes
if [ -f "src/components/Global/OfflineScreen/index.tsx" ]; then
echo "=== Checking for kebab-case SVG attributes ==="
grep -n "stroke-width\|stroke-linecap\|stroke-dasharray\|fill-opacity\|stroke-dashoffset" "src/components/Global/OfflineScreen/index.tsx" | head -20
echo ""
echo "=== File excerpt (first 50 lines) ==="
head -50 "src/components/Global/OfflineScreen/index.tsx"
fiLength of output: 3936
🏁 Script executed:
#!/bin/bash
# Verify if other SVG files in the project use camelCase
echo "=== Checking SVG attribute patterns in .tsx/.jsx files ==="
rg -n "strokeWidth|strokeLinecap" src/ --type=ts --type=tsx -c | head -10Length of output: 86
🌐 Web query:
Does React/TSX require camelCase for SVG attributes like strokeWidth instead of stroke-width?
💡 Result:
Yes — in JSX/TSX you should use the camelCase DOM/JS property names (e.g. strokeWidth) rather than hyphenated SVG attribute names (stroke-width). React converts attributes to JS object keys and will warn about unknown DOM properties. Exceptions: aria-* and data-* stay hyphenated. [1][2]
Sources:
[1] React docs — Writing Markup with JSX (camelCase convention, example strokeWidth; aria/data exception).
[2] MDN / SVG reference (SVG attributes like stroke/stroke-width; shows original names used by SVG).
[/skip_cloning]
🏁 Script executed:
#!/bin/bash
# Count all kebab-case SVG attributes in the file
echo "=== Total kebab-case SVG attributes found ==="
grep -o "stroke-width\|stroke-linecap\|stroke-dasharray\|fill-opacity\|stroke-dashoffset" "src/components/Global/OfflineScreen/index.tsx" | sort | uniq -c
echo ""
echo "=== Total count ==="
grep -c "stroke-width\|stroke-linecap" "src/components/Global/OfflineScreen/index.tsx"
echo ""
echo "=== Unique kebab-case attribute types ==="
grep -o '\b[a-z]*-[a-z]*=' "src/components/Global/OfflineScreen/index.tsx" | sort | uniqLength of output: 197
🏁 Script executed:
#!/bin/bash
# Show all lines with kebab-case attributes for complete picture
echo "=== All occurrences of kebab-case SVG attributes ==="
grep -n "stroke-width\|stroke-linecap" "src/components/Global/OfflineScreen/index.tsx"Length of output: 359
Fix SVG attribute casing for TSX/React compatibility
The inline PeanutIcon SVG uses kebab-case SVG attributes. In TSX/React, these must be camelCased (e.g., strokeWidth, strokeLinecap), otherwise TypeScript will error on unknown DOM properties and attributes won't render correctly.
Fix the following 6 occurrences in src/components/Global/OfflineScreen/index.tsx:
- Line 19:
stroke-width="12.6195"→strokeWidth="12.6195" - Line 23:
stroke-width="8.41298"→strokeWidth="8.41298"andstroke-linecap="round"→strokeLinecap="round" - Line 30:
stroke-width="8.41298"→strokeWidth="8.41298" - Line 38:
stroke-width="8.41298"→strokeWidth="8.41298" - Line 57:
stroke-width="8.41298"→strokeWidth="8.41298" - Line 58:
stroke-linecap="round"→strokeLinecap="round"
🤖 Prompt for AI Agents
In src/components/Global/OfflineScreen/index.tsx around lines 5 to 119, the
inline PeanutIcon SVG uses kebab-case SVG attributes which are invalid in
JSX/TSX; replace the occurrences with their camelCased React equivalents: change
stroke-width to strokeWidth (all occurrences), stroke-linecap to strokeLinecap
(all occurrences), and stroke-linejoin if present similarly; specifically update
the six spots called out in the review (line ~19: stroke-width → strokeWidth,
line ~23: stroke-width → strokeWidth and stroke-linecap → strokeLinecap, line
~30: stroke-width → strokeWidth, line ~38: stroke-width → strokeWidth, line ~57:
stroke-width → strokeWidth, line ~58: stroke-linecap → strokeLinecap), ensuring
you update attribute names everywhere in the SVG and keep numeric/string values
unchanged.
| // wait for both user and invite data to be loaded | ||
| if (!user?.user || !inviteCodeData || isLoading || isFetchingUser) { | ||
| return | ||
| } | ||
|
|
||
| // If user has app access and invite is valid, redirect to inviter's profile, if a campaign is provided, award the badge and redirect to the home page | ||
| // prevent running the effect multiple times (ref doesn't trigger re-renders) | ||
| if (hasStartedAwardingRef.current) { | ||
| return | ||
| } | ||
|
|
||
| // if user has app access and invite is valid, handle redirect | ||
| if (!redirectUri && user.user.hasAppAccess && inviteCodeData.success && inviteCodeData.username) { | ||
| // If the potential ambassador is already a peanut user, simply award the badge and redirect to the home page | ||
| // if campaign is present, award the badge and redirect to home | ||
| if (campaign) { | ||
| invitesApi.awardBadge(campaign).then(() => { | ||
| router.push('/home') | ||
| }) | ||
| hasStartedAwardingRef.current = true | ||
| setIsAwardingBadge(true) | ||
| invitesApi | ||
| .awardBadge(campaign) | ||
| .then(async () => { | ||
| // refetch user data to get the newly awarded badge | ||
| await fetchUser() | ||
| router.push('/home') | ||
| }) | ||
| .catch(async () => { | ||
| // if badge awarding fails, still refetch and redirect | ||
| await fetchUser() | ||
| router.push('/home') | ||
| }) | ||
| .finally(() => { | ||
| setIsAwardingBadge(false) | ||
| }) | ||
| } else { | ||
| // no campaign, just redirect to inviter's profile | ||
| router.push(`/${inviteCodeData.username}`) | ||
| } | ||
| } | ||
| }, [user, inviteCodeData, isLoading, isFetchingUser, router, campaign, redirectUri]) | ||
| }, [user, inviteCodeData, isLoading, isFetchingUser, router, campaign, redirectUri, fetchUser]) |
There was a problem hiding this comment.
Redirect + badge award flow can get stuck if fetchUser repeatedly fails
The redirect effect correctly:
- Waits for
userandinviteCodeDatato be loaded. - Avoids re-running via
hasStartedAwardingRef. - Awards a badge when
campaignis present, then refetches user and redirects; otherwise redirects to the inviter’s profile.
However, there’s a subtle failure mode in the badge path:
invitesApi
.awardBadge(campaign)
.then(async () => {
await fetchUser()
router.push('/home')
})
.catch(async () => {
await fetchUser()
router.push('/home')
})
.finally(() => {
setIsAwardingBadge(false)
})If fetchUser() rejects both in the .then and again in the .catch handler, router.push('/home') is never reached, but:
hasStartedAwardingRef.currentremainstrue(so the effect won’t try again).- The other effect will still consider the user “has app access + invite success”, so
shouldShowContentstaysfalse.
Result: the user can be left on an infinite loading state with no redirect path.
To make this resilient, you can:
- Treat redirect to
/homeas unconditional in.finally. - Wrap
fetchUser()in a safe helper that swallows/logs errors so a failure there doesn’t block navigation.
Suggested refactor:
- if (campaign) {
- hasStartedAwardingRef.current = true
- setIsAwardingBadge(true)
- invitesApi
- .awardBadge(campaign)
- .then(async () => {
- // refetch user data to get the newly awarded badge
- await fetchUser()
- router.push('/home')
- })
- .catch(async () => {
- // if badge awarding fails, still refetch and redirect
- await fetchUser()
- router.push('/home')
- })
- .finally(() => {
- setIsAwardingBadge(false)
- })
- } else {
+ if (campaign) {
+ hasStartedAwardingRef.current = true
+ setIsAwardingBadge(true)
+
+ const safeFetchUser = async () => {
+ try {
+ await fetchUser()
+ } catch (e) {
+ console.error('Error refetching user after awarding badge', e)
+ }
+ }
+
+ invitesApi
+ .awardBadge(campaign)
+ .catch((e) => {
+ console.error('Error awarding badge:', e)
+ })
+ .finally(async () => {
+ // Always attempt to refresh user data, but never block redirect on it
+ await safeFetchUser()
+ router.push('/home')
+ setIsAwardingBadge(false)
+ })
+ } else {
// no campaign, just redirect to inviter's profile
router.push(`/${inviteCodeData.username}`)
}Optionally, if you want to guard against repeated redirects in the no‑campaign branch as well (e.g., in React StrictMode dev builds), you could also set hasStartedAwardingRef.current = true there, but that’s less critical than fixing the stuck-loading scenario.
🤖 Prompt for AI Agents
In src/components/Invites/InvitesPage.tsx around lines 83 to 119, the
badge-award flow can get stuck if fetchUser() rejects and prevents
router.push('/home') from running; change the flow so navigation is
unconditional: keep setting hasStartedAwardingRef.current = true and
setIsAwardingBadge(true) before calling invitesApi.awardBadge(campaign), but
move router.push('/home') into a finally block that always runs regardless of
awardBadge or fetchUser failures; additionally wrap fetchUser() calls in a safe
helper (or try/catch) that logs/swallow errors so a rejected fetchUser cannot
block execution, and optionally set hasStartedAwardingRef.current = true in the
no-campaign branch as well to avoid double runs in StrictMode.
| const handleToggleChange = async () => { | ||
| if (isToggleLoading) return | ||
| const newValue = !showFullName | ||
| setShowFullName(newValue) | ||
|
|
||
| setIsToggleLoading(true) | ||
| try { | ||
| setShowFullName(!showFullName) | ||
| await updateUserById({ | ||
| userId: user?.user.userId, | ||
| showFullName: !showFullName, | ||
| // Fire-and-forget: don't await fetchUser() to allow quick navigation | ||
| updateUserById({ | ||
| userId: user?.user.userId, | ||
| showFullName: newValue, | ||
| }) | ||
| .then(() => { | ||
| // Refetch user data in background without blocking | ||
| fetchUser() | ||
| }) | ||
| .catch((error) => { | ||
| console.error('Failed to update preferences:', error) | ||
| // Revert on error | ||
| setShowFullName(!newValue) | ||
| }) |
There was a problem hiding this comment.
Critical: Error handling is broken and will never revert state on failure.
The .catch block will never execute because updateUserById never throws—it always resolves with either { data } or { error }. When an error occurs, the API returns { error: "..." }, which still triggers the .then block, causing fetchUser() to be called even after a failed update. The error property is never checked, and the state is never reverted.
Apply this diff to properly handle errors:
- // Fire-and-forget: don't await fetchUser() to allow quick navigation
updateUserById({
userId: user?.user.userId,
showFullName: newValue,
})
- .then(() => {
+ .then((result) => {
+ if (result.error) {
+ console.error('Failed to update preferences:', result.error)
+ // Revert on error
+ setShowFullName(!newValue)
+ return
+ }
// Refetch user data in background without blocking
fetchUser()
})
.catch((error) => {
- console.error('Failed to update preferences:', error)
+ console.error('Unexpected error updating preferences:', error)
// Revert on error
setShowFullName(!newValue)
})| await updateUserById({ | ||
| userId: user?.user.userId, | ||
| showFullName: !showFullName, | ||
| // Fire-and-forget: don't await fetchUser() to allow quick navigation |
There was a problem hiding this comment.
Fix misleading comment.
The comment references fetchUser() but the function being called without await on the next line is updateUserById.
Apply this diff:
- // Fire-and-forget: don't await fetchUser() to allow quick navigation
+ // Optimistic update: call API without blocking UI📝 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.
| // Fire-and-forget: don't await fetchUser() to allow quick navigation | |
| // Optimistic update: call API without blocking UI |
🤖 Prompt for AI Agents
In src/components/Profile/components/ShowNameToggle.tsx around line 15, the
inline comment incorrectly references fetchUser() while the actual
fire-and-forget call is updateUserById; update the comment to refer to
updateUserById (or make it generic like "don't await updateUserById to allow
quick navigation / fire-and-forget") so the comment accurately describes the
code behavior.
| shortenStringLong, | ||
| } from '@/utils' | ||
| import React from 'react' | ||
| import React, { lazy, Suspense } from 'react' |
There was a problem hiding this comment.
🧩 Analysis chain
Lazy-loaded TransactionDetailsDrawer still imports eagerly; gate render by isDrawerOpen
The TransactionDetailsDrawer is now wrapped in React.lazy + Suspense + LazyLoadErrorBoundary, which is good, but because it’s always rendered (even when closed), the lazy chunk will still be requested as soon as the TransactionCard mounts. The isOpen prop being false does not prevent the lazy component from being evaluated, so we don’t actually defer the network/parse cost until the user taps a transaction, contrary to the comment on Lines 25–27.
You likely want the drawer tree itself to be conditionally rendered so the lazy chunk is only pulled once a transaction is opened.
A minimal refactor:
- {/* Transaction Details Drawer */}
- <LazyLoadErrorBoundary>
- <Suspense fallback={null}>
- <TransactionDetailsDrawer
- isOpen={isDrawerOpen && selectedTransaction?.id === transaction.id}
- onClose={closeTransactionDetails}
- transaction={selectedTransaction}
- transactionAmount={displayAmount}
- avatarUrl={avatarUrl}
- />
- </Suspense>
- </LazyLoadErrorBoundary>
+ {/* Transaction Details Drawer */}
+ {isDrawerOpen && selectedTransaction?.id === transaction.id && (
+ <LazyLoadErrorBoundary>
+ <Suspense fallback={null}>
+ <TransactionDetailsDrawer
+ isOpen={true}
+ onClose={closeTransactionDetails}
+ transaction={selectedTransaction}
+ transactionAmount={displayAmount}
+ avatarUrl={avatarUrl}
+ />
+ </Suspense>
+ </LazyLoadErrorBoundary>
+ )}This keeps behavior the same but ensures the chunk is only loaded once the user actually opens a transaction. Please verify against your lazy‑loading patterns elsewhere in the app.
Also applies to: 25-34, 187-197
I'll verify the lazy-loading implementation by examining the actual code in the repository.
🏁 Script executed:
#!/bin/bash
# View the TransactionCard.tsx file to verify the lazy-loading setup
cat "src/components/TransactionDetails/TransactionCard.tsx"Length of output: 10509
🏁 Script executed:
#!/bin/bash
# Search for how TransactionDetailsDrawer is lazy-loaded and used elsewhere
rg -n "TransactionDetailsDrawer" -A 5 -B 5 --type ts --type tsxLength of output: 27
🏁 Script executed:
#!/bin/bash
# Check for lazy-loading patterns in the components directory
rg -n "lazy\(" src/components --type ts --type tsx | head -20Length of output: 27
I need to search for other lazy-loading patterns using corrected rg syntax:
🏁 Script executed:
#!/bin/bash
# Search for lazy() patterns using corrected rg syntax
rg -n "lazy\(" src/components -A 10 | head -40Length of output: 966
🏁 Script executed:
#!/bin/bash
# Look for conditional rendering patterns around lazy-loaded components
rg -n "lazy\(" --glob="*.tsx" --glob="*.ts" -A 15 src/components | grep -A 15 "import"Length of output: 1241
🏁 Script executed:
#!/bin/bash
# Check if TransactionDetailsDrawer has an isOpen prop that controls visibility
fd "TransactionDetailsDrawer.tsx" src/components --exec head -50 {}Length of output: 1790
🏁 Script executed:
#!/bin/bash
# Find all lazy-loaded components and check if they're conditionally rendered
rg -n "lazy\(" -A 20 src/components | grep -B 5 -A 15 "return\|{.*&&"Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check if conditional rendering pattern is used with lazy components anywhere
rg -n "&&.*lazy\(|{.*&&.*<" src/components -A 5 -B 5Length of output: 105
🏁 Script executed:
#!/bin/bash
# Search for pattern: conditional render + lazy component
rg -n "\{.*&&" src/components -A 10 | grep -A 10 "Suspense\|lazy"Length of output: 1504
Conditional rendering required to defer lazy chunk loading until drawer opens
The review comment's concern is verified as correct. The TransactionDetailsDrawer lazy-loaded component is currently unconditionally rendered (lines 187–197), which means React downloads and evaluates the chunk when TransactionCard mounts, not when the user actually opens the drawer.
While the isOpen prop controls the drawer's visibility, it does not prevent the lazy chunk from being downloaded. React's lazy-loading defers code splitting but requires unconditional rendering to also defer instantiation. The component must be conditionally rendered in the JSX tree for the chunk to be loaded only when needed.
The suggested refactor is correct: wrap the entire LazyLoadErrorBoundary + Suspense + TransactionDetailsDrawer tree in a conditional so the lazy chunk is only pulled once isDrawerOpen && selectedTransaction?.id === transaction.id becomes true.
🤖 Prompt for AI Agents
In src/components/TransactionDetails/TransactionCard.tsx around line 16, the
lazy-loaded TransactionDetailsDrawer subtree is currently rendered
unconditionally which causes its chunk to be downloaded when TransactionCard
mounts; change the JSX to only render the entire LazyLoadErrorBoundary +
Suspense + TransactionDetailsDrawer block when the drawer should actually open
(e.g., wrap it in a conditional like isDrawerOpen && selectedTransaction?.id ===
transaction.id) so the lazy chunk is requested only when the drawer is opened,
preserving the existing Suspense fallback and error boundary.
No description provided.