-
Notifications
You must be signed in to change notification settings - Fork 13
add buttons to swap page #186
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Warning Rate limit exceeded@bordalix has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 8 minutes and 16 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughAdded UI state and action flow to the Boltz Swap screen to conditionally claim or refund HTLCs via the swap provider, refresh swap status after the action, surface success/error UI, updated swap history init to refresh statuses first, and applied a minor formatting edit. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SwapUI as Boltz Swap Component
participant Handler as buttonHandler
participant Provider as swapProvider
participant State as Local State
User->>SwapUI: Click action button
SwapUI->>Handler: invoke buttonHandler
Handler->>State: set cooking = true
alt reverse swap & claimable
Handler->>Provider: claimVHTLC(swapId)
else submarine swap & refundable
Handler->>Provider: refundVHTLC(swapId)
end
Handler->>Provider: refreshSwapsStatus()
Provider-->>Handler: updated status
alt action succeeded
Handler->>State: set success = true
Handler->>State: set cooking = false
SwapUI->>User: show success Info
else action failed
Provider-->>Handler: throws error
Handler->>State: set error = extractedError
Handler->>State: set cooking = false
SwapUI->>User: show ErrorMessage
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Deploying wallet-signet with
|
| Latest commit: |
2ff59b6
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://2f0f32b2.wallet-23u.pages.dev |
| Branch Preview URL: | https://buttons-on-swaps.wallet-23u.pages.dev |
Deploying wallet-mutinynet with
|
| Latest commit: |
2ff59b6
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://370fe59b.arkade-wallet.pages.dev |
| Branch Preview URL: | https://buttons-on-swaps.arkade-wallet.pages.dev |
Deploying wallet-bitcoin with
|
| Latest commit: |
2ff59b6
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://a99eb991.wallet-bitcoin.pages.dev |
| Branch Preview URL: | https://buttons-on-swaps.wallet-bitcoin.pages.dev |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/screens/Apps/Boltz/Swap.tsx (1)
59-61: Consider making the button label logic more explicit.The
buttonLabellogic assumes a binary state (either claimable or refundable), but doesn't handle the case where neither is true. While theButtonscomponent protects against this by conditional rendering, this creates fragile coupling between the two.Apply this diff for more explicit logic:
const isRefundable = isSubmarineSwapRefundable(swapInfo) const isClaimable = isReverseClaimableStatus(swapInfo.status) -const buttonLabel = isClaimable ? 'Complete swap' : 'Refund swap' +const buttonLabel = isClaimable ? 'Complete swap' : isRefundable ? 'Refund swap' : ''
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
package.json(1 hunks)src/screens/Apps/Boltz/Swap.tsx(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/screens/Apps/Boltz/Swap.tsx (11)
src/providers/flow.tsx (1)
FlowContext(82-95)src/providers/lightning.tsx (1)
LightningContext(24-30)src/lib/error.ts (1)
extractError(1-10)src/lib/logs.ts (1)
consoleError(40-44)src/components/ButtonsOnBottom.tsx (1)
ButtonsOnBottom(10-26)src/components/Button.tsx (1)
Button(20-62)src/components/Error.tsx (1)
ErrorMessage(10-21)src/components/Info.tsx (1)
Info(14-30)src/icons/CheckMark.tsx (1)
CheckMarkIcon(1-11)src/components/Text.tsx (1)
TextSecondary(91-97)src/components/Table.tsx (1)
Table(6-24)
🔇 Additional comments (5)
src/screens/Apps/Boltz/Swap.tsx (4)
1-21: LGTM!The new imports are properly used throughout the component to add claim/refund functionality.
26-31: LGTM!The swapProvider from context and the local state variables (error, cooking, success) are properly integrated for managing the claim/refund operations.
79-84: LGTM!The conditional rendering logic correctly shows the button only when an action is available and not yet succeeded. The button is properly disabled during operations.
102-102: LGTM!The button is properly positioned at the bottom of the layout.
package.json (1)
7-7: No issues found—package version is current and secure.The latest version of @arkade-os/boltz-swap on npm is 0.2.5, matching the version specified in package.json, and no known security vulnerabilities were detected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/components/SwapsList.tsx (1)
121-126: Consider adding a loading state.Users won't see any feedback while swap data is being fetched and refreshed. Adding a loading state would improve the user experience by providing visual feedback during the async operations.
You could add a loading state like this:
export default function SwapsList() { const { swapProvider } = useContext(LightningContext) const [swapHistory, setSwapHistory] = useState<(PendingReverseSwap | PendingSubmarineSwap)[]>([]) + const [loading, setLoading] = useState(true) useEffect(() => { const choresOnInit = async () => { if (!swapProvider) return + setLoading(true) try { await swapProvider.refreshSwapsStatus() setSwapHistory(await swapProvider.getSwapHistory()) } catch (error) { console.error('Failed to fetch swap history:', error) + } finally { + setLoading(false) } } choresOnInit() }, [swapProvider]) + if (loading) return <div>Loading...</div> if (swapHistory.length === 0) return <EmptySwapList />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/SwapsList.tsx(1 hunks)src/screens/Apps/Boltz/Index.tsx(1 hunks)
🔇 Additional comments (1)
src/screens/Apps/Boltz/Index.tsx (1)
27-27: LGTM!Formatting improvement that adds visual separation between the component definition and the return statement.
@tiero please review
Summary by CodeRabbit
New Features
Improvement
Style