fix: refetch wallet balance after claim/req-pay/send flow completion#665
fix: refetch wallet balance after claim/req-pay/send flow completion#665kushagrasarathe merged 4 commits intopeanut-walletfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis pull request enhances wallet balance updating across several components. The changes integrate a Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ClaimComponent
participant WalletHook
User->>ClaimComponent: Initiate claim transaction
ClaimComponent->>WalletHook: Call claimLink()
Note over ClaimComponent, WalletHook: Transaction succeeds
ClaimComponent->>WalletHook: Call refetchBalances(address)
WalletHook-->>ClaimComponent: Return updated balance
sequenceDiagram
participant User
participant CreateComponent
participant WalletHook
User->>CreateComponent: Start link creation
CreateComponent->>WalletHook: Call createLinkWrapper()
Note over CreateComponent, WalletHook: Transaction succeeds
CreateComponent->>WalletHook: Call refetchBalances(selectedWallet?.address || '')
WalletHook-->>CreateComponent: Return updated balance
sequenceDiagram
participant User
participant PaymentComponent
participant WalletHook
User->>PaymentComponent: Initiate payment request
PaymentComponent->>PaymentComponent: Process transaction
Note over PaymentComponent: Transaction successful
PaymentComponent->>WalletHook: Call refetchBalances(address)
WalletHook-->>PaymentComponent: Return updated wallet balance
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/components/Request/Pay/Views/Success.view.tsx (1)
31-80: Optimize useEffect dependencies and consolidate state management.Based on previous learnings,
transactionHashshould not be in dependency arrays as it remains constant in success view. Additionally, the component has complex state management spread across multiple effects.Consider these improvements:
- Remove
transactionHashfrom dependency arrays:- }, [explorerUrlDestChainWithTxHash, requestLinkData, address]) + }, [explorerUrlDestChainWithTxHash, requestLinkData, address]) - }, [isXChain, requestLinkData, transactionHash, address, selectedTokenAddress]) + }, [isXChain, requestLinkData, address, selectedTokenAddress])
- Consolidate the transaction submission logic into a single effect or custom hook to improve maintainability:
const useTransactionSubmission = (params) => { useEffect(() => { const submitTransaction = async () => { if (shouldSubmit) { await peanut.submitRequestLinkFulfillment(/* ... */); await utils.saveRequestLinkFulfillmentToLocalStorage(/* ... */); } }; submitTransaction(); }, [/* dependencies */]); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/components/Claim/useClaimLink.tsx(2 hunks)src/components/Create/useCreateLink.tsx(1 hunks)src/components/Request/Pay/Views/Initial.view.tsx(2 hunks)src/components/Request/Pay/Views/Success.view.tsx(1 hunks)
🧰 Additional context used
📓 Learnings (2)
src/components/Request/Pay/Views/Success.view.tsx (1)
Learnt from: jjramirezn
PR: peanutprotocol/peanut-ui#409
File: src/components/Request/Pay/Views/Success.view.tsx:23-23
Timestamp: 2024-11-12T09:39:20.720Z
Learning: In the `SuccessView` component, `transactionHash` remains constant because we are in a success view, so it's unnecessary to include it in the dependency array of `useEffect` hooks.
src/components/Request/Pay/Views/Initial.view.tsx (1)
Learnt from: jjramirezn
PR: peanutprotocol/peanut-ui#410
File: src/components/Request/Pay/Views/Initial.view.tsx:87-93
Timestamp: 2024-11-12T09:39:20.720Z
Learning: When refactoring to eliminate code duplication, prioritize readability and consider whether the change significantly improves the code. If it doesn't enhance readability or maintainability, it's acceptable to keep the existing code structure.
🔇 Additional comments (3)
src/components/Claim/useClaimLink.tsx (1)
14-14: LGTM! Wallet balance is now properly refreshed after claiming.The integration of
refetchBalancesand its placement after the successful claim transaction ensures that the wallet balance stays up-to-date.Also applies to: 29-30
src/components/Request/Pay/Views/Initial.view.tsx (1)
83-83: LGTM! Wallet balance is now properly refreshed after payment request.The integration of
refetchBalancesand its placement after the successful payment transaction ensures that the wallet balance stays up-to-date.Also applies to: 337-338
src/components/Create/useCreateLink.tsx (1)
764-765: LGTM! Improved clarity in wallet balance refresh after link creation.The change to use
selectedWallet?.addressis more explicit and clearer than using the genericaddressvariable, making the code's intent more obvious.
| }) | ||
|
|
||
| // refetch wallet balance after successful claim | ||
| await refetchBalances(address) |
There was a problem hiding this comment.
good catch, probably not, removed it
|
|
||
| await refetchBalances(address ?? '') | ||
| // refetch wallet balance after successful link creation | ||
| await refetchBalances(selectedWallet?.address || '') |
| onNext() | ||
|
|
||
| // update wallet balance after successful req payment | ||
| await refetchBalances(address) |
There was a problem hiding this comment.
- same
- flag: repetitive code. Would be good to at least evaluate a more holistic approach (even if actually implementing it is high effort and we go with the more hacky short term solution)
There was a problem hiding this comment.
not sure what you mean by repetitive code here, but we are already reusing the refetchBalances fn from wallet hook, and these are three different actions that are being done where refetchBalances is used, create/pay/claim flows, where we need to refetch the balance
| </div> | ||
| ) | ||
| } | ||
| // todo: manage loading state properly, currently it keeps loading forever |
There was a problem hiding this comment.
This is pending on request v2?
There was a problem hiding this comment.
the loading state was buggy, it kept loading forever, so to smoothen the ux, removed it for a while, yes, in req v2 will have it improved
Summary by CodeRabbit
New Features
Bug Fixes