feat: deposit modal implementation #98
Conversation
WalkthroughAdds modal-driven multi-step Deposit and Withdraw flows with new pages, hooks, components, styles, modal state/component and provider integration, fee estimation and submission updates (BridgeTxResult/onCompleted), wallet-icon hook, AnimatedHeight/input improvements, skip-assets/all-balances queries, and Vite example buttons. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant App
participant Modal
participant Form as TransferForm
participant Assets
participant Route as RouteQuery
participant Fee as FeeEstimator
participant Bridge as Bridge/Skip
User->>App: click openDeposit/openWithdraw
App->>Modal: openModal(path, options)
Modal->>Form: mount Deposit/Withdraw form
User->>Form: select local asset
Form->>Assets: fetch external options & balances
Assets-->>Form: return assets & balances
User->>Form: enter quantity
Form->>Route: debounce -> query routes
Route-->>Form: return route & pricing
User->>Form: confirm transfer
Form->>Fee: simulate / estimate gas
Fee-->>Form: return gas / fee options
Form->>Bridge: submitTx/requestTx with chosen fee
Bridge-->>Form: tx result (txhash / status)
Form->>Modal: render TransferCompleted view
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro Cache: Disabled due to data retention organization setting Knowledge base: Disabled due to 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (2)
Comment |
Deploying interwovenkit-staging with
|
| Latest commit: |
cb7029a
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://4f3433dd.interwovenkit-staging.pages.dev |
| Branch Preview URL: | https://feat-deposit-withdraw.interwovenkit-staging.pages.dev |
Deploying interwovenkit-testnet with
|
| Latest commit: |
cb7029a
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://d99549c6.interwovenkit-testnet.pages.dev |
| Branch Preview URL: | https://feat-deposit-withdraw.interwovenkit-testnet.pages.dev |
Deploying interwovenkit with
|
| Latest commit: |
cb7029a
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://fe1a0175.interwovenkit.pages.dev |
| Branch Preview URL: | https://feat-deposit-withdraw.interwovenkit.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (15)
packages/interwovenkit-react/src/pages/deposit/SelectSrcAsset.module.css (1)
1-119: Consider adding WebKit scrollbar-hiding rule for.listYou already hide the scrollbar via
scrollbar-widthand-moz-scrollbars-none; if you also want this in WebKit, consider adding:.list::-webkit-scrollbar { display: none; }This keeps behavior consistent across browsers.
packages/interwovenkit-react/src/data/ui.ts (1)
60-79: Avoid double “Widget Closed” analytics on disconnect
useDisconnectcalls bothcloseDrawer()andcloseModal(), each of which tracks"Widget Closed". If only one surface is visible, this still emits two close events, which may skew analytics. Consider a sharedcloseWidget()helper that:
- Closes both atoms, but
- Emits
"Widget Closed"only once based on(isDrawerOpen || isModalOpen).packages/interwovenkit-react/src/pages/deposit/hooks.ts (1)
10-38: Overall hook design is cohesive; consider minor reuse to reduce duplicationThe deposit hooks are nicely layered (form → selected asset → candidate chains → filtered assets), and the
useAllBalancesQueryquery shape matches how you consume it.There’s some repeated logic filtering out external Cosmos chains (
chain.chain_type === "cosmos" && chain.bech32_prefix !== "init") in bothuseDepositAssetsanduseFilteredDepositAssets. If this grows further, you might want a small helper (e.g.isSupportedDepositChain(chain)) to centralize that condition and avoid drift.This is non-blocking; the current implementation is clear and correct aside from the balance indexing issue above.
Also applies to: 41-61, 63-107, 109-119
packages/interwovenkit-react/src/public/app/WidgetHeader.tsx (1)
9-9: Header wallet icon handling is fine; consider a fallback when no icon is availableSwitching to
useConnectedWalletIconand gating the header onaddresssimplifies the component and keeps it aligned with the global connection state fromuseInterwovenKit.One small robustness improvement:
useConnectedWalletIconcan returnundefined, andImageis currently always rendered withsrc={icon}. If there’s ever a connector without an icon, you may want to:{icon && <Image src={icon} width={18} height={18} />}or provide a default placeholder icon to avoid layout shifts.
Non-blocking; the current implementation will still work in most cases.
Also applies to: 16-18, 59-61, 64-75
examples/vite/src/Deposit.tsx (1)
1-16: ExampleDepositcomponent correctly wires touseInterwovenKitThe component cleanly uses
useInterwovenKit’saddressandopenDeposit, returningnullwhen disconnected and triggering the modal on click. This is a good minimal example of how consumers should integrate the new deposit flow.You could slightly simplify the handler to
onClick={openDeposit}instead ofonClick={() => openDeposit()}, but that’s purely stylistic.examples/vite/src/Providers.tsx (1)
46-52: ConfirmdepositOptionschain IDs for testnet/mainnet setupsPassing
depositOptionsdirectly intoInterwovenKitProvideris the right place to expose these, and the structure matches the expected{ denom, chainId }shape.However, both entries are hard-coded to
chainId: "interwoven-1"and are applied regardless ofisTestnet. Since you already toggle other config viaisTestnet ? TESTNET : {}, it’s worth double-checking that:
- These options are valid for your mainnet AND testnet environments, or
- You intentionally only want them on mainnet (in which case, you may want to conditionally set them, or derive them from the same config used inside the library to avoid drift with that long IBC denom).
Functionally this is fine, but configuration mismatches here would only surface at runtime when trying to deposit.
packages/interwovenkit-react/src/public/app/Modal.module.css (1)
10-25: Consider adding responsive width and overflow handling.The modal has a fixed
width: 400pxwhich may cause issues on smaller viewports (mobile devices). Additionally, consider addingoverflow-y: autoto handle content that exceedsmax-height..modal { position: fixed; top: 50%; left: 50%; z-index: 9999; - width: 400px; + width: min(400px, calc(100vw - 32px)); background-color: var(--gray-9); border: 1px solid var(--gray-6); border-radius: 20px; font-family: var(--font-family); max-height: 90vh; + overflow-y: auto; padding: 16px 20px; transform: translate(-50%, -50%); }packages/interwovenkit-react/src/pages/deposit/SelectDstAsset.tsx (1)
17-39: Handle empty options and missing logo gracefully.Two potential UX issues:
- If
optionsis empty (no matching assets), the user sees only the title with no feedback.logo_uriis optional inRouterAsset- if undefined, the image will fail to load.return ( <> <h3 className={styles.title}>Select an asset to receive</h3> <div className={styles.list}> - {options.map(({ denom, chain_id, symbol, logo_uri }) => ( + {options.length === 0 && <p>No deposit options available</p>} + {options.map(({ denom, chain_id, symbol, logo_uri }) => ( <button className={styles.asset} key={`${denom}-${chain_id}`} onClick={() => { setValue("dstDenom", denom) setValue("dstChainId", chain_id) // reset other values setValue("quantity", "") setValue("srcDenom", "") setValue("srcChainId", "") }} > - <img src={logo_uri} alt={symbol} /> + {logo_uri && <img src={logo_uri} alt={symbol} />} {symbol} </button> ))} </div> </> )packages/interwovenkit-react/src/public/app/Modal.tsx (1)
14-24: Backdrop click may triggercloseModaltwice.Both
onOpenChange(line 14) and the backdrop'sonClick(line 16) callcloseModal(). When clicking the backdrop, the Dialog component may fireonOpenChangeinternally, resulting in duplicate close calls.Consider removing the explicit
onClickon the backdrop if the Dialog handles it:<Dialog.Root open={isModalOpen} onOpenChange={(isOpen) => !isOpen && closeModal()}> <Dialog.Portal container={portalContainer}> - <Dialog.Backdrop className={styles.backdrop} onClick={closeModal} /> + <Dialog.Backdrop className={styles.backdrop} /> <Dialog.Popup className={clsx(styles.modal)}>Also,
clsx(styles.modal)on line 17 can be simplified to juststyles.modalsince there's only one class.packages/interwovenkit-react/src/pages/deposit/DepositFields.tsx (2)
60-67:useMemoimplicitly returnsundefinedwhen validation passes.The function doesn't explicitly return for the valid case, relying on implicit
undefined. While this works, it's clearer to be explicit. Also, the trailing comment suggests incomplete work.const disabledMessage = useMemo(() => { if (!srcAsset) return "Select asset" if (!quantity) return "Enter amount" if (errors.quantity) return errors.quantity.message if (routeError) return "No route found" - - // Destructure error fields in deps to properly track each field change + return undefined }, [quantity, errors.quantity, srcAsset, routeError])
143-157: DuplicateformatAmountcalls; extract to a variable.
formatAmount(balance, { decimals: srcAsset?.decimals || 6 })is called three times with identical arguments. Extract to a variable for clarity and minor performance improvement.+ const formattedBalance = formatAmount(balance, { decimals: srcAsset?.decimals || 6 }) <button className={styles.maxButton} onClick={() => { - if ( - Number(quantity) === - Number(formatAmount(balance, { decimals: srcAsset?.decimals || 6 })) - ) - return - - setValue("quantity", formatAmount(balance, { decimals: srcAsset?.decimals || 6 })) + if (quantity === formattedBalance) return + setValue("quantity", formattedBalance) }} > - <IconWallet /> {formatAmount(balance, { decimals: srcAsset?.decimals || 6 })}{" "} + <IconWallet /> {formattedBalance}{" "} <span>MAX</span> </button>Note: Move
formattedBalancecalculation outside the JSX, inside the component body wherebalanceis checked.packages/interwovenkit-react/src/pages/deposit/FooterWithTxFee.tsx (1)
24-57: Query key includes entiretxobject; consider using a stable identifier.Including the entire
txobject inqueryKeymay cause unnecessary refetches if the object reference changes even when the content is the same. Iftxhas a unique identifier or hash, prefer using that.const { data: gasEstimate, isLoading } = useQuery({ - queryKey: ["tx-gas-estimate", tx, address, srcChainId], + queryKey: ["tx-gas-estimate", JSON.stringify(tx.cosmos_tx?.msgs), address, srcChainId],Alternatively, if a transaction hash or identifier is available, use that instead for a more stable key.
packages/interwovenkit-react/src/pages/deposit/DepositTxDetails.tsx (1)
57-64: Consider adding explicit dimensions to wallet icon.The wallet icon image lacks explicit width and height attributes, which could cause layout shift during loading.
Apply this diff:
-<img src={walletIcon} alt="Wallet" /> {truncate(address)} +<img src={walletIcon} alt="Wallet" width="16" height="16" /> {truncate(address)}packages/interwovenkit-react/src/pages/deposit/DepositFooter.tsx (1)
52-78: Consider memoizing getFeeDetails for performance.The
getFeeDetailsfunction performs BigNumber calculations and is called multiple times ingetInitialFeeDenom(lines 83, 88), potentially in a loop (line 88). This could impact performance when there are many fee options.Consider memoizing the results:
const feeDetailsCache = useMemo(() => { const cache = new Map<string, ReturnType<typeof getFeeDetails>>() return (feeDenom: string) => { if (cache.has(feeDenom)) return cache.get(feeDenom)! const details = getFeeDetails(feeDenom) cache.set(feeDenom, details) return details } }, [balances, feeCoins, srcAsset, srcDenom, quantity, findAsset])Or compute all fee details once upfront if the number of options is small.
packages/interwovenkit-react/src/pages/deposit/DepositFields.module.css (1)
1-8: Consider adding z-index and responsive behavior to fixed back button.The fixed positioning without a
z-indexcould cause stacking issues with other fixed elements. Additionally, the hardcodedtop: 16px; left: 20pxmay not adapt well to different viewport sizes or modal contexts.Consider adding:
.back { position: fixed; top: 16px; left: 20px; + z-index: 10; color: var(--gray-4); padding: 3px; } + +@media (max-width: 768px) { + .back { + top: 12px; + left: 12px; + } +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
⛔ Files ignored due to path filters (3)
packages/interwovenkit-react/src/pages/deposit/assets/Completed.webmis excluded by!**/*.webmpackages/interwovenkit-react/src/pages/deposit/assets/Failed.svgis excluded by!**/*.svgpackages/interwovenkit-react/src/pages/deposit/assets/Loading.webmis excluded by!**/*.webm
📒 Files selected for processing (31)
examples/vite/src/App.tsx(2 hunks)examples/vite/src/Deposit.tsx(1 hunks)examples/vite/src/Providers.tsx(1 hunks)packages/interwovenkit-react/src/components/form/QuantityInput.tsx(2 hunks)packages/interwovenkit-react/src/data/config.ts(2 hunks)packages/interwovenkit-react/src/data/ui.ts(2 hunks)packages/interwovenkit-react/src/hooks/useConnectedWalletIcon.ts(1 hunks)packages/interwovenkit-react/src/pages/bridge/BridgePreviewFooter.tsx(1 hunks)packages/interwovenkit-react/src/pages/bridge/data/assets.ts(2 hunks)packages/interwovenkit-react/src/pages/bridge/data/skip.ts(1 hunks)packages/interwovenkit-react/src/pages/bridge/data/tx.ts(6 hunks)packages/interwovenkit-react/src/pages/deposit/Deposit.tsx(1 hunks)packages/interwovenkit-react/src/pages/deposit/DepositCompleted.module.css(1 hunks)packages/interwovenkit-react/src/pages/deposit/DepositCompleted.tsx(1 hunks)packages/interwovenkit-react/src/pages/deposit/DepositFields.module.css(1 hunks)packages/interwovenkit-react/src/pages/deposit/DepositFields.tsx(1 hunks)packages/interwovenkit-react/src/pages/deposit/DepositFooter.tsx(1 hunks)packages/interwovenkit-react/src/pages/deposit/DepositTxDetails.tsx(1 hunks)packages/interwovenkit-react/src/pages/deposit/FooterWithTxFee.tsx(1 hunks)packages/interwovenkit-react/src/pages/deposit/SelectDstAsset.module.css(1 hunks)packages/interwovenkit-react/src/pages/deposit/SelectDstAsset.tsx(1 hunks)packages/interwovenkit-react/src/pages/deposit/SelectSrcAsset.module.css(1 hunks)packages/interwovenkit-react/src/pages/deposit/SelectSrcAsset.tsx(1 hunks)packages/interwovenkit-react/src/pages/deposit/hooks.ts(1 hunks)packages/interwovenkit-react/src/public/app/InterwovenKitProvider.tsx(3 hunks)packages/interwovenkit-react/src/public/app/Modal.module.css(1 hunks)packages/interwovenkit-react/src/public/app/Modal.tsx(1 hunks)packages/interwovenkit-react/src/public/app/Routes.tsx(2 hunks)packages/interwovenkit-react/src/public/app/WidgetHeader.tsx(2 hunks)packages/interwovenkit-react/src/public/data/constants.ts(1 hunks)packages/interwovenkit-react/src/public/data/hooks.ts(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (13)
packages/interwovenkit-react/src/pages/deposit/SelectSrcAsset.tsx (1)
packages/interwovenkit-react/src/pages/deposit/hooks.ts (3)
useFilteredDepositAssets(87-107)useDepositForm(117-119)useDstDepositAsset(41-50)
packages/interwovenkit-react/src/public/app/Modal.tsx (1)
packages/interwovenkit-react/src/data/ui.ts (1)
useModal(35-58)
examples/vite/src/App.tsx (1)
packages/interwovenkit-react/src/pages/deposit/Deposit.tsx (1)
Deposit(8-19)
packages/interwovenkit-react/src/pages/bridge/data/assets.ts (3)
packages/interwovenkit-react/src/pages/bridge/data/skip.ts (2)
useSkip(20-23)skipQueryKeys(7-18)packages/interwovenkit-react/src/pages/bridge/data/chains.ts (1)
useSkipChains(38-54)packages/interwovenkit-react/src/data/http.ts (1)
STALE_TIMES(4-8)
packages/interwovenkit-react/src/pages/deposit/FooterWithTxFee.tsx (3)
packages/interwovenkit-react/src/pages/bridge/data/tx.ts (1)
useBridgePreviewState(53-55)packages/interwovenkit-react/src/public/data/hooks.ts (1)
useInitiaAddress(16-20)packages/interwovenkit-react/src/data/signer.ts (2)
useAminoTypes(43-46)useCreateSigningStargateClient(239-265)
packages/interwovenkit-react/src/public/app/WidgetHeader.tsx (3)
packages/interwovenkit-react/src/data/ui.ts (1)
useDisconnect(60-80)packages/interwovenkit-react/src/public/data/hooks.ts (1)
useInterwovenKit(51-99)packages/interwovenkit-react/src/hooks/useConnectedWalletIcon.ts (1)
useConnectedWalletIcon(3-11)
packages/interwovenkit-react/src/public/app/InterwovenKitProvider.tsx (1)
packages/interwovenkit-react/src/pages/bridge/data/assets.ts (1)
useAllSkipAssets(15-43)
examples/vite/src/Deposit.tsx (2)
packages/interwovenkit-react/src/pages/deposit/Deposit.tsx (1)
Deposit(8-19)packages/interwovenkit-react/src/public/data/hooks.ts (1)
useInterwovenKit(51-99)
packages/interwovenkit-react/src/public/data/hooks.ts (1)
packages/interwovenkit-react/src/data/ui.ts (2)
useDrawer(10-33)useModal(35-58)
packages/interwovenkit-react/src/pages/deposit/SelectDstAsset.tsx (3)
packages/interwovenkit-react/src/data/config.ts (1)
useConfig(38-42)packages/interwovenkit-react/src/pages/bridge/data/assets.ts (1)
useAllSkipAssets(15-43)packages/interwovenkit-react/src/pages/deposit/hooks.ts (2)
useDepositForm(117-119)useAllBalancesQuery(10-39)
packages/interwovenkit-react/src/data/ui.ts (1)
packages/interwovenkit-react/src/data/analytics.ts (1)
useAnalyticsTrack(39-53)
packages/interwovenkit-react/src/pages/bridge/data/tx.ts (1)
packages/interwovenkit-react/src/public/data/hooks.ts (1)
useInterwovenKit(51-99)
packages/interwovenkit-react/src/pages/bridge/BridgePreviewFooter.tsx (1)
packages/interwovenkit-react/src/pages/bridge/data/tx.ts (1)
useBridgeTx(62-272)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cloudflare Pages: interwovenkit-testnet
🔇 Additional comments (21)
packages/interwovenkit-react/src/data/config.ts (1)
7-10: DepositOption and Config.depositOptions typing look consistentThe new
DepositOptionshape and optionaldepositOptionsfield onConfigare straightforward, fit the existing config model, and align with the MAINNET configuration usage.Also applies to: 27-34
packages/interwovenkit-react/src/pages/bridge/data/skip.ts (1)
7-18: New skip query keys integrate cleanly with existing key factory usage
allAssetsandallBalancesfollow the existing key patterns and should work fine with React Query’s key hashing and thecreateQueryKeysnamespace.packages/interwovenkit-react/src/components/form/QuantityInput.tsx (1)
16-20: className extension on QuantityInput is safe and flexibleThreading
classNamethrough and combining it withstyles.inputviaclsxkeeps existing styling while allowing per‑use overrides. Validation and form wiring remain unchanged.Also applies to: 22-48
packages/interwovenkit-react/src/public/data/constants.ts (1)
6-20: MAINNET.depositOptions matches the new Config surfaceThe
depositOptionsentries for MAINNET conform toDepositOptionand keep TESTNET untouched, so this is a backwards-compatible configuration extension.packages/interwovenkit-react/src/data/ui.ts (1)
2-3: Drawer and modal atoms are modeled cleanly with mutual exclusivity
isModalOpenAtomplususeModalmirror the existing drawer behavior, and explicitly closing the other surface before opening ensures only one of drawer or modal is active at a time. The analytics hooks are reused consistently.Also applies to: 7-9, 10-33, 35-58
packages/interwovenkit-react/src/pages/bridge/data/tx.ts (1)
57-60: useBridgeTx options (customFee + navigateTo) are wired in a backwards-compatible wayThe
UseBridgeTxOptionsshape anduseBridgeTx(tx, options)signature preserve existing behavior whenoptionsis omitted, while allowing:
- A
customFeepath for Initia viasubmitTxSync.- Existing
requestTxSyncusage when nocustomFeeis supplied.
This keeps the non-Initia and default flows unchanged.Also applies to: 62-63, 73-76, 102-119
packages/interwovenkit-react/src/pages/bridge/BridgePreviewFooter.tsx (1)
1-22: BridgePreviewFooter correctly forwards optional fee and navigation optionsExtending props with
feeandnavigateToand passing them through as{ customFee: fee, navigateTo }touseBridgeTxpreserves existing behavior while enabling the new custom-fee and post-tx navigation flows.packages/interwovenkit-react/src/public/data/hooks.ts (1)
10-10: Modal integration andisOpensemantics look consistentUsing both
useDraweranduseModalhere, exposingopenDepositviaopenModal("/deposit"), and redefiningisOpenasisDrawerOpen || isModalOpenaligns with the drawer/modal coordination already present inuseDisconnect. This should keep the public API intuitive (single “open” flag for either surface) while enabling the new deposit modal.Just be aware this slightly broadens the meaning of
isOpenfor existing consumers; if any caller depended on it representing the drawer only, they’ll now see it true when the deposit modal is open as well.Also applies to: 60-61, 75-77, 90-95
packages/interwovenkit-react/src/public/app/Routes.tsx (1)
11-12: Deposit routes are wired consistently with existing routingImporting
Deposit/DepositCompletedand handling/depositand/deposit/completedin theswitchfits the existing pattern (e.g., bridge/history/op routes). The priorif (!address)guard still ensures we never render these pages when disconnected, and the address-changeuseEffectonly normalizes bridge/NFT paths, so deposit paths aren’t unintentionally redirected.No issues from a routing/control-flow perspective.
Also applies to: 95-98
examples/vite/src/App.tsx (1)
5-5: Example app integration ofDepositis straightforward and safeAdding
<Deposit />next to<Send />and<Bridge />is fine here: theDepositexample component already returnsnullwhen there’s no address, so this doesn’t alter behavior for disconnected users and keeps the example simple.Looks good as an integration point for the new modal flow.
Also applies to: 31-34
packages/interwovenkit-react/src/pages/deposit/DepositCompleted.module.css (1)
1-46: Deposit completion styles align with existing design patternsThe container, typography, spacing, and error styles are consistent with the rest of the kit’s CSS modules (use of
var(--gray-*)andvar(--error), small font sizes, centered layout). Nothing problematic from a layout or theming perspective.No changes needed here.
packages/interwovenkit-react/src/hooks/useConnectedWalletIcon.ts (1)
1-11: LGTM!Clean implementation. The early return for missing connector and the Privy-specific icon handling are appropriate.
packages/interwovenkit-react/src/public/app/InterwovenKitProvider.tsx (1)
82-87: Verify that rendering<Routes />in both Modal and Drawer works as intended.Both
ModalandDrawernow wrap<Routes />. Ensure that the routing state (viaMemoryRouter) and component mounts behave correctly when only one container is visible at a time, and that there are no duplicate side effects from both route trees being mounted simultaneously.packages/interwovenkit-react/src/pages/deposit/SelectDstAsset.module.css (1)
1-33: LGTM!Clean and well-structured CSS module with appropriate use of flexbox and consistent spacing.
packages/interwovenkit-react/src/pages/deposit/DepositTxDetails.tsx (1)
1-11: LGTM!Imports are well-organized and appropriate for the component's functionality.
packages/interwovenkit-react/src/pages/deposit/DepositFooter.tsx (4)
40-48: LGTM! Fee calculation logic is sound.The fee calculation correctly:
- Applies gas adjustment with
Math.ceilto avoid underestimation- Only calculates fees for Cosmos transactions
- Maps over gas prices to generate multiple fee options
80-94: LGTM! Fee denomination selection logic is well-structured.The function appropriately prioritizes:
- Last used denomination (if sufficient)
- First sufficient denomination
- First available denomination (fallback)
Since this is called in a
useStateinitializer (line 96), it only executes once during initial render.
114-146: LGTM! Fee rendering logic handles both single and multiple options elegantly.The function appropriately:
- Returns null when no options exist
- Displays a simple label for single options
- Renders a dropdown for multiple options with proper formatting
1-17: The imports and API usage are compatible with @cosmjs/stargate v0.36.2. BothcalculateFeeandGasPrice.fromStringare correctly exported and used according to the library's documented API—specifically on line 45 whereGasPrice.fromString(amount + denom)produces the expected format (e.g., "0.0025uatom").packages/interwovenkit-react/src/pages/deposit/DepositFields.module.css (2)
54-64: LGTM! Chain icon positioning creates proper badge effect.The absolute positioning with
right: -4pxintentionally extends the chain icon beyond the parent asset icon boundary to create an overlapping badge effect, which is a common UI pattern. The parent.assetIconhasposition: relative(line 45) to establish the positioning context.
10-165: LGTM! Well-structured CSS with good use of custom properties.The styles are well-organized with:
- Semantic class names
- Proper use of CSS custom properties for theming
- Appropriate layout techniques (flexbox, grid)
- Consistent spacing and sizing
| select: ({ chain_to_assets_map }) => { | ||
| const allAssets: RouterAsset[] = [] | ||
|
|
||
| for (const chainId in chain_to_assets_map) { | ||
| if (!chains.find((chain) => chain.chain_id === chainId)) continue | ||
| const { assets } = chain_to_assets_map[chainId] ?? { assets: [] } | ||
| queryClient.setQueryData(skipQueryKeys.assets(chainId).queryKey, { chain_to_assets_map }) | ||
| allAssets.push(...assets) | ||
|
|
||
| for (const asset of assets) { | ||
| queryClient.setQueryData(skipQueryKeys.asset(chainId, asset.denom).queryKey, asset) | ||
| } | ||
| } | ||
| return allAssets |
There was a problem hiding this comment.
Side effects in select function and incorrect cache data.
Two issues:
-
Side effects in
select: Theselectfunction should be pure. CallingqueryClient.setQueryData()during selection can cause unpredictable behavior with React Query's rendering cycle. Move cache population toonSuccessor auseEffect. -
Line 31 caches incorrect data: You're caching the entire
chain_to_assets_mapfor each chain's query key, butuseSkipAssetsexpects only that chain's subset. This could cause inconsistencies whenuseSkipAssetsreads from the cache.
export function useAllSkipAssets() {
const skip = useSkip()
const chains = useSkipChains()
const queryClient = useQueryClient()
+ const chainIdSet = new Set(chains.map((c) => c.chain_id))
+
const { data } = useSuspenseQuery({
queryKey: skipQueryKeys.allAssets().queryKey,
queryFn: () =>
skip
.get("v2/fungible/assets")
.json<{ chain_to_assets_map: Partial<Record<string, { assets: RouterAsset[] }>> }>(),
select: ({ chain_to_assets_map }) => {
const allAssets: RouterAsset[] = []
for (const chainId in chain_to_assets_map) {
- if (!chains.find((chain) => chain.chain_id === chainId)) continue
+ if (!chainIdSet.has(chainId)) continue
const { assets } = chain_to_assets_map[chainId] ?? { assets: [] }
- queryClient.setQueryData(skipQueryKeys.assets(chainId).queryKey, { chain_to_assets_map })
allAssets.push(...assets)
-
- for (const asset of assets) {
- queryClient.setQueryData(skipQueryKeys.asset(chainId, asset.denom).queryKey, asset)
- }
}
return allAssets
},
staleTime: STALE_TIMES.MINUTE,
})
+
+ // Populate cache outside of select
+ useEffect(() => {
+ if (!data) return
+ // Re-fetch response or derive from data to populate individual caches
+ }, [data, queryClient])
+
return data
}Note: This diff shows the direction - you'll need to restructure to access chain_to_assets_map outside select for proper cache population.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/interwovenkit-react/src/pages/bridge/data/assets.ts around lines
25–38, the select function currently performs side effects (calling
queryClient.setQueryData) and caches the entire chain_to_assets_map for each
chain key—both wrong; remove all queryClient.setQueryData calls from the select
to keep it pure, and instead access the fetched chain_to_assets_map in the
query's onSuccess (or a useEffect) where you should iterate over
chain_to_assets_map, for each chainId set the per-chain cache to only that
chain's assets (not the whole map) and set per-asset cache entries for each
asset; ensure select only returns the aggregated allAssets array and that cache
writes use the correct subset shape expected by useSkipAssets.
| export interface FormValues { | ||
| quantity: string | ||
| } | ||
|
|
||
| export const Deposit = () => { | ||
| const form = useForm<FormValues>({ | ||
| mode: "onChange", | ||
| defaultValues: { quantity: "" }, |
There was a problem hiding this comment.
FormValues interface is incomplete; missing fields used elsewhere.
The FormValues interface only declares quantity, but DepositFields.tsx and SelectSrcAsset.tsx call setValue for srcDenom, srcChainId, dstDenom, and dstChainId. This type mismatch will cause TypeScript errors or lose type safety.
export interface FormValues {
+ srcDenom: string
+ srcChainId: string
+ dstDenom: string
+ dstChainId: string
quantity: string
}
export const Deposit = () => {
const form = useForm<FormValues>({
mode: "onChange",
- defaultValues: { quantity: "" },
+ defaultValues: {
+ srcDenom: "",
+ srcChainId: "",
+ dstDenom: "",
+ dstChainId: "",
+ quantity: "",
+ },
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export interface FormValues { | |
| quantity: string | |
| } | |
| export const Deposit = () => { | |
| const form = useForm<FormValues>({ | |
| mode: "onChange", | |
| defaultValues: { quantity: "" }, | |
| export interface FormValues { | |
| srcDenom: string | |
| srcChainId: string | |
| dstDenom: string | |
| dstChainId: string | |
| quantity: string | |
| } | |
| export const Deposit = () => { | |
| const form = useForm<FormValues>({ | |
| mode: "onChange", | |
| defaultValues: { | |
| srcDenom: "", | |
| srcChainId: "", | |
| dstDenom: "", | |
| dstChainId: "", | |
| quantity: "", | |
| }, |
🤖 Prompt for AI Agents
In packages/interwovenkit-react/src/pages/deposit/Deposit.tsx around lines 4–11,
the FormValues interface only declares quantity but other components call
setValue for srcDenom, srcChainId, dstDenom, and dstChainId; add those
properties to FormValues (e.g., srcDenom: string; srcChainId: string; dstDenom:
string; dstChainId: string) and update the useForm defaultValues to include
initial values for these keys so TypeScript types align with setValue calls and
remain type-safe.
| export function DepositCompleted() { | ||
| const state = useLocationState<SuccessState | ErrorState>() | ||
| const { route, values } = useBridgePreviewState() | ||
| const { closeModal } = useModal() | ||
|
|
||
| const isError = "error" in state | ||
| const txHash = !isError ? state.txHash : "" | ||
| const chainId = !isError ? state.chainId : "" | ||
| const timestamp = ("timestamp" in state ? state.timestamp : undefined) ?? 0 | ||
|
|
||
| // Get source asset information for display | ||
| const { srcDenom, srcChainId, quantity } = values | ||
| const srcAsset = useSkipAsset(srcDenom, srcChainId) |
There was a problem hiding this comment.
Missing null checks for state, route, and srcAsset can cause runtime crashes.
If a user navigates directly to this page or if data is unavailable:
statecould be undefined (line 43)routefromuseBridgePreviewState()could be undefined (accessed on line 93)srcAssetfromuseSkipAsset()could be null/undefined (accessed on line 104)
Consider adding early returns or fallbacks:
export function DepositCompleted() {
const state = useLocationState<SuccessState | ErrorState>()
const { route, values } = useBridgePreviewState()
const { closeModal } = useModal()
+ // Guard against missing state (e.g., direct navigation)
+ if (!state || !route || !values) {
+ return (
+ <div className={styles.container}>
+ <h3 className={styles.title}>Invalid state</h3>
+ <Button.White fullWidth onClick={closeModal}>
+ Close
+ </Button.White>
+ </div>
+ )
+ }
+
const isError = "error" in stateAlso guard srcAsset before use on line 104.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function DepositCompleted() { | |
| const state = useLocationState<SuccessState | ErrorState>() | |
| const { route, values } = useBridgePreviewState() | |
| const { closeModal } = useModal() | |
| const isError = "error" in state | |
| const txHash = !isError ? state.txHash : "" | |
| const chainId = !isError ? state.chainId : "" | |
| const timestamp = ("timestamp" in state ? state.timestamp : undefined) ?? 0 | |
| // Get source asset information for display | |
| const { srcDenom, srcChainId, quantity } = values | |
| const srcAsset = useSkipAsset(srcDenom, srcChainId) | |
| export function DepositCompleted() { | |
| const state = useLocationState<SuccessState | ErrorState>() | |
| const { route, values } = useBridgePreviewState() | |
| const { closeModal } = useModal() | |
| // Guard against missing state (e.g., direct navigation) | |
| if (!state || !route || !values) { | |
| return ( | |
| <div className={styles.container}> | |
| <h3 className={styles.title}>Invalid state</h3> | |
| <Button.White fullWidth onClick={closeModal}> | |
| Close | |
| </Button.White> | |
| </div> | |
| ) | |
| } | |
| const isError = "error" in state | |
| const txHash = !isError ? state.txHash : "" | |
| const chainId = !isError ? state.chainId : "" | |
| const timestamp = ("timestamp" in state ? state.timestamp : undefined) ?? 0 | |
| // Get source asset information for display | |
| const { srcDenom, srcChainId, quantity } = values | |
| const srcAsset = useSkipAsset(srcDenom, srcChainId) |
🤖 Prompt for AI Agents
In packages/interwovenkit-react/src/pages/deposit/DepositCompleted.tsx around
lines 42 to 54, the code assumes useLocationState(), useBridgePreviewState() and
useSkipAsset() always return values; add null/undefined guards and fallbacks to
avoid runtime crashes: check if state is falsy before reading "error", txHash,
chainId, and timestamp and return early or render a safe fallback; validate
route from useBridgePreviewState() before accessing any of its properties (line
~93) and handle missing route with an early return or placeholder; guard
srcAsset from useSkipAsset() before using its fields (line ~104) and provide a
fallback asset display or early return. Ensure the checks are minimal and
consistent (e.g. if (!state) return null) and update dependent variables to use
safe defaults when appropriate.
| const minimumReceived = | ||
| (BigInt(route.estimated_amount_out) * BigInt(10000 - Number(values.slippagePercent) * 100)) / | ||
| BigInt(10000) |
There was a problem hiding this comment.
Add validation for slippagePercent to prevent NaN and overflow.
The calculation assumes slippagePercent is a valid number, but Number(values.slippagePercent) could return NaN if the value is invalid. Additionally, multiplying by 100 before subtraction could theoretically overflow for extreme values.
Consider adding validation:
+const slippage = Number(values.slippagePercent)
+if (isNaN(slippage) || slippage < 0 || slippage > 100) {
+ // Handle invalid slippage
+ return null
+}
+
const minimumReceived =
- (BigInt(route.estimated_amount_out) * BigInt(10000 - Number(values.slippagePercent) * 100)) /
+ (BigInt(route.estimated_amount_out) * BigInt(10000 - slippage * 100)) /
BigInt(10000)Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/interwovenkit-react/src/pages/deposit/DepositTxDetails.tsx around
lines 32–34, the slippagePercent is converted with Number(...) without
validation which can produce NaN or allow extreme values that could cause
incorrect math/overflow when turned into BigInt; change the code to parse and
validate slippagePercent first (use parseFloat or Number and check isFinite and
not NaN), clamp it to a safe range (e.g., 0–100), convert to basis points or a
scaled integer safely (e.g., percent * 100) after validation, provide a sensible
default (like 0.5) when invalid, and only then perform the BigInt arithmetic so
the calculation never receives NaN or out-of-range values.
| <div className={styles.detail}> | ||
| <p className={styles.detailLabel}>Minimum received</p> | ||
| <p className={styles.detailValue}> | ||
| <img src={dstAsset.logo_uri} alt={dstAsset.symbol} />{" "} | ||
| {formatAmount(minimumReceived, { decimals: dstAsset.decimals })} {dstAsset.symbol} | ||
| </p> | ||
| </div> |
There was a problem hiding this comment.
Add explicit dimensions to asset logo.
The asset logo at line 70 lacks explicit width and height attributes. Consider adding dimensions for consistency and to prevent layout shift:
-<img src={dstAsset.logo_uri} alt={dstAsset.symbol} />{" "}
+<img src={dstAsset.logo_uri} alt={dstAsset.symbol} width="16" height="16" />{" "}Note: The formatAmount function appears to handle the numeric value correctly based on usage patterns throughout the codebase.
📝 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.
| <div className={styles.detail}> | |
| <p className={styles.detailLabel}>Minimum received</p> | |
| <p className={styles.detailValue}> | |
| <img src={dstAsset.logo_uri} alt={dstAsset.symbol} />{" "} | |
| {formatAmount(minimumReceived, { decimals: dstAsset.decimals })} {dstAsset.symbol} | |
| </p> | |
| </div> | |
| <div className={styles.detail}> | |
| <p className={styles.detailLabel}>Minimum received</p> | |
| <p className={styles.detailValue}> | |
| <img src={dstAsset.logo_uri} alt={dstAsset.symbol} width="16" height="16" />{" "} | |
| {formatAmount(minimumReceived, { decimals: dstAsset.decimals })} {dstAsset.symbol} | |
| </p> | |
| </div> |
🤖 Prompt for AI Agents
In packages/interwovenkit-react/src/pages/deposit/DepositTxDetails.tsx around
lines 67 to 73, the asset logo image element lacks explicit width and height
attributes causing potential layout shift; update the <img> tag to include fixed
width and height (use appropriate pixel values consistent with other asset icons
in the app or a constant from styles), and ensure the alt remains
dstAsset.symbol; if CSS controls sizing, set matching width/height attributes to
match those styles to prevent CLS.
| const messages = tx.cosmos_tx.msgs.map(({ msg_type_url, msg }) => { | ||
| if (!(msg_type_url && msg)) throw new Error("Invalid transaction data") | ||
| // Note: `typeUrl` comes in proto format, but `msg` is in amino format. | ||
| return aminoTypes.fromAmino({ | ||
| type: aminoConverters[msg_type_url].aminoType, | ||
| value: JSON.parse(msg), | ||
| }) | ||
| }) |
There was a problem hiding this comment.
Missing guard for aminoConverters[msg_type_url] lookup.
If msg_type_url is not present in aminoConverters, the lookup returns undefined and accessing .aminoType throws. While inside try/catch, this silently fails without a clear error message.
const messages = tx.cosmos_tx.msgs.map(({ msg_type_url, msg }) => {
if (!(msg_type_url && msg)) throw new Error("Invalid transaction data")
+ const converter = aminoConverters[msg_type_url]
+ if (!converter) throw new Error(`Unsupported message type: ${msg_type_url}`)
// Note: `typeUrl` comes in proto format, but `msg` is in amino format.
return aminoTypes.fromAmino({
- type: aminoConverters[msg_type_url].aminoType,
+ type: converter.aminoType,
value: JSON.parse(msg),
})
})🤖 Prompt for AI Agents
In packages/interwovenkit-react/src/pages/deposit/FooterWithTxFee.tsx around
lines 33 to 40, the code assumes aminoConverters[msg_type_url] exists and
accesses .aminoType which can be undefined and will cause a confusing failure;
add a guard after validating msg_type_url and msg to check that
aminoConverters[msg_type_url] is defined, and if not throw a clear, descriptive
error (including the msg_type_url) or handle the missing converter appropriately
(e.g., skip the message or provide a fallback) before calling
aminoTypes.fromAmino so failures are explicit and informative.
packages/interwovenkit-react/src/pages/deposit/SelectSrcAsset.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
packages/interwovenkit-react/src/pages/deposit/SelectSrcAsset.tsx (1)
57-60: Address the past review comment about NaN display.The issue flagged in the previous review remains unresolved. When
balance.value_usdisundefinedornull,Number(balance.value_usd)returnsNaN, displaying$NaNto users.Apply this diff to handle missing USD values:
<p className={styles.balance}> {formatAmount(balance?.amount, { decimals: balance.decimals || 6 })} </p> - <p className={styles.value}>${Number(balance.value_usd).toFixed(2)}</p> + <p className={styles.value}> + {balance.value_usd != null ? `$${Number(balance.value_usd).toFixed(2)}` : "-"} + </p>
🧹 Nitpick comments (1)
packages/interwovenkit-react/src/pages/deposit/SelectDstAsset.module.css (1)
12-14: Consider reordering margin properties.The
margin-bottomappears beforemargin-top, which is unconventional. For better readability, consider placingmargin-topfirst.-.list { - display: flex; - flex-direction: column; - gap: 12px; - - margin-bottom: 8px; - - margin-top: 20px; -} +.list { + display: flex; + flex-direction: column; + gap: 12px; + margin-top: 20px; + margin-bottom: 8px; +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
⛔ Files ignored due to path filters (1)
packages/interwovenkit-react/src/pages/deposit/assets/Empty.svgis excluded by!**/*.svg
📒 Files selected for processing (4)
packages/interwovenkit-react/src/pages/deposit/SelectDstAsset.module.css(1 hunks)packages/interwovenkit-react/src/pages/deposit/SelectSrcAsset.module.css(1 hunks)packages/interwovenkit-react/src/pages/deposit/SelectSrcAsset.tsx(1 hunks)packages/interwovenkit-react/src/public/app/Modal.module.css(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/interwovenkit-react/src/public/app/Modal.module.css
- packages/interwovenkit-react/src/pages/deposit/SelectSrcAsset.module.css
🧰 Additional context used
🧬 Code graph analysis (1)
packages/interwovenkit-react/src/pages/deposit/SelectSrcAsset.tsx (1)
packages/interwovenkit-react/src/pages/deposit/hooks.ts (3)
useFilteredDepositAssets(87-107)useDepositForm(117-119)useDstDepositAsset(41-50)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cloudflare Pages: interwovenkit-testnet
🔇 Additional comments (7)
packages/interwovenkit-react/src/pages/deposit/SelectDstAsset.module.css (1)
36-42: LGTM!The circular image styling with fixed dimensions and border is appropriate for asset icons.
packages/interwovenkit-react/src/pages/deposit/SelectSrcAsset.tsx (6)
1-5: LGTM!Imports are clean and properly structured.
7-12: LGTM!The component correctly retrieves filtered assets and destination asset information. The early return ensures the component only renders when a destination asset is selected, which aligns with the deposit flow requirements.
14-17: LGTM!The
navigateBackfunction correctly clears the destination asset selection to return to the previous step in the deposit flow.
19-31: LGTM!The empty state is properly gated by loading state and provides clear feedback to users when no supported assets are available for deposit.
45-49: LGTM!The onClick handler correctly updates the form with the selected source asset details and clears the quantity field for new input.
68-68: LGTM!Standard default export.
| .asset { | ||
| display: flex; | ||
| justify-content: flex-start; | ||
| align-items: center; | ||
| gap: 8px; | ||
|
|
||
| width: 100%; | ||
|
|
||
| border: 1px solid var(--gray-5); | ||
| border-radius: 12px; | ||
| font-size: 14px; | ||
| font-weight: 600; | ||
| padding: 11px 12px; | ||
| } |
There was a problem hiding this comment.
Add cursor pointer for interactive element.
The .asset class has a hover state, indicating it's interactive, but lacks cursor: pointer. Users expect visual feedback when hovering over clickable elements.
.asset {
display: flex;
justify-content: flex-start;
align-items: center;
gap: 8px;
width: 100%;
border: 1px solid var(--gray-5);
border-radius: 12px;
font-size: 14px;
font-weight: 600;
padding: 11px 12px;
+ cursor: pointer;
}📝 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.
| .asset { | |
| display: flex; | |
| justify-content: flex-start; | |
| align-items: center; | |
| gap: 8px; | |
| width: 100%; | |
| border: 1px solid var(--gray-5); | |
| border-radius: 12px; | |
| font-size: 14px; | |
| font-weight: 600; | |
| padding: 11px 12px; | |
| } | |
| .asset { | |
| display: flex; | |
| justify-content: flex-start; | |
| align-items: center; | |
| gap: 8px; | |
| width: 100%; | |
| border: 1px solid var(--gray-5); | |
| border-radius: 12px; | |
| font-size: 14px; | |
| font-weight: 600; | |
| padding: 11px 12px; | |
| cursor: pointer; | |
| } |
🤖 Prompt for AI Agents
In packages/interwovenkit-react/src/pages/deposit/SelectDstAsset.module.css
around lines 17 to 30, the .asset class is styled as an interactive element but
lacks a cursor change; add cursor: pointer to the .asset rule so the pointer
cursor appears on hover, keeping existing layout and visual styles unchanged.
| .asset:hover { | ||
| background-color: var(--gray-8); | ||
| } |
There was a problem hiding this comment.
Add focus styles for keyboard accessibility.
The .asset class is interactive but lacks focus styles, preventing keyboard users from navigating the asset selection. This blocks task completion for users who rely on keyboard navigation.
.asset:hover {
background-color: var(--gray-8);
}
+
+.asset:focus-visible {
+ background-color: var(--gray-8);
+ outline: 2px solid var(--primary);
+ outline-offset: 2px;
+}📝 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.
| .asset:hover { | |
| background-color: var(--gray-8); | |
| } | |
| .asset:hover { | |
| background-color: var(--gray-8); | |
| } | |
| .asset:focus-visible { | |
| background-color: var(--gray-8); | |
| outline: 2px solid var(--primary); | |
| outline-offset: 2px; | |
| } |
🤖 Prompt for AI Agents
In packages/interwovenkit-react/src/pages/deposit/SelectDstAsset.module.css
around lines 32 to 34, the interactive .asset rule only styles :hover and lacks
:focus/:focus-visible styles for keyboard users; add focus and focus-visible
selectors matching or complementing the hover state (e.g., outline or
background-color change and visible focus ring using accessible contrast and
offset) and ensure :focus-visible is preferred to avoid double indicators,
keeping styles consistent with :hover and maintaining accessibility (consider
adding :active as well).
| <img src={asset.logo_uri} alt={asset.symbol} className={styles.assetIcon} /> | ||
| <img src={chain.logo_uri || ""} alt={chain.chain_name} className={styles.chainIcon} /> |
There was a problem hiding this comment.
Fallback image for missing chain logo.
If chain.logo_uri is falsy, the empty string fallback will render a broken image element. Consider using a fallback icon or hiding the image when the URI is unavailable.
Apply this diff to handle missing logos gracefully:
- <img src={chain.logo_uri || ""} alt={chain.chain_name} className={styles.chainIcon} />
+ {chain.logo_uri && (
+ <img src={chain.logo_uri} alt={chain.chain_name} className={styles.chainIcon} />
+ )}🤖 Prompt for AI Agents
In packages/interwovenkit-react/src/pages/deposit/SelectSrcAsset.tsx around
lines 52-53, the chain image currently uses an empty-string fallback which
renders a broken <img>; change this to handle missing logos by either
conditionally rendering the <img> only when chain.logo_uri is truthy or by
supplying a real fallback (e.g., a local/default icon path) and attach an
onError handler to swap to that fallback if loading fails; ensure the alt text
remains meaningful and the JSX uses the chosen conditional/fallback approach so
broken images are not shown.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/interwovenkit-react/src/pages/deposit/DepositFields.module.css (2)
109-120: Consider adding hover states and transitions for better UX.Interactive buttons like
.maxButtonand.detailsButtonwould benefit from hover states and smooth transitions to provide visual feedback and improve the user experience.Add hover and transition properties:
.maxButton { display: flex; gap: 4px; color: var(--gray-3); font-size: 12px; font-weight: 500; transition: color 0.2s ease; cursor: pointer; } .maxButton:hover { color: var(--gray-1); } .detailsButton { justify-content: flex-start; gap: 2px; color: var(--gray-2); font-size: 12px; font-weight: 500; transition: color 0.2s ease; cursor: pointer; } .detailsButton:hover { color: var(--gray-1); }Also applies to: 139-146
18-20: Consider using margin instead of a spacer element.The
.dividerclass serves solely as vertical spacing. This pattern requires an extra DOM element. Consider applyingmargin-topormargin-bottomto adjacent elements or usinggapin flex/grid containers for cleaner markup.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (1)
packages/interwovenkit-react/src/pages/deposit/DepositFields.module.css(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cloudflare Pages: interwovenkit-testnet
| .asset { | ||
| display: grid; | ||
| grid-template-columns: auto 1fr auto; | ||
| align-items: center; | ||
|
|
||
| width: 100%; | ||
| height: 56px; | ||
|
|
||
| background-color: var(--gray-7); | ||
| border-radius: 8px; | ||
| column-gap: 8px; | ||
| margin-bottom: 20px; | ||
| padding: 6px 12px; | ||
| } |
There was a problem hiding this comment.
Add focus styles for keyboard accessibility.
Interactive elements like .asset (asset selector) and .input lack explicit focus indicators. While browsers provide default focus styles, defining consistent, visible focus states is essential for keyboard navigation and accessibility compliance.
Add focus styles to ensure keyboard users can see which element is focused:
.asset:focus-visible {
outline: 2px solid var(--gray-3);
outline-offset: 2px;
}
.input:focus {
outline: 2px solid var(--gray-3);
outline-offset: -2px;
}Also applies to: 82-92
🤖 Prompt for AI Agents
In packages/interwovenkit-react/src/pages/deposit/DepositFields.module.css
around lines 29-42 (and also apply same change to lines 82-92), the .asset and
.input elements lack explicit keyboard focus styles; add CSS focus rules so
keyboard users see the focused element — add a :focus-visible rule for .asset
(e.g., 2px solid var(--gray-3) with a small outline-offset) and a focus (or
focus-visible) rule for .input with the same visible outline and appropriate
outline-offset to avoid clipping; ensure to use :focus-visible where possible
for better mouse/keyboard differentiation and include both rules in the file at
the indicated lines.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/interwovenkit-react/src/pages/deposit/DepositTxDetails.tsx (1)
32-35: Validate slippagePercent before calculation to prevent NaN and runtime errors.The calculation converts
values.slippagePercentusingNumber()without validation. If the value is invalid,Number()returnsNaN, andBigInt(NaN)throws aRangeError. This was flagged in a previous review but remains unresolved.Apply validation before the calculation:
+ const slippage = Number(values.slippagePercent) + if (isNaN(slippage) || slippage < 0 || slippage > 100) { + // Handle invalid slippage with a safe default + const safeSlippage = 0.5 + const minimumReceived = route.does_swap + ? (BigInt(route.estimated_amount_out) * BigInt(10000 - safeSlippage * 100)) / BigInt(10000) + : BigInt(route.estimated_amount_out) + } else { const minimumReceived = route.does_swap - ? (BigInt(route.estimated_amount_out) * BigInt(10000 - Number(values.slippagePercent) * 100)) / + ? (BigInt(route.estimated_amount_out) * BigInt(10000 - slippage * 100)) / BigInt(10000) : BigInt(route.estimated_amount_out) + }
🧹 Nitpick comments (1)
packages/interwovenkit-react/src/pages/deposit/DepositTxDetails.tsx (1)
70-76: Consider adding explicit dimensions to the asset logo.Line 73 includes
className={styles.detailToken}(improvement since the past review), but still lacks explicitwidthandheightattributes. Adding them helps prevent layout shift and improves rendering consistency.- <img src={dstAsset.logo_uri} alt={dstAsset.symbol} className={styles.detailToken} /> + <img src={dstAsset.logo_uri} alt={dstAsset.symbol} className={styles.detailToken} width="16" height="16" />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (2)
packages/interwovenkit-react/src/pages/deposit/DepositFields.module.css(1 hunks)packages/interwovenkit-react/src/pages/deposit/DepositTxDetails.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/interwovenkit-react/src/pages/deposit/DepositFields.module.css
🧰 Additional context used
🧬 Code graph analysis (1)
packages/interwovenkit-react/src/pages/deposit/DepositTxDetails.tsx (5)
packages/interwovenkit-react/src/pages/bridge/data/simulate.ts (1)
RouterRouteResponseJson(14-19)packages/interwovenkit-react/src/pages/bridge/data/tx.ts (1)
useBridgePreviewState(53-55)packages/interwovenkit-react/src/pages/bridge/data/assets.ts (1)
useAllSkipAssets(15-43)packages/interwovenkit-react/src/public/data/hooks.ts (1)
useInitiaAddress(16-20)packages/interwovenkit-react/src/hooks/useConnectedWalletIcon.ts (1)
useConnectedWalletIcon(3-11)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cloudflare Pages: interwovenkit-testnet
| {address && ( | ||
| <div className={styles.detail}> | ||
| <p className={styles.detailLabel}>Receiving address</p> | ||
| <p className={styles.detailValue}> | ||
| <img src={walletIcon} alt="Wallet" /> {truncate(address)} | ||
| </p> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
Guard against undefined walletIcon to prevent invalid HTML.
Line 64 renders <img src={walletIcon}> without checking if walletIcon is defined. When useConnectedWalletIcon() returns undefined, this produces <img src="undefined">, causing a failed network request and a broken image icon.
Conditionally render the wallet icon:
{address && (
<div className={styles.detail}>
<p className={styles.detailLabel}>Receiving address</p>
<p className={styles.detailValue}>
- <img src={walletIcon} alt="Wallet" /> {truncate(address)}
+ {walletIcon && <img src={walletIcon} alt="Wallet" />} {truncate(address)}
</p>
</div>
)}📝 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.
| {address && ( | |
| <div className={styles.detail}> | |
| <p className={styles.detailLabel}>Receiving address</p> | |
| <p className={styles.detailValue}> | |
| <img src={walletIcon} alt="Wallet" /> {truncate(address)} | |
| </p> | |
| </div> | |
| )} | |
| {address && ( | |
| <div className={styles.detail}> | |
| <p className={styles.detailLabel}>Receiving address</p> | |
| <p className={styles.detailValue}> | |
| {walletIcon && <img src={walletIcon} alt="Wallet" />} {truncate(address)} | |
| </p> | |
| </div> | |
| )} |
🤖 Prompt for AI Agents
In packages/interwovenkit-react/src/pages/deposit/DepositTxDetails.tsx around
lines 60 to 67, the JSX renders <img src={walletIcon}> without guarding against
walletIcon being undefined which leads to src="undefined" and a broken request;
update the rendering so the <img> is only output when walletIcon is truthy
(e.g., conditionally render the <img> or render a placeholder/svg when
walletIcon is falsy), leaving the rest of the address markup unchanged.
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI Agents
In @packages/interwovenkit-react/src/pages/deposit/Fields.module.css:
- Around line 163-174: The .detailsButton rule in Fields.module.css is missing
display: flex so its justify-content, align-items, and gap settings are ignored;
update the .detailsButton CSS block to include display: flex (e.g., add
"display: flex;") so the layout properties take effect and the button renders as
intended.
- Around line 96-106: The .input style lacks keyboard focus indicators; update
the CSS for the .input selector to include explicit focus styles (use
.input:focus and .input:focus-visible) that provide a high-contrast visible
outline or box-shadow (e.g., a 2px solid or accessible colored ring) and ensure
you do not rely on outline: none without replacement; apply the focus styles to
the .input class used in the form fields so keyboard users can see focus state
and the indicator meets contrast requirements.
- Around line 163-178: Add explicit keyboard focus styles for the .detailsButton
selector: define :focus and preferably :focus-visible rules for .detailsButton
to provide a visible indicator (e.g., outline or box-shadow and outline-offset)
and ensure the color/contrast meets accessibility; keep hover color behavior but
ensure focus state does not rely on :hover, and include focus styles for both
keyboard focus and when the element is focused programmatically.
- Around line 6-17: The .back CSS rule lacks keyboard focus styles; add explicit
focus indicators by extending the .back selector with .back:focus and/or
.back:focus-visible (and optionally .back:active) to apply a visible outline or
box-shadow and ensure sufficient color/contrast for accessibility; keep hover
behavior intact and prefer focus-visible to avoid styling mouse clicks while
providing a clear keyboard focus ring for the .back element.
- Around line 38-54: The .asset rule lacks keyboard focus styles; make the
interactive asset element visibly focusable by adding focus rules (e.g.,
.asset:focus and/or .asset:focus-visible) that apply a clear outline or
box-shadow and preserve the existing hover look, and ensure the element that
uses .asset is focusable (has tabindex or is a button/anchor) so keyboard users
see the indicator; update the CSS selectors for .asset:focus and
.asset:focus-visible to use a high-contrast outline/box-shadow and maintain
border-radius and padding for consistent appearance.
- Around line 126-140: The .maxButton lacks keyboard focus indicators; add
explicit focus styles for .maxButton (and optionally .maxButton:focus-visible)
to show a clear visible outline/box-shadow (contrasting color or using
var(--focus-color)), set outline-offset and border-radius to match the element,
and ensure the span styling remains readable on focus; if the element with class
.maxButton is not naturally focusable, make it focusable in markup (e.g., add
tabindex="0") so the new focus styles apply to keyboard users.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (1)
packages/interwovenkit-react/src/pages/deposit/Fields.module.css
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cloudflare Pages: interwovenkit-testnet
| .back { | ||
| position: absolute; | ||
| top: 16px; | ||
| left: 20px; | ||
|
|
||
| color: var(--gray-4); | ||
| padding: 3px; | ||
| } | ||
|
|
||
| .back:hover { | ||
| color: var(--gray-2); | ||
| } |
There was a problem hiding this comment.
Add focus styles for keyboard accessibility.
The .back button still lacks explicit keyboard focus indicators, which are essential for accessibility compliance and keyboard navigation.
🔎 Add focus styles
.back:hover {
color: var(--gray-2);
}
+
+.back:focus-visible {
+ outline: 2px solid var(--gray-3);
+ outline-offset: 2px;
+}🤖 Prompt for AI Agents
In @packages/interwovenkit-react/src/pages/deposit/Fields.module.css around
lines 6 - 17, The .back CSS rule lacks keyboard focus styles; add explicit focus
indicators by extending the .back selector with .back:focus and/or
.back:focus-visible (and optionally .back:active) to apply a visible outline or
box-shadow and ensure sufficient color/contrast for accessibility; keep hover
behavior intact and prefer focus-visible to avoid styling mouse clicks while
providing a clear keyboard focus ring for the .back element.
| .asset { | ||
| display: grid; | ||
| grid-template-columns: auto 1fr auto; | ||
| align-items: center; | ||
|
|
||
| width: 100%; | ||
| height: 56px; | ||
|
|
||
| background-color: var(--gray-7); | ||
| border-radius: 8px; | ||
| column-gap: 8px; | ||
| padding: 6px 12px; | ||
| } | ||
|
|
||
| .asset:hover { | ||
| background-color: var(--gray-6); | ||
| } |
There was a problem hiding this comment.
Add focus styles for keyboard accessibility.
The .asset selector still lacks explicit focus indicators. Since this is an interactive element for asset selection, keyboard users need visible focus states.
🔎 Add focus styles
.asset:hover {
background-color: var(--gray-6);
}
+
+.asset:focus-visible {
+ outline: 2px solid var(--gray-3);
+ outline-offset: 2px;
+}📝 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.
| .asset { | |
| display: grid; | |
| grid-template-columns: auto 1fr auto; | |
| align-items: center; | |
| width: 100%; | |
| height: 56px; | |
| background-color: var(--gray-7); | |
| border-radius: 8px; | |
| column-gap: 8px; | |
| padding: 6px 12px; | |
| } | |
| .asset:hover { | |
| background-color: var(--gray-6); | |
| } | |
| .asset { | |
| display: grid; | |
| grid-template-columns: auto 1fr auto; | |
| align-items: center; | |
| width: 100%; | |
| height: 56px; | |
| background-color: var(--gray-7); | |
| border-radius: 8px; | |
| column-gap: 8px; | |
| padding: 6px 12px; | |
| } | |
| .asset:hover { | |
| background-color: var(--gray-6); | |
| } | |
| .asset:focus-visible { | |
| outline: 2px solid var(--gray-3); | |
| outline-offset: 2px; | |
| } |
🤖 Prompt for AI Agents
In @packages/interwovenkit-react/src/pages/deposit/Fields.module.css around
lines 38 - 54, The .asset rule lacks keyboard focus styles; make the interactive
asset element visibly focusable by adding focus rules (e.g., .asset:focus and/or
.asset:focus-visible) that apply a clear outline or box-shadow and preserve the
existing hover look, and ensure the element that uses .asset is focusable (has
tabindex or is a button/anchor) so keyboard users see the indicator; update the
CSS selectors for .asset:focus and .asset:focus-visible to use a high-contrast
outline/box-shadow and maintain border-radius and padding for consistent
appearance.
| .input { | ||
| width: 100%; | ||
| height: 56px; | ||
|
|
||
| background-color: var(--gray-7); | ||
| border-radius: 8px; | ||
| font-size: 16px; | ||
| font-weight: 500; | ||
| padding: 18px 16px; | ||
| text-align: left; | ||
| } |
There was a problem hiding this comment.
Add focus styles for keyboard accessibility.
The .input field still lacks explicit focus indicators, which are necessary for form accessibility and keyboard navigation.
🔎 Add focus styles
.input {
width: 100%;
height: 56px;
background-color: var(--gray-7);
border-radius: 8px;
font-size: 16px;
font-weight: 500;
padding: 18px 16px;
text-align: left;
}
+
+.input:focus {
+ outline: 2px solid var(--gray-3);
+ outline-offset: -2px;
+}📝 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.
| .input { | |
| width: 100%; | |
| height: 56px; | |
| background-color: var(--gray-7); | |
| border-radius: 8px; | |
| font-size: 16px; | |
| font-weight: 500; | |
| padding: 18px 16px; | |
| text-align: left; | |
| } | |
| .input { | |
| width: 100%; | |
| height: 56px; | |
| background-color: var(--gray-7); | |
| border-radius: 8px; | |
| font-size: 16px; | |
| font-weight: 500; | |
| padding: 18px 16px; | |
| text-align: left; | |
| } | |
| .input:focus { | |
| outline: 2px solid var(--gray-3); | |
| outline-offset: -2px; | |
| } |
🤖 Prompt for AI Agents
In @packages/interwovenkit-react/src/pages/deposit/Fields.module.css around
lines 96 - 106, The .input style lacks keyboard focus indicators; update the CSS
for the .input selector to include explicit focus styles (use .input:focus and
.input:focus-visible) that provide a high-contrast visible outline or box-shadow
(e.g., a 2px solid or accessible colored ring) and ensure you do not rely on
outline: none without replacement; apply the focus styles to the .input class
used in the form fields so keyboard users can see focus state and the indicator
meets contrast requirements.
| .maxButton { | ||
| display: flex; | ||
| gap: 4px; | ||
|
|
||
| color: var(--gray-3); | ||
| font-family: var(--monospace); | ||
| font-size: 12px; | ||
| font-weight: 500; | ||
| } | ||
|
|
||
| .maxButton span { | ||
| color: var(--gray-0); | ||
| font-family: var(--font-family); | ||
| font-weight: 600; | ||
| } |
There was a problem hiding this comment.
Add focus styles for keyboard accessibility.
The .maxButton still lacks explicit focus indicators for keyboard navigation.
🔎 Add focus styles
.maxButton span {
color: var(--gray-0);
font-family: var(--font-family);
font-weight: 600;
}
+
+.maxButton:focus-visible {
+ outline: 2px solid var(--gray-3);
+ outline-offset: 2px;
+}📝 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.
| .maxButton { | |
| display: flex; | |
| gap: 4px; | |
| color: var(--gray-3); | |
| font-family: var(--monospace); | |
| font-size: 12px; | |
| font-weight: 500; | |
| } | |
| .maxButton span { | |
| color: var(--gray-0); | |
| font-family: var(--font-family); | |
| font-weight: 600; | |
| } | |
| .maxButton { | |
| display: flex; | |
| gap: 4px; | |
| color: var(--gray-3); | |
| font-family: var(--monospace); | |
| font-size: 12px; | |
| font-weight: 500; | |
| } | |
| .maxButton span { | |
| color: var(--gray-0); | |
| font-family: var(--font-family); | |
| font-weight: 600; | |
| } | |
| .maxButton:focus-visible { | |
| outline: 2px solid var(--gray-3); | |
| outline-offset: 2px; | |
| } |
🤖 Prompt for AI Agents
In @packages/interwovenkit-react/src/pages/deposit/Fields.module.css around
lines 126 - 140, The .maxButton lacks keyboard focus indicators; add explicit
focus styles for .maxButton (and optionally .maxButton:focus-visible) to show a
clear visible outline/box-shadow (contrasting color or using
var(--focus-color)), set outline-offset and border-radius to match the element,
and ensure the span styling remains readable on focus; if the element with class
.maxButton is not naturally focusable, make it focusable in markup (e.g., add
tabindex="0") so the new focus styles apply to keyboard users.
| .detailsButton { | ||
| justify-content: flex-start; | ||
| align-items: center; | ||
| gap: 2px; | ||
|
|
||
| width: fit-content; | ||
| height: 18px; | ||
|
|
||
| color: var(--gray-3); | ||
| font-size: 12px; | ||
| font-weight: 500; | ||
| } |
There was a problem hiding this comment.
CRITICAL: Add missing display: flex; property.
The .detailsButton class uses justify-content, align-items, and gap properties (lines 164-166), which only work with flexbox or grid layout. Without display: flex;, these properties are ignored by the browser and the button will not layout correctly.
This issue was flagged in a previous review but has not been addressed.
🔎 Proposed fix
.detailsButton {
+ display: flex;
justify-content: flex-start;
align-items: center;
gap: 2px;
width: fit-content;
height: 18px;
color: var(--gray-3);
font-size: 12px;
font-weight: 500;
}📝 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.
| .detailsButton { | |
| justify-content: flex-start; | |
| align-items: center; | |
| gap: 2px; | |
| width: fit-content; | |
| height: 18px; | |
| color: var(--gray-3); | |
| font-size: 12px; | |
| font-weight: 500; | |
| } | |
| .detailsButton { | |
| display: flex; | |
| justify-content: flex-start; | |
| align-items: center; | |
| gap: 2px; | |
| width: fit-content; | |
| height: 18px; | |
| color: var(--gray-3); | |
| font-size: 12px; | |
| font-weight: 500; | |
| } |
🤖 Prompt for AI Agents
In @packages/interwovenkit-react/src/pages/deposit/Fields.module.css around
lines 163 - 174, The .detailsButton rule in Fields.module.css is missing
display: flex so its justify-content, align-items, and gap settings are ignored;
update the .detailsButton CSS block to include display: flex (e.g., add
"display: flex;") so the layout properties take effect and the button renders as
intended.
| .detailsButton { | ||
| justify-content: flex-start; | ||
| align-items: center; | ||
| gap: 2px; | ||
|
|
||
| width: fit-content; | ||
| height: 18px; | ||
|
|
||
| color: var(--gray-3); | ||
| font-size: 12px; | ||
| font-weight: 500; | ||
| } | ||
|
|
||
| .detailsButton:hover { | ||
| color: var(--gray-1); | ||
| } |
There was a problem hiding this comment.
Add focus styles for keyboard accessibility.
The .detailsButton still lacks explicit focus indicators for keyboard navigation.
🔎 Add focus styles
.detailsButton:hover {
color: var(--gray-1);
}
+
+.detailsButton:focus-visible {
+ outline: 2px solid var(--gray-3);
+ outline-offset: 2px;
+}🤖 Prompt for AI Agents
In @packages/interwovenkit-react/src/pages/deposit/Fields.module.css around
lines 163 - 178, Add explicit keyboard focus styles for the .detailsButton
selector: define :focus and preferably :focus-visible rules for .detailsButton
to provide a visible indicator (e.g., outline or box-shadow and outline-offset)
and ensure the color/contrast meets accessibility; keep hover color behavior but
ensure focus state does not rely on :hover, and include focus styles for both
keyboard focus and when the element is focused programmatically.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/interwovenkit-react/src/pages/deposit/Deposit.tsx (1)
44-55: Consider adding a default case for runtime safety.The switch statement lacks a default case. While TypeScript provides compile-time exhaustiveness checking, adding a default case improves runtime safety and makes bugs like the "completed" vs "transfer-completed" mismatch more apparent during development.
🔎 Suggested addition
const renderPage = (currentPage: typeof page) => { switch (currentPage) { case "select-local": return <SelectLocalAsset /> case "select-external": return <SelectExternalAsset /> case "fields": return <DepositFields /> - case "completed": + case "transfer-completed": return <TransferCompleted type="deposit" /> + default: + // Exhaustiveness check - should never reach here + console.error(`Unknown page: ${currentPage}`) + return null } }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (2)
packages/interwovenkit-react/src/pages/deposit/Deposit.tsxpackages/interwovenkit-react/src/pages/deposit/Withdraw.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/interwovenkit-react/src/pages/deposit/Withdraw.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
packages/interwovenkit-react/src/pages/deposit/Deposit.tsx (3)
packages/interwovenkit-react/src/pages/deposit/Withdraw.tsx (1)
FormValues(11-14)packages/interwovenkit-react/src/pages/deposit/hooks.ts (2)
useAllBalancesQuery(17-46)useTransferForm(137-139)packages/interwovenkit-react/src/pages/deposit/TransferCompleted.tsx (1)
TransferCompleted(34-169)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cloudflare Pages: interwovenkit-testnet
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/interwovenkit-react/src/pages/bridge/BridgePreviewFooter.tsx (1)
19-25: Consider separating error display from button label.The current implementation displays error text inside the button label when the button is disabled. While this creates an inline validation pattern, it has some UX considerations:
- Unconventional pattern: Most applications display errors separately (above/below the button) rather than replacing the button text
- Text overflow: Long error messages might not fit well within button constraints
- Accessibility: Screen readers may not clearly communicate that the button is disabled because of the error shown in the label
However, this pattern may be intentional for the modal context. If you prefer the current approach, consider:
- Keeping error messages concise to fit button width
- Testing with screen readers to ensure disabled state + error reason are announced properly
Alternative: Separate error display
return ( <Footer> + {error && <div className={styles.error}>{error}</div>} <Button.White onClick={() => mutate()} loading={isPending && "Signing transaction..."} disabled={!!error} > - {error || confirmMessage || "Confirm"} + {confirmMessage || "Confirm"} </Button.White> </Footer> )This approach keeps button text stable and displays errors in a dedicated element.
packages/interwovenkit-react/src/pages/deposit/TransferFooter.tsx (1)
32-35: Extract duplicated onCompleted handler.The
onCompletedfunction is duplicated in bothTransferFooterWithFee(lines 32-35) andTransferFooter(lines 184-187). Consider extracting this to a shared helper or defining it once at the top level of the file.♻️ Refactor to extract shared logic
+const useTransferOnCompleted = () => { + const { setValue } = useTransferForm() + return (result: BridgeTxResult) => { + setValue("page", "completed") + setValue("result", result) + } +} + const TransferFooterWithFee = ({ tx, gas }: Props) => { const path = usePath() const isWithdraw = path.startsWith("/withdraw") const confirmMessage = isWithdraw ? "Withdraw" : "Deposit" - const { setValue } = useTransferForm() - - function onCompleted(result: BridgeTxResult) { - setValue("page", "completed") - setValue("result", result) - } + const onCompleted = useTransferOnCompleted() // ... rest of the component } const TransferFooter = ({ tx, gas }: Props) => { const path = usePath() - const { setValue } = useTransferForm() - - function onCompleted(result: BridgeTxResult) { - setValue("page", "completed") - setValue("result", result) - } + const onCompleted = useTransferOnCompleted() // ... rest of the component }Also applies to: 184-187
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (2)
packages/interwovenkit-react/src/pages/bridge/BridgePreviewFooter.tsxpackages/interwovenkit-react/src/pages/deposit/TransferFooter.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
packages/interwovenkit-react/src/pages/deposit/TransferFooter.tsx (9)
packages/interwovenkit-react/src/pages/deposit/hooks.ts (1)
useTransferForm(137-139)packages/interwovenkit-react/src/pages/bridge/data/tx.ts (2)
BridgeTxResult(57-66)useBridgePreviewState(53-55)packages/interwovenkit-react/src/pages/bridge/data/assets.ts (1)
useAllSkipAssets(15-43)packages/interwovenkit-react/src/data/chains.ts (1)
useChain(97-100)packages/interwovenkit-react/src/data/account.ts (1)
useBalances(43-47)packages/interwovenkit-react/src/data/fee.ts (2)
useGasPrices(16-43)useLastFeeDenom(45-75)packages/interwovenkit-react/src/data/assets.ts (1)
useFindAsset(76-88)packages/interwovenkit-react/src/public/data/constants.ts (1)
DEFAULT_GAS_ADJUSTMENT(3-3)packages/interwovenkit-react/src/components/Dropdown.tsx (1)
DropdownOption(7-11)
packages/interwovenkit-react/src/pages/bridge/BridgePreviewFooter.tsx (1)
packages/interwovenkit-react/src/pages/bridge/data/tx.ts (2)
BridgeTxResult(57-66)useBridgeTx(73-297)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cloudflare Pages: interwovenkit-testnet
🔇 Additional comments (3)
packages/interwovenkit-react/src/pages/bridge/BridgePreviewFooter.tsx (3)
1-1: LGTM! Type imports are correct.The type imports for
StdFeeandBridgeTxResultproperly support the new props and align with the bridge transaction handling indata/tx.ts.Also applies to: 5-5
7-13: LGTM! Props interface extended appropriately.The new optional props support the modal-driven flow requirements:
feefor custom fee overrideonCompletedfor handling transaction resultsconfirmMessagefor custom button texterrorfor validation/error feedbackAll props are optional, maintaining backward compatibility.
15-16: LGTM! Hook integration is correct.The component correctly passes
customFeeandonCompletedtouseBridgeTx, which according to the implementation indata/tx.tssupports these options for transaction customization and completion callbacks.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/interwovenkit-react/src/pages/bridge/FooterWithMsgs.tsx (1)
26-63: Race condition: stale fetch results can overwrite current data.If the user changes inputs (slippage, amounts, etc.) while a fetch is in progress, multiple concurrent fetches will execute. The later fetch may complete first, then the earlier fetch overwrites it with stale transaction data. In a bridge/transaction context, this could cause the user to sign a transaction with incorrect parameters, potentially resulting in financial loss.
🔒 Proposed fix with cleanup and cancellation
useEffect(() => { + let cancelled = false + const fetchMessages = async () => { try { if (route.required_op_hook && !signedOpHook) { throw new Error("Op hook is required") } setLoading(true) setError(null) const params = { address_list: addressList, amount_in: route.amount_in, amount_out: route.amount_out, source_asset_chain_id: route.source_asset_chain_id, source_asset_denom: route.source_asset_denom, dest_asset_chain_id: route.dest_asset_chain_id, dest_asset_denom: route.dest_asset_denom, slippage_tolerance_percent: values.slippagePercent, operations: route.operations, signed_op_hook: signedOpHook, } const { txs } = await skip .post("v2/fungible/msgs", { json: params }) .json<MsgsResponseJson>() + + if (cancelled) return + if (!txs || txs.length === 0) throw new Error("No transaction data found") const [tx] = txs setValue(tx) } catch (error) { + if (cancelled) return setError(error as Error) } finally { + if (!cancelled) { setLoading(false) + } } } fetchMessages() + + return () => { + cancelled = true + } }, [route, values, addressList, signedOpHook, skip])
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (1)
packages/interwovenkit-react/src/pages/bridge/FooterWithMsgs.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cloudflare Pages: interwovenkit-testnet
🔇 Additional comments (1)
packages/interwovenkit-react/src/pages/bridge/FooterWithMsgs.tsx (1)
69-77: LGTM! Cleaner loading state consolidation.The refactor properly consolidates the loading and no-value conditions into a single guard clause, making the control flow more straightforward while maintaining type safety.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @packages/interwovenkit-react/src/pages/deposit/SelectExternalAsset.tsx:
- Around line 64-66: The empty-state text always says "deposit" but
SelectExternalAsset supports both flows; update the rendered message to branch
on the isWithdraw prop (used in this component) and show "withdraw" when
isWithdraw is true and "deposit" otherwise, still including localAsset.symbol
and using the same styles.empty element so the UI and styling remain unchanged.
- Around line 37-48: The back button rendered in SelectExternalAsset lacks an
accessible label for screen readers; update the <button> element (styles.close)
that contains <IconBack size={14} /> so it includes an accessible name—either
add an aria-label (e.g., aria-label="Back") or include visually-hidden text
describing the action (e.g., "Back to fields" or "Back to select local"
depending on context using isWithdraw/isExternalSelected) while keeping the
existing onClick logic that calls setValue("page", ...) intact.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (2)
packages/interwovenkit-react/src/pages/deposit/SelectExternalAsset.module.csspackages/interwovenkit-react/src/pages/deposit/SelectExternalAsset.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/interwovenkit-react/src/pages/deposit/SelectExternalAsset.module.css
🧰 Additional context used
🧬 Code graph analysis (1)
packages/interwovenkit-react/src/pages/deposit/SelectExternalAsset.tsx (2)
packages/interwovenkit-react/src/pages/deposit/hooks.ts (4)
useExternalAssetOptions(86-122)useTransferForm(137-139)useLocalAssetDepositAsset(56-69)useLocalAssetOptions(48-54)packages/interwovenkit-react/src/data/config.ts (1)
useConfig(32-36)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cloudflare Pages: interwovenkit-testnet
| <button | ||
| className={styles.close} | ||
| onClick={() => { | ||
| if (isWithdraw || isExternalSelected) { | ||
| setValue("page", "fields") | ||
| } else { | ||
| setValue("page", "select-local") | ||
| } | ||
| }} | ||
| > | ||
| <IconBack size={14} /> | ||
| </button> |
There was a problem hiding this comment.
Add accessible label to back button.
The back button still lacks an accessible label for screen readers. Users relying on assistive technology won't know the button's purpose.
♿ Recommended fix
<button
className={styles.close}
+ aria-label={
+ isWithdraw || isExternalSelected
+ ? "Go back to transfer fields"
+ : "Go back to select local asset"
+ }
onClick={() => {
if (isWithdraw || isExternalSelected) {
setValue("page", "fields")
} else {
setValue("page", "select-local")
}
}}
>📝 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.
| <button | |
| className={styles.close} | |
| onClick={() => { | |
| if (isWithdraw || isExternalSelected) { | |
| setValue("page", "fields") | |
| } else { | |
| setValue("page", "select-local") | |
| } | |
| }} | |
| > | |
| <IconBack size={14} /> | |
| </button> | |
| <button | |
| className={styles.close} | |
| aria-label={ | |
| isWithdraw || isExternalSelected | |
| ? "Go back to transfer fields" | |
| : "Go back to select local asset" | |
| } | |
| onClick={() => { | |
| if (isWithdraw || isExternalSelected) { | |
| setValue("page", "fields") | |
| } else { | |
| setValue("page", "select-local") | |
| } | |
| }} | |
| > | |
| <IconBack size={14} /> | |
| </button> |
🤖 Prompt for AI Agents
In @packages/interwovenkit-react/src/pages/deposit/SelectExternalAsset.tsx
around lines 37 - 48, The back button rendered in SelectExternalAsset lacks an
accessible label for screen readers; update the <button> element (styles.close)
that contains <IconBack size={14} /> so it includes an accessible name—either
add an aria-label (e.g., aria-label="Back") or include visually-hidden text
describing the action (e.g., "Back to fields" or "Back to select local"
depending on context using isWithdraw/isExternalSelected) while keeping the
existing onClick logic that calls setValue("page", ...) intact.
| <p className={styles.empty}> | ||
| You do not have supported assets to deposit {localAsset.symbol}. | ||
| </p> |
There was a problem hiding this comment.
Empty state message is incorrect for withdraw flow.
The message always references "deposit" but this component handles both deposit and withdraw flows (controlled by isWithdraw on line 25).
📝 Recommended fix
<p className={styles.empty}>
- You do not have supported assets to deposit {localAsset.symbol}.
+ {isWithdraw
+ ? `There are no available chains to withdraw ${localAsset.symbol} to.`
+ : `You do not have supported assets to deposit ${localAsset.symbol}.`}
</p>📝 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.
| <p className={styles.empty}> | |
| You do not have supported assets to deposit {localAsset.symbol}. | |
| </p> | |
| <p className={styles.empty}> | |
| {isWithdraw | |
| ? `There are no available chains to withdraw ${localAsset.symbol} to.` | |
| : `You do not have supported assets to deposit ${localAsset.symbol}.`} | |
| </p> |
🤖 Prompt for AI Agents
In @packages/interwovenkit-react/src/pages/deposit/SelectExternalAsset.tsx
around lines 64 - 66, The empty-state text always says "deposit" but
SelectExternalAsset supports both flows; update the rendered message to branch
on the isWithdraw prop (used in this component) and show "withdraw" when
isWithdraw is true and "deposit" otherwise, still including localAsset.symbol
and using the same styles.empty element so the UI and styling remain unchanged.
| <button | ||
| key={`${asset.chain_id}-${asset.denom}`} | ||
| className={clsx(styles.asset, isActive && styles.activeAsset)} | ||
| onClick={() => { | ||
| setValue(externalDenomKey, asset.denom) | ||
| setValue(externalChainIdKey, chain.chain_id) | ||
| if (!isWithdraw) setValue("quantity", "") | ||
| setValue("page", "fields") | ||
| }} | ||
| > | ||
| <div className={styles.iconContainer}> | ||
| <img src={asset.logo_uri} alt={asset.symbol} className={styles.assetIcon} /> | ||
| <img | ||
| src={chain.logo_uri || ""} | ||
| alt={chain.chain_name} | ||
| className={styles.chainIcon} | ||
| /> | ||
| </div> | ||
| <p className={styles.assetName}> | ||
| {asset.symbol}{" "} | ||
| {isActive && ( | ||
| <span> | ||
| <IconCheck size={16} /> | ||
| </span> | ||
| )} | ||
| </p> | ||
| <p className={styles.assetChain}>on {chain.pretty_name}</p> | ||
| {balance && ( | ||
| <> | ||
| <p className={styles.balance}> | ||
| {formatAmount(balance.amount, { decimals: balance.decimals || 6 })} | ||
| </p> | ||
| <p className={styles.value}>{formatValue(balance.value_usd || 0)}</p> | ||
| </> | ||
| )} | ||
| </button> |
There was a problem hiding this comment.
Asset buttons lack accessible labels.
Each asset button only contains visual elements (images, text nodes) without a programmatic label. Screen readers cannot convey the complete button purpose to users.
♿ Recommended fix
Add an aria-label to each button that describes the full action:
<button
key={`${asset.chain_id}-${asset.denom}`}
className={clsx(styles.asset, isActive && styles.activeAsset)}
+ aria-label={`Select ${asset.symbol} on ${chain.pretty_name}${
+ balance ? ` with balance ${formatAmount(balance.amount, { decimals: balance.decimals || 6 })}` : ""
+ }`}
onClick={() => {
setValue(externalDenomKey, asset.denom)
setValue(externalChainIdKey, chain.chain_id)
if (!isWithdraw) setValue("quantity", "")
setValue("page", "fields")
}}
>📝 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.
| <button | |
| key={`${asset.chain_id}-${asset.denom}`} | |
| className={clsx(styles.asset, isActive && styles.activeAsset)} | |
| onClick={() => { | |
| setValue(externalDenomKey, asset.denom) | |
| setValue(externalChainIdKey, chain.chain_id) | |
| if (!isWithdraw) setValue("quantity", "") | |
| setValue("page", "fields") | |
| }} | |
| > | |
| <div className={styles.iconContainer}> | |
| <img src={asset.logo_uri} alt={asset.symbol} className={styles.assetIcon} /> | |
| <img | |
| src={chain.logo_uri || ""} | |
| alt={chain.chain_name} | |
| className={styles.chainIcon} | |
| /> | |
| </div> | |
| <p className={styles.assetName}> | |
| {asset.symbol}{" "} | |
| {isActive && ( | |
| <span> | |
| <IconCheck size={16} /> | |
| </span> | |
| )} | |
| </p> | |
| <p className={styles.assetChain}>on {chain.pretty_name}</p> | |
| {balance && ( | |
| <> | |
| <p className={styles.balance}> | |
| {formatAmount(balance.amount, { decimals: balance.decimals || 6 })} | |
| </p> | |
| <p className={styles.value}>{formatValue(balance.value_usd || 0)}</p> | |
| </> | |
| )} | |
| </button> | |
| <button | |
| key={`${asset.chain_id}-${asset.denom}`} | |
| className={clsx(styles.asset, isActive && styles.activeAsset)} | |
| aria-label={`Select ${asset.symbol} on ${chain.pretty_name}${ | |
| balance ? ` with balance ${formatAmount(balance.amount, { decimals: balance.decimals || 6 })}` : "" | |
| }`} | |
| onClick={() => { | |
| setValue(externalDenomKey, asset.denom) | |
| setValue(externalChainIdKey, chain.chain_id) | |
| if (!isWithdraw) setValue("quantity", "") | |
| setValue("page", "fields") | |
| }} | |
| > | |
| <div className={styles.iconContainer}> | |
| <img src={asset.logo_uri} alt={asset.symbol} className={styles.assetIcon} /> | |
| <img | |
| src={chain.logo_uri || ""} | |
| alt={chain.chain_name} | |
| className={styles.chainIcon} | |
| /> | |
| </div> | |
| <p className={styles.assetName}> | |
| {asset.symbol}{" "} | |
| {isActive && ( | |
| <span> | |
| <IconCheck size={16} /> | |
| </span> | |
| )} | |
| </p> | |
| <p className={styles.assetChain}>on {chain.pretty_name}</p> | |
| {balance && ( | |
| <> | |
| <p className={styles.balance}> | |
| {formatAmount(balance.amount, { decimals: balance.decimals || 6 })} | |
| </p> | |
| <p className={styles.value}>{formatValue(balance.value_usd || 0)}</p> | |
| </> | |
| )} | |
| </button> |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @packages/interwovenkit-react/src/pages/deposit/TransferTxDetails.tsx:
- Around line 36-40: minimumReceived calculation can throw when
values.slippagePercent is missing or invalid; validate and normalize
slippagePercent before converting to BigInt: parse it to a finite number (e.g.,
use Number or parseFloat), fallback to 0 if NaN, clamp to allowed range (>=0),
compute slippageBasisPoints as Math.round(slippagePercent * 100) to get an
integer, then use that integer when computing BigInt(10000 -
slippageBasisPoints); update the expression around minimumReceived (the
route.does_swap branch) to use the validated slippageBasisPoints so BigInt never
receives NaN.
- Around line 66-72: The Receiving address block in TransferTxDetails uses
walletIcon from useConnectedWalletIcon() which can be undefined, causing a
broken <img>; update the rendering in the TransferTxDetails component so the
icon is only rendered when walletIcon is truthy (e.g., conditionally render the
<img> inside the detailValue or render a fallback element) and leave
truncate(address) unchanged; reference the walletIcon variable and the
useConnectedWalletIcon hook to locate the change.
🧹 Nitpick comments (2)
packages/interwovenkit-react/src/pages/deposit/TransferTxDetails.tsx (2)
15-17: Redundant| undefinedin optional prop type.The
?modifier already implies the property can be undefined.Suggested simplification
interface Props { - renderFee?: (() => React.ReactNode) | undefined + renderFee?: () => React.ReactNode }
85-91: Consider fallback for missinglogo_uri.If
dstAsset.logo_uriis undefined or null, this renders a broken image. Consider providing a placeholder or conditionally rendering the image.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (1)
packages/interwovenkit-react/src/pages/deposit/TransferTxDetails.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
packages/interwovenkit-react/src/pages/deposit/TransferTxDetails.tsx (5)
packages/interwovenkit-react/src/pages/bridge/data/simulate.ts (1)
RouterRouteResponseJson(14-19)packages/interwovenkit-react/src/pages/bridge/data/tx.ts (1)
useBridgePreviewState(53-55)packages/interwovenkit-react/src/pages/bridge/data/assets.ts (1)
useAllSkipAssets(15-43)packages/interwovenkit-react/src/public/data/hooks.ts (1)
useInitiaAddress(17-21)packages/interwovenkit-react/src/hooks/useConnectedWalletIcon.ts (1)
useConnectedWalletIcon(3-11)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cloudflare Pages: interwovenkit-testnet
🔇 Additional comments (1)
packages/interwovenkit-react/src/pages/deposit/TransferTxDetails.tsx (1)
21-22: Potential runtime error ifvaluesis undefined.
useBridgePreviewState()returnsuseLocationState<BridgePreviewState>(), which may returnundefinedif the location state is not set. Destructuringvalueson line 21 and then accessingdstDenom,dstChainIdon line 22 will throw ifvaluesis undefined.Consider adding a guard or providing defaults:
Suggested defensive approach
- const { values } = useBridgePreviewState() - const { dstDenom, dstChainId } = values + const state = useBridgePreviewState() + const values = state?.values + const { dstDenom, dstChainId } = values ?? {}Then update the early return on line 34:
- if (!route || !dstAsset) return null + if (!route || !dstAsset || !values) return null
| const minimumReceived = route.does_swap | ||
| ? (BigInt(route.amount_out || route.estimated_amount_out) * | ||
| BigInt(10000 - Number(values.slippagePercent) * 100)) / | ||
| BigInt(10000) | ||
| : BigInt(route.amount_out || route.estimated_amount_out) |
There was a problem hiding this comment.
Potential BigInt conversion error if slippagePercent is invalid.
If values.slippagePercent is undefined or not a valid number, Number(values.slippagePercent) * 100 yields NaN, and BigInt(10000 - NaN) throws a TypeError.
Suggested fix with fallback
const minimumReceived = route.does_swap
- ? (BigInt(route.amount_out || route.estimated_amount_out) *
- BigInt(10000 - Number(values.slippagePercent) * 100)) /
- BigInt(10000)
+ ? (BigInt(route.amount_out || route.estimated_amount_out) *
+ BigInt(10000 - Math.round((Number(values.slippagePercent) || 0) * 100))) /
+ BigInt(10000)
: BigInt(route.amount_out || route.estimated_amount_out)📝 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.
| const minimumReceived = route.does_swap | |
| ? (BigInt(route.amount_out || route.estimated_amount_out) * | |
| BigInt(10000 - Number(values.slippagePercent) * 100)) / | |
| BigInt(10000) | |
| : BigInt(route.amount_out || route.estimated_amount_out) | |
| const minimumReceived = route.does_swap | |
| ? (BigInt(route.amount_out || route.estimated_amount_out) * | |
| BigInt(10000 - Math.round((Number(values.slippagePercent) || 0) * 100))) / | |
| BigInt(10000) | |
| : BigInt(route.amount_out || route.estimated_amount_out) |
🤖 Prompt for AI Agents
In @packages/interwovenkit-react/src/pages/deposit/TransferTxDetails.tsx around
lines 36 - 40, minimumReceived calculation can throw when values.slippagePercent
is missing or invalid; validate and normalize slippagePercent before converting
to BigInt: parse it to a finite number (e.g., use Number or parseFloat),
fallback to 0 if NaN, clamp to allowed range (>=0), compute slippageBasisPoints
as Math.round(slippagePercent * 100) to get an integer, then use that integer
when computing BigInt(10000 - slippageBasisPoints); update the expression around
minimumReceived (the route.does_swap branch) to use the validated
slippageBasisPoints so BigInt never receives NaN.
| {address && ( | ||
| <div className={styles.detail}> | ||
| <p className={styles.detailLabel}>Receiving address</p> | ||
| <p className={styles.detailValue}> | ||
| <img src={walletIcon} alt="Wallet" height={12} width={12} /> {truncate(address)} | ||
| </p> | ||
| </div> |
There was a problem hiding this comment.
Handle undefined walletIcon to avoid broken image.
useConnectedWalletIcon() can return undefined when no wallet connector is present. Rendering <img src={walletIcon} /> with an undefined src displays a broken image icon.
Suggested fix
- {address && (
+ {address && walletIcon && (
<div className={styles.detail}>
<p className={styles.detailLabel}>Receiving address</p>
<p className={styles.detailValue}>
<img src={walletIcon} alt="Wallet" height={12} width={12} /> {truncate(address)}
</p>
</div>
)}Alternatively, conditionally render the icon only if defined:
<p className={styles.detailValue}>
- <img src={walletIcon} alt="Wallet" height={12} width={12} /> {truncate(address)}
+ {walletIcon && <img src={walletIcon} alt="Wallet" height={12} width={12} />} {truncate(address)}
</p>📝 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.
| {address && ( | |
| <div className={styles.detail}> | |
| <p className={styles.detailLabel}>Receiving address</p> | |
| <p className={styles.detailValue}> | |
| <img src={walletIcon} alt="Wallet" height={12} width={12} /> {truncate(address)} | |
| </p> | |
| </div> | |
| {address && walletIcon && ( | |
| <div className={styles.detail}> | |
| <p className={styles.detailLabel}>Receiving address</p> | |
| <p className={styles.detailValue}> | |
| <img src={walletIcon} alt="Wallet" height={12} width={12} /> {truncate(address)} | |
| </p> | |
| </div> | |
| )} |
🤖 Prompt for AI Agents
In @packages/interwovenkit-react/src/pages/deposit/TransferTxDetails.tsx around
lines 66 - 72, The Receiving address block in TransferTxDetails uses walletIcon
from useConnectedWalletIcon() which can be undefined, causing a broken <img>;
update the rendering in the TransferTxDetails component so the icon is only
rendered when walletIcon is truthy (e.g., conditionally render the <img> inside
the detailValue or render a fallback element) and leave truncate(address)
unchanged; reference the walletIcon variable and the useConnectedWalletIcon hook
to locate the change.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @packages/interwovenkit-react/src/pages/deposit/WithdrawFields.tsx:
- Line 74: The recipient assignment uses
InitiaAddress(state.recipientAddress).hex which can throw on invalid input; wrap
the conversion in a validation/try-catch around InitiaAddress (referencing
InitiaAddress and state.recipientAddress) and on error fallback to hexAddress
(or set recipient to an empty/validated value) and optionally log or surface the
error so the component won't crash; ensure the resulting value assigned to
recipient is always a safe hex string.
- Around line 54-60: Refactor the imperative disabledMessage logic in
WithdrawFields.tsx into a memoized value: replace the let/if block that sets
disabledMessage (which checks localAsset, Number(quantity), and compares
Number(quantity) to Number(formatAmount(balance, { decimals:
localAsset?.decimals || 6 }))) with a useMemo hook that returns the same string
values ("Select asset", "Enter amount", "Insufficient balance" or undefined)
based on those same inputs; ensure the dependency array includes localAsset,
quantity, balance and any helper like formatAmount so the value updates
correctly and matches the pattern used in DepositFields.tsx.
- Around line 68-81: The effect depends on the unstable getValues reference
causing re-runs; wrap getValues in a stable callback (e.g., create
stableGetValues via useCallback(() => getValues(), [getValues])) and replace
getValues with stableGetValues in the useEffect dependency array (or
alternatively read form values once outside the effect and pass that snapshot
into navigate) so the effect no longer retriggers on each render; update
references to getValues in WithdrawFields.tsx accordingly.
In @packages/interwovenkit-react/src/public/app/Modal.tsx:
- Line 18: The Dialog.Backdrop currently calls closeModal via onClick which can
duplicate the dialog's built-in dismiss behavior; remove the explicit onClick
from Dialog.Backdrop and rely on Dialog.Root's dismissible/onOpenChange handling
(or alternatively set dismissible={false} on Dialog.Root if you want to handle
backdrop clicks manually), ensuring closeModal is only invoked once via the
onOpenChange handler that receives open state changes.
🧹 Nitpick comments (6)
packages/interwovenkit-react/src/public/app/Modal.tsx (1)
19-19: Unnecessaryclsxwrapper for single class.
clsx(styles.modal)with a single argument adds no value. You can passstyles.modaldirectly.Suggested simplification
- <Dialog.Popup className={clsx(styles.modal)} ref={setContainer}> + <Dialog.Popup className={styles.modal} ref={setContainer}>packages/interwovenkit-react/src/pages/deposit/WithdrawFields.tsx (1)
117-125: Optimize balance comparison in MAX button.The
onClickhandler comparesNumber(quantity)withformatAmount(balance, ...)on every click, andformatAmountis also called on line 124. Consider computing the formatted balance once outside the handler.♻️ Compute formatted balance once
+ const formattedBalance = useMemo( + () => formatAmount(balance, { decimals: localAsset?.decimals || 6 }), + [balance, localAsset?.decimals] + ) + {balance && ( <div className={styles.balanceContainer}> <p className={styles.value}>{quantityValue ? formatValue(quantityValue) : "$-"}</p> <button className={styles.maxButton} onClick={() => { - if ( - Number(quantity) === - Number(formatAmount(balance, { decimals: localAsset?.decimals || 6 })) - ) + if (Number(quantity) === Number(formattedBalance)) return - setValue("quantity", formatAmount(balance, { decimals: localAsset?.decimals || 6 })) + setValue("quantity", formattedBalance) }} > - <IconWallet size={16} />{" "} - {formatAmount(balance, { decimals: localAsset?.decimals || 6 })} <span>MAX</span> + <IconWallet size={16} /> {formattedBalance} <span>MAX</span> </button> </div> )}packages/interwovenkit-react/src/pages/deposit/DepositFields.tsx (2)
62-63: Remove or complete the incomplete comment.The comment "Destructure error fields in deps to properly track each field change" doesn't align with the actual dependencies
[quantity, balance, externalAsset]. There are no "error fields" being destructured, making this comment misleading.📝 Clarify or remove comment
- // Destructure error fields in deps to properly track each field change }, [quantity, balance, externalAsset])
32-198: Consider extracting shared logic to reduce duplication.
DepositFields.tsxandWithdrawFields.tsxshare substantial logic (balance handling, MAX button, route queries, footer rendering, navigation effects). This duplication increases maintenance burden and the risk of inconsistencies (e.g., thedisabledMessageimplementation differs between them).Consider extracting shared logic into:
- A custom hook for balance/quantity/MAX logic
- A shared
TransferFieldscomponent that accepts a mode prop ("deposit"or"withdraw")- Shared utility functions for address conversion, balance formatting, etc.
This would ensure consistent behavior and reduce the risk of divergence.
packages/interwovenkit-react/src/pages/deposit/SelectExternalAsset.tsx (2)
30-50: Optimize function definition to prevent recreation.The
renderBackButtonfunction is defined inside the component body, causing it to be recreated on every render. While this doesn't break functionality, moving it outside the component or usinguseCallbackwould be more idiomatic.♻️ Move function outside or use useCallback
Option 1: Move outside (preferred if no dependencies change)
+function BackButton({ + isWithdraw, + isExternalSelected, + optionsLength, + setValue, +}: { + isWithdraw: boolean + isExternalSelected: boolean + optionsLength: number + setValue: (key: string, value: string) => void +}) { + if (!isExternalSelected && optionsLength <= 1 && !isWithdraw) return null + + return ( + <button + className={styles.close} + onClick={() => { + if (isWithdraw || isExternalSelected) { + setValue("page", "fields") + } else { + setValue("page", "select-local") + } + }} + > + <IconBack size={20} /> + </button> + ) +} + const SelectExternalAsset = () => { // ... other code ... - function renderBackButton() { - const externalDenom = isWithdraw ? dstDenom : srcDenom - const externalChainId = isWithdraw ? dstChainId : srcChainId - const isExternalSelected = externalDenom && externalChainId - if (!isExternalSelected && options.length <= 1 && !isWithdraw) return null - - return ( - <button - className={styles.close} - onClick={() => { - if (isWithdraw || isExternalSelected) { - setValue("page", "fields") - } else { - setValue("page", "select-local") - } - }} - > - <IconBack size={20} /> - </button> - ) - } + const externalDenom = isWithdraw ? dstDenom : srcDenom + const externalChainId = isWithdraw ? dstChainId : srcChainId + const isExternalSelected = !!(externalDenom && externalChainId) // ... rest of component ... - {renderBackButton()} + <BackButton + isWithdraw={isWithdraw} + isExternalSelected={isExternalSelected} + optionsLength={options.length} + setValue={setValue} + />
88-136: Memoize sorted asset list to avoid re-sorting on every render.The asset sorting by
value_usd(lines 90-93) executes on every render, even whenfilteredAssetshasn't changed. This is unnecessary work, especially with large asset lists.Additionally, lines 95-96 recompute
externalDenomandexternalChainIdfor each asset in the map, when these values are constant for the entire list.⚡ Memoize sorted assets and lift constant variables
+ const externalDenom = isWithdraw ? dstDenom : srcDenom + const externalChainId = isWithdraw ? dstChainId : srcChainId + + const sortedAssets = useMemo( + () => + filteredAssets.sort( + ({ balance: a }, { balance: b }) => + Number(b?.value_usd ?? 0) - Number(a?.value_usd ?? 0), + ), + [filteredAssets] + ) <div className={styles.list}> {isLoading && Array.from({ length: 5 }).map((_, i) => ( <div key={i} className={clsx(styles.asset, styles.placeholder)}> <div className={styles.iconContainer}> <div className={styles.assetIcon} /> <div className={styles.chainIcon} /> </div> <div className={styles.assetPlaceholder} /> </div> ))} {!isLoading && - filteredAssets - .sort( - ({ balance: a }, { balance: b }) => - Number(b?.value_usd ?? 0) - Number(a?.value_usd ?? 0), - ) - .map(({ asset, chain, balance }) => { - const externalDenom = isWithdraw ? dstDenom : srcDenom - const externalChainId = isWithdraw ? dstChainId : srcChainId + sortedAssets.map(({ asset, chain, balance }) => { const isActive = externalDenom === asset.denom && externalChainId === chain.chain_id return ( <button key={`${asset.chain_id}-${asset.denom}`} className={clsx(styles.asset, isActive && styles.activeAsset)} onClick={() => { setValue(externalDenomKey, asset.denom) setValue(externalChainIdKey, chain.chain_id) if (!isWithdraw) setValue("quantity", "") setValue("page", "fields") }} > {/* ... rest of button content ... */} </button> ) })} </div>
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (7)
packages/interwovenkit-react/src/pages/deposit/DepositFields.tsxpackages/interwovenkit-react/src/pages/deposit/Fields.module.csspackages/interwovenkit-react/src/pages/deposit/SelectExternalAsset.module.csspackages/interwovenkit-react/src/pages/deposit/SelectExternalAsset.tsxpackages/interwovenkit-react/src/pages/deposit/WithdrawFields.tsxpackages/interwovenkit-react/src/public/app/Modal.module.csspackages/interwovenkit-react/src/public/app/Modal.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/interwovenkit-react/src/pages/deposit/Fields.module.css
- packages/interwovenkit-react/src/public/app/Modal.module.css
- packages/interwovenkit-react/src/pages/deposit/SelectExternalAsset.module.css
🧰 Additional context used
🧬 Code graph analysis (2)
packages/interwovenkit-react/src/pages/deposit/WithdrawFields.tsx (4)
packages/interwovenkit-react/src/pages/bridge/data/simulate.ts (2)
RouterRouteResponseJson(14-19)useRouteQuery(21-74)packages/interwovenkit-react/src/pages/deposit/hooks.ts (5)
useExternalAssetOptions(86-122)useAllBalancesQuery(17-46)useTransferForm(137-139)useLocalAssetDepositAsset(56-69)useExternalDepositAsset(71-84)packages/interwovenkit-react/src/pages/bridge/data/chains.ts (1)
useFindSkipChain(56-78)packages/interwovenkit-react/src/public/data/hooks.ts (1)
useHexAddress(23-29)
packages/interwovenkit-react/src/public/app/Modal.tsx (1)
packages/interwovenkit-react/src/data/ui.ts (1)
useModal(35-58)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cloudflare Pages: interwovenkit-testnet
🔇 Additional comments (1)
packages/interwovenkit-react/src/public/app/Modal.tsx (1)
10-27: Overall component structure looks good.The Modal correctly:
- Uses controlled Dialog state via
useModalhook- Leverages portal container for proper DOM mounting
- Provides accessible close button with
aria-label- Forwards ref to
setContainerfor nested portal context
packages/interwovenkit-react/src/pages/deposit/WithdrawFields.tsx
Outdated
Show resolved
Hide resolved
| useEffect(() => { | ||
| navigate(0, { | ||
| ...state, | ||
| route, | ||
| values: { | ||
| sender: hexAddress, | ||
| recipient: state.recipientAddress ? InitiaAddress(state.recipientAddress).hex : hexAddress, | ||
| slippagePercent: "1", | ||
| ...getValues(), | ||
| }, | ||
| }) | ||
|
|
||
| // we don't want to trigger this when state changes, to avoid infinite loop | ||
| }, [route, getValues, hexAddress, navigate]) // eslint-disable-line react-hooks/exhaustive-deps |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's look at the WithdrawFields.tsx file to understand context
cd packages/interwovenkit-react/src/pages/deposit/ 2>/dev/null && ls -la | head -20Repository: initia-labs/interwovenkit
Length of output: 1374
🏁 Script executed:
# Get the complete WithdrawFields.tsx file to understand the full context
wc -l packages/interwovenkit-react/src/pages/deposit/WithdrawFields.tsxRepository: initia-labs/interwovenkit
Length of output: 138
🏁 Script executed:
# Read the WithdrawFields.tsx file around the mentioned lines
head -100 packages/interwovenkit-react/src/pages/deposit/WithdrawFields.tsx | tail -50Repository: initia-labs/interwovenkit
Length of output: 1577
🏁 Script executed:
# Also look at the imports to understand where getValues comes from
head -20 packages/interwovenkit-react/src/pages/deposit/WithdrawFields.tsxRepository: initia-labs/interwovenkit
Length of output: 1040
🏁 Script executed:
# Search for other useEffect hooks that depend on getValues in the deposit folder
rg -n -B 3 -A 10 'useEffect\(' packages/interwovenkit-react/src/pages/deposit/ | head -100Repository: initia-labs/interwovenkit
Length of output: 2743
🏁 Script executed:
# Search for getValues and useForm usage in WithdrawFields.tsx
rg -n 'getValues|useForm' packages/interwovenkit-react/src/pages/deposit/WithdrawFields.tsxRepository: initia-labs/interwovenkit
Length of output: 258
🏁 Script executed:
# Look at the full WithdrawFields.tsx file to see the component structure
cat -n packages/interwovenkit-react/src/pages/deposit/WithdrawFields.tsx | head -120Repository: initia-labs/interwovenkit
Length of output: 5100
🏁 Script executed:
# Check where navigate comes from
rg -n 'useNavigate|navigate' packages/interwovenkit-react/src/pages/deposit/WithdrawFields.tsx | head -20Repository: initia-labs/interwovenkit
Length of output: 328
🏁 Script executed:
# Find the useTransferForm hook implementation
fd -n "hooks" packages/interwovenkit-react/src/pages/deposit/ -type fRepository: initia-labs/interwovenkit
Length of output: 299
🏁 Script executed:
# Read the hooks file
cat packages/interwovenkit-react/src/pages/deposit/hooks.tsRepository: initia-labs/interwovenkit
Length of output: 4841
🏁 Script executed:
# Find the useNavigate hook implementation
fd -n "router" packages/interwovenkit-react/src/lib/ -type fRepository: initia-labs/interwovenkit
Length of output: 299
🏁 Script executed:
# Read the router file to understand useNavigate
cat packages/interwovenkit-react/src/lib/router.ts 2>/dev/null || cat packages/interwovenkit-react/src/lib/router.tsx 2>/dev/null || cat packages/interwovenkit-react/src/lib/router/index.ts 2>/dev/nullRepository: initia-labs/interwovenkit
Length of output: 198
🏁 Script executed:
# Find all files in the router directory
find packages/interwovenkit-react/src/lib/router* -type fRepository: initia-labs/interwovenkit
Length of output: 296
🏁 Script executed:
# Look for the RouterContext file
cat packages/interwovenkit-react/src/lib/router/RouterContext.ts 2>/dev/null || cat packages/interwovenkit-react/src/lib/router/RouterContext.tsx 2>/dev/nullRepository: initia-labs/interwovenkit
Length of output: 1083
🏁 Script executed:
# Search for useNavigate implementation
rg -n 'export.*useNavigate|function useNavigate' packages/interwovenkit-react/src/lib/ -A 10Repository: initia-labs/interwovenkit
Length of output: 824
🏁 Script executed:
# Check if getValues is wrapped in useCallback or similar in hooks.ts
rg -n 'useCallback|useMemo' packages/interwovenkit-react/src/pages/deposit/hooks.tsRepository: initia-labs/interwovenkit
Length of output: 51
🏁 Script executed:
# Search the entire deposit folder for any memoization of getValues
rg -n 'useCallback.*getValues|useMemo.*getValues' packages/interwovenkit-react/src/pages/deposit/Repository: initia-labs/interwovenkit
Length of output: 51
🏁 Script executed:
# Check the MemoryRouter implementation to see if navigate is memoized
cat packages/interwovenkit-react/src/lib/router/MemoryRouter.tsxRepository: initia-labs/interwovenkit
Length of output: 1944
🏁 Script executed:
# Also verify react-hook-form's getValues behavior - check if there are any docs or comments
rg -n 'getValues|react-hook-form' packages/interwovenkit-react/src/pages/deposit/WithdrawFields.tsx -B 2 -A 2Repository: initia-labs/interwovenkit
Length of output: 660
🏁 Script executed:
# Check if there are any similar patterns in DepositFields.tsx
cat -n packages/interwovenkit-react/src/pages/deposit/DepositFields.tsx | sed -n '71,84p'Repository: initia-labs/interwovenkit
Length of output: 626
🏁 Script executed:
# Check useHexAddress implementation to see if it returns a stable value
rg -n 'export.*useHexAddress|function useHexAddress' packages/interwovenkit-react/ -A 10Repository: initia-labs/interwovenkit
Length of output: 1078
🏁 Script executed:
# Check useRouteQuery to see if route changes frequently
rg -n 'export.*useRouteQuery|function useRouteQuery' packages/interwovenkit-react/ -B 5 -A 15 | head -60Repository: initia-labs/interwovenkit
Length of output: 2115
🏁 Script executed:
# Let's understand what react-hook-form's getValues behavior is
# by checking the package.json to see the version
cat packages/interwovenkit-react/package.json | grep -A 2 -B 2 'react-hook-form'Repository: initia-labs/interwovenkit
Length of output: 430
🏁 Script executed:
# Check if there's any documentation comment or usage of getValues elsewhere
rg -n 'getValues' packages/interwovenkit-react/src/pages/ --no-filename | head -30Repository: initia-labs/interwovenkit
Length of output: 688
🏁 Script executed:
# Let's see the complete DepositFields.tsx to verify it has the same pattern
wc -l packages/interwovenkit-react/src/pages/deposit/DepositFields.tsxRepository: initia-labs/interwovenkit
Length of output: 137
🏁 Script executed:
# Check if there are any actual infinite loop issues reported or handled
rg -n 'infinite|loop' packages/interwovenkit-react/src/pages/deposit/ -iRepository: initia-labs/interwovenkit
Length of output: 571
🌐 Web query:
react-hook-form v7 getValues stable reference memoized
💡 Result:
Short answer: React Hook Form's docs describe getValues as an optimized helper that reads values without subscribing or triggering re-renders, but they don't guarantee its function identity is stable across renders. If you need a stable/memoized reference for deps or callbacks, store/memoize it yourself.
How to make a stable reference
- useRef: const getValuesRef = useRef(getValues); useEffect(() => { getValuesRef.current = getValues }, [getValues]); then call getValuesRef.current().
- useCallback wrapper: const stableGetValues = useCallback((...args) => getValues(...args), [getValues]); (works if you accept re-creating stableGetValues only when getValues changes).
- Or capture values via useWatch/useWatch + getValues combo when you need reactive values without rerenders.
References
- getValues docs (description and behavior). [1]
- FAQ: difference between watch and getValues. [2]
- GitHub discussions about stable form state / object references (context why some form APIs may change references). [3]
References:
[1] https://react-hook-form.com/docs/useform/getvalues
[2] https://react-hook-form.com/faqs
[3] react-hook-form/react-hook-form#3706
Fix unstable getValues dependency in useEffect to prevent infinite loops.
The getValues function from react-hook-form v7 does not have a stable reference across renders—it returns a new function on each render. Including it in the effect dependency array causes the effect to re-run whenever it's called, which triggers navigation updates, re-renders the component, and generates a new getValues reference, creating an infinite loop cycle.
The current eslint-disable comment acknowledges excluding state to avoid infinite loops, but the real issue is getValues itself. Wrap getValues with useCallback to create a stable reference:
const stableGetValues = useCallback(() => getValues(), [getValues])Then update the dependency array to use stableGetValues instead of getValues, or restructure the effect to capture form values directly without relying on getValues in the dependencies.
🤖 Prompt for AI Agents
In @packages/interwovenkit-react/src/pages/deposit/WithdrawFields.tsx around
lines 68 - 81, The effect depends on the unstable getValues reference causing
re-runs; wrap getValues in a stable callback (e.g., create stableGetValues via
useCallback(() => getValues(), [getValues])) and replace getValues with
stableGetValues in the useEffect dependency array (or alternatively read form
values once outside the effect and pass that snapshot into navigate) so the
effect no longer retriggers on each render; update references to getValues in
WithdrawFields.tsx accordingly.
| route, | ||
| values: { | ||
| sender: hexAddress, | ||
| recipient: state.recipientAddress ? InitiaAddress(state.recipientAddress).hex : hexAddress, |
There was a problem hiding this comment.
Add error handling for address conversion.
InitiaAddress(state.recipientAddress).hex can throw if state.recipientAddress contains an invalid address format. This would crash the component.
🛡️ Add try-catch or validation
useEffect(() => {
+ let recipientHex = hexAddress
+ if (state.recipientAddress) {
+ try {
+ recipientHex = InitiaAddress(state.recipientAddress).hex
+ } catch {
+ // Fall back to hexAddress if conversion fails
+ recipientHex = hexAddress
+ }
+ }
+
navigate(0, {
...state,
route,
values: {
sender: hexAddress,
- recipient: state.recipientAddress ? InitiaAddress(state.recipientAddress).hex : hexAddress,
+ recipient: recipientHex,
slippagePercent: "1",
...getValues(),
},
})
// we don't want to trigger this when state changes, to avoid infinite loop
}, [route, getValues, hexAddress, navigate]) // eslint-disable-line react-hooks/exhaustive-deps📝 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.
| recipient: state.recipientAddress ? InitiaAddress(state.recipientAddress).hex : hexAddress, | |
| useEffect(() => { | |
| let recipientHex = hexAddress | |
| if (state.recipientAddress) { | |
| try { | |
| recipientHex = InitiaAddress(state.recipientAddress).hex | |
| } catch { | |
| // Fall back to hexAddress if conversion fails | |
| recipientHex = hexAddress | |
| } | |
| } | |
| navigate(0, { | |
| ...state, | |
| route, | |
| values: { | |
| sender: hexAddress, | |
| recipient: recipientHex, | |
| slippagePercent: "1", | |
| ...getValues(), | |
| }, | |
| }) | |
| // we don't want to trigger this when state changes, to avoid infinite loop | |
| }, [route, getValues, hexAddress, navigate]) // eslint-disable-line react-hooks/exhaustive-deps |
🤖 Prompt for AI Agents
In @packages/interwovenkit-react/src/pages/deposit/WithdrawFields.tsx at line
74, The recipient assignment uses InitiaAddress(state.recipientAddress).hex
which can throw on invalid input; wrap the conversion in a validation/try-catch
around InitiaAddress (referencing InitiaAddress and state.recipientAddress) and
on error fallback to hexAddress (or set recipient to an empty/validated value)
and optionally log or surface the error so the component won't crash; ensure the
resulting value assigned to recipient is always a safe hex string.
| return ( | ||
| <Dialog.Root open={isModalOpen} onOpenChange={(isOpen) => !isOpen && closeModal()}> | ||
| <Dialog.Portal container={portalContainer}> | ||
| <Dialog.Backdrop className={styles.backdrop} onClick={closeModal} /> |
There was a problem hiding this comment.
Potential double-close with backdrop click handler.
Base UI Dialog has a dismissible prop (true by default) that automatically closes the dialog when clicking outside the Popup. Adding an explicit onClick={closeModal} on the Backdrop may cause closeModal() to be invoked twice—once from your handler and once from the dialog's internal dismiss logic via onOpenChange. This could trigger duplicate analytics events ("Widget Closed" tracked twice).
Consider removing the explicit onClick and relying on the built-in dismiss behavior, or explicitly set dismissible={false} on Dialog.Root if you want manual control.
Proposed fix (remove redundant onClick)
- <Dialog.Backdrop className={styles.backdrop} onClick={closeModal} />
+ <Dialog.Backdrop className={styles.backdrop} />📝 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.
| <Dialog.Backdrop className={styles.backdrop} onClick={closeModal} /> | |
| <Dialog.Backdrop className={styles.backdrop} /> |
🤖 Prompt for AI Agents
In @packages/interwovenkit-react/src/public/app/Modal.tsx at line 18, The
Dialog.Backdrop currently calls closeModal via onClick which can duplicate the
dialog's built-in dismiss behavior; remove the explicit onClick from
Dialog.Backdrop and rely on Dialog.Root's dismissible/onOpenChange handling (or
alternatively set dismissible={false} on Dialog.Root if you want to handle
backdrop clicks manually), ensuring closeModal is only invoked once via the
onOpenChange handler that receives open state changes.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @packages/interwovenkit-react/src/components/form/NumericInput.tsx:
- Line 30: The local displayValue state (const [displayValue, setDisplayValue])
causes two issues: empty string is treated as falsy by the expression using ||
and external updates to field.value are ignored; to fix, change any fallback
from displayValue || field.value to use nullish coalescing (displayValue ??
field.value) so "" is preserved, and add a useEffect that watches field.value
(or the form value) to call setDisplayValue when field.value changes so the
input stays in sync with programmatic updates; update any related logic that
compares Number(displayValue) so it handles "" explicitly if needed.
- Around line 42-48: The numeric equivalence check in NumericInput (variables
currentNumeric/newNumeric) treats empty string as "0", which blocks clearing "0"
to empty; update the condition so you still call field.onChange(sanitized) when
sanitized === "" (or when field.value === "" differs), and only perform the
Number(...) equality shortcut when both field.value and sanitized are non-empty;
locate the comparison around the if (Number(currentNumeric) !==
Number(newNumeric)) and adjust it to allow cleared values while keeping
numeric-equality suppression for genuinely equivalent numeric inputs before
calling field.onChange(sanitized).
- Around line 62-63: The paste validation regex in NumericInput.tsx
(formattedNumberRegex) rejects inputs like ".5"; update the check so it allows a
leading decimal by replacing the current regex with one that permits zero digits
before the decimal and requires digits after it (for example
/^[0-9,]*\.?[0-9]+$/), or alternatively call the existing sanitizeNumericInput
logic against pastedText instead of using formattedNumberRegex; ensure the same
symbols are updated (formattedNumberRegex and the if
(!formattedNumberRegex.test(pastedText)) return) so ".5" is accepted.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (1)
packages/interwovenkit-react/src/components/form/NumericInput.tsx
🔇 Additional comments (1)
packages/interwovenkit-react/src/components/form/NumericInput.tsx (1)
68-71: Props spread order allows overriding defaults.The
{...attrs}spread afterplaceholderandinputModeallows callers to override these defaults. This is likely intentional for flexibility, but verify this is the desired behavior.
| function NumericInput<T extends FieldValues>(props: Props<T>) { | ||
| const { name, control, dp = 6, className, rules, ...attrs } = props | ||
| const autoFocusRef = useAutoFocus() | ||
| const [displayValue, setDisplayValue] = useState("") |
There was a problem hiding this comment.
State desync risk with external form updates and falsy value handling.
Two related issues with the local displayValue state approach:
-
Falsy empty string:
displayValue || field.valueuses the||operator, which falls back tofield.valuewhendisplayValueis""(empty string is falsy). Combined with the Number comparison treating""as"0", users cannot clear an input that contains"0"— the input will keep showing"0". -
External updates ignored: If
field.valueis set programmatically (viaform.setValue(),form.reset(), etc.),displayValuewon't update, causing the input to show stale data.
🔧 Suggested fix using nullish coalescing and sync effect
function NumericInput<T extends FieldValues>(props: Props<T>) {
const { name, control, dp = 6, className, rules, ...attrs } = props
const autoFocusRef = useAutoFocus()
- const [displayValue, setDisplayValue] = useState("")
+ const [displayValue, setDisplayValue] = useState<string | undefined>(undefined)
return (
<Controller
name={name}
control={control}
rules={rules}
render={({ field }) => {
+ // Sync displayValue when field.value changes externally
+ const fieldValue = field.value ?? ""
+ if (displayValue !== undefined && displayValue !== fieldValue && Number(displayValue) === Number(fieldValue)) {
+ // Values are numerically equal but string differs - keep displayValue
+ } else if (displayValue === undefined || (Number(displayValue || "0") !== Number(fieldValue || "0") && displayValue !== fieldValue)) {
+ setDisplayValue(undefined)
+ }
+
const handleChange = (newValue: string) => {
const sanitized = sanitizeNumericInput(newValue, dp)
setDisplayValue(sanitized)
// ...
}
return (
<input
{...field}
- value={displayValue || field.value}
+ value={displayValue ?? field.value}Using ?? (nullish coalescing) instead of || ensures empty string "" is respected. Consider a more robust sync strategy for external updates.
Also applies to: 54-54
🤖 Prompt for AI Agents
In @packages/interwovenkit-react/src/components/form/NumericInput.tsx at line
30, The local displayValue state (const [displayValue, setDisplayValue]) causes
two issues: empty string is treated as falsy by the expression using || and
external updates to field.value are ignored; to fix, change any fallback from
displayValue || field.value to use nullish coalescing (displayValue ??
field.value) so "" is preserved, and add a useEffect that watches field.value
(or the form value) to call setDisplayValue when field.value changes so the
input stays in sync with programmatic updates; update any related logic that
compares Number(displayValue) so it handles "" explicitly if needed.
| // Only update form if numeric value changed | ||
| const currentNumeric = field.value === "" ? "0" : field.value | ||
| const newNumeric = sanitized === "" ? "0" : sanitized | ||
|
|
||
| if (Number(currentNumeric) !== Number(newNumeric)) { | ||
| field.onChange(sanitized) | ||
| }} | ||
| placeholder="0" | ||
| inputMode="decimal" | ||
| autoComplete="off" | ||
| {...attrs} | ||
| ref={mergeRefs([field.ref, autoFocusRef])} | ||
| /> | ||
| )} | ||
| } |
There was a problem hiding this comment.
Numeric equivalence check prevents clearing to empty when value is "0".
The logic treats empty string as "0" for comparison purposes:
const currentNumeric = field.value === "" ? "0" : field.value
const newNumeric = sanitized === "" ? "0" : sanitizedIf field.value is "0" and the user clears the input, Number("0") === Number("0") is true, so field.onChange is never called. Combined with the || fallback on line 54, the input will refuse to clear.
🔧 Suggested fix to distinguish empty from zero
const handleChange = (newValue: string) => {
const sanitized = sanitizeNumericInput(newValue, dp)
setDisplayValue(sanitized)
- // Only update form if numeric value changed
- const currentNumeric = field.value === "" ? "0" : field.value
- const newNumeric = sanitized === "" ? "0" : sanitized
-
- if (Number(currentNumeric) !== Number(newNumeric)) {
+ // Update form if string value changed (preserves "" vs "0" distinction)
+ if (field.value !== sanitized) {
field.onChange(sanitized)
}
}If the intent is to prevent unnecessary updates for numerically equivalent values (e.g., "1" vs "1.0"), consider a more nuanced check that still allows clearing.
🤖 Prompt for AI Agents
In @packages/interwovenkit-react/src/components/form/NumericInput.tsx around
lines 42 - 48, The numeric equivalence check in NumericInput (variables
currentNumeric/newNumeric) treats empty string as "0", which blocks clearing "0"
to empty; update the condition so you still call field.onChange(sanitized) when
sanitized === "" (or when field.value === "" differs), and only perform the
Number(...) equality shortcut when both field.value and sanitized are non-empty;
locate the comparison around the if (Number(currentNumeric) !==
Number(newNumeric)) and adjust it to allow cleared values while keeping
numeric-equality suppression for genuinely equivalent numeric inputs before
calling field.onChange(sanitized).
| const formattedNumberRegex = /^[0-9,]+(\.[0-9]+)?$/ | ||
| if (!formattedNumberRegex.test(pastedText)) return |
There was a problem hiding this comment.
Paste regex rejects valid decimal inputs like ".5".
The regex /^[0-9,]+(\.[0-9]+)?$/ requires at least one digit or comma before an optional decimal portion. This means pasting ".5" (a valid number that the normal input path accepts via sanitizeNumericInput) will be rejected.
🔧 Suggested fix to allow leading decimal
- const formattedNumberRegex = /^[0-9,]+(\.[0-9]+)?$/
+ const formattedNumberRegex = /^[0-9,]*\.?[0-9]+$/Or match the behavior more closely with what sanitizeNumericInput accepts.
📝 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.
| const formattedNumberRegex = /^[0-9,]+(\.[0-9]+)?$/ | |
| if (!formattedNumberRegex.test(pastedText)) return | |
| const formattedNumberRegex = /^[0-9,]*\.?[0-9]+$/ | |
| if (!formattedNumberRegex.test(pastedText)) return |
🤖 Prompt for AI Agents
In @packages/interwovenkit-react/src/components/form/NumericInput.tsx around
lines 62 - 63, The paste validation regex in NumericInput.tsx
(formattedNumberRegex) rejects inputs like ".5"; update the check so it allows a
leading decimal by replacing the current regex with one that permits zero digits
before the decimal and requires digits after it (for example
/^[0-9,]*\.?[0-9]+$/), or alternatively call the existing sanitizeNumericInput
logic against pastedText instead of using formattedNumberRegex; ensure the same
symbols are updated (formattedNumberRegex and the if
(!formattedNumberRegex.test(pastedText)) return) so ".5" is accepted.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/interwovenkit-react/src/pages/deposit/DepositFields.tsx (1)
54-63: Stale comment in dependency array.The comment on line 62 mentions "Destructure error fields in deps to properly track each field change" but there are no error fields being destructured or used. This appears to be a leftover from copy-paste or incomplete refactoring.
Suggested fix
const disabledMessage = useMemo(() => { if (!externalAsset) return "Select asset" if (!Number(quantity)) return "Enter amount" if ( Number(quantity) > Number(formatAmount(balance, { decimals: externalAsset?.decimals || 6 })) ) return "Insufficient balance" - - // Destructure error fields in deps to properly track each field change }, [quantity, balance, externalAsset])packages/interwovenkit-react/src/pages/deposit/WithdrawFields.tsx (1)
139-164: ExtractfindChain(dstChainId)to a variable to avoid repeated lookups.
findChain(dstChainId)is called 3 times during render (lines 144, 145, 160). InDepositFields.tsx, this is correctly extracted tosrcChain. Apply the same pattern here for consistency and minor performance improvement.Suggested fix
Add near line 46:
const localAsset = useLocalAssetDepositAsset() const externalAsset = useExternalDepositAsset() + const dstChain = dstChainId ? findChain(dstChainId) : nullThen update the JSX:
- src={findChain(dstChainId)?.logo_uri || ""} - alt={findChain(dstChainId)?.pretty_name} + src={dstChain?.logo_uri || ""} + alt={dstChain?.pretty_name} ... - <span>on {findChain(dstChainId)?.pretty_name}</span> + <span>on {dstChain?.pretty_name}</span>
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (2)
packages/interwovenkit-react/src/pages/deposit/DepositFields.tsxpackages/interwovenkit-react/src/pages/deposit/WithdrawFields.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
packages/interwovenkit-react/src/pages/deposit/DepositFields.tsx (4)
packages/interwovenkit-react/src/pages/bridge/data/simulate.ts (2)
RouterRouteResponseJson(14-19)useRouteQuery(21-74)packages/interwovenkit-react/src/pages/deposit/hooks.ts (4)
useAllBalancesQuery(17-46)useTransferForm(137-139)useLocalAssetDepositAsset(56-69)useExternalDepositAsset(71-84)packages/interwovenkit-react/src/pages/bridge/data/chains.ts (1)
useFindSkipChain(56-78)packages/interwovenkit-react/src/public/data/hooks.ts (1)
useHexAddress(23-29)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cloudflare Pages: interwovenkit-testnet
🔇 Additional comments (2)
packages/interwovenkit-react/src/pages/deposit/DepositFields.tsx (1)
32-198: LGTM!The component correctly orchestrates the deposit flow with proper hooks integration, debounced quantity for route queries, and appropriate UI state handling. The render props pattern for the footer aligns with existing bridge patterns.
packages/interwovenkit-react/src/pages/deposit/WithdrawFields.tsx (1)
32-197: LGTM!The withdraw flow component mirrors the deposit flow appropriately, with correct adjustments for source/destination asset handling. The form validation and route query integration work correctly.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/interwovenkit-react/src/components/form/NumericInput.tsx (2)
38-45: Consider handling intermediate invalid states like a lone decimal point.The comparison
Number(field.value || 0) !== Number(sanitized || 0)will always evaluate totruewhensanitizedis"."becauseNumber(".")returnsNaN, andNaN !== xis always true. This causesfield.onChangeto fire with"."as the value.This may trigger validation errors during typing. If you want to suppress onChange for incomplete numeric inputs, consider adding a guard:
Suggested approach
const handleChange = (newValue: string) => { const sanitized = sanitizeNumericInput(newValue, dp) setDisplayValue(sanitized) + // Skip onChange for incomplete inputs that aren't valid numbers + if (sanitized === "." || sanitized === "") { + if (sanitized === "" && field.value) field.onChange("") + return + } + if (Number(field.value || 0) !== Number(sanitized || 0)) { field.onChange(sanitized) } }
58-68: Paste validation rejects valid decimal inputs like.5.The regex
/^[0-9,]+(\.[0-9]+)?$/requires at least one digit or comma before the decimal point. Pasting.5or.123will be silently rejected, though these are valid numeric inputs thatsanitizeNumericInputwould otherwise handle correctly.If this is intentional to enforce formatted numbers only, consider adding a comment. Otherwise:
Suggested fix to allow leading decimals
- const formattedNumberRegex = /^[0-9,]+(\.[0-9]+)?$/ + // Allow formatted numbers (1,234.56) and leading decimals (.5) + const formattedNumberRegex = /^[0-9,]*\.?[0-9]*$/ if (!formattedNumberRegex.test(pastedText)) return + if (pastedText === "" || pastedText === ".") return
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (1)
packages/interwovenkit-react/src/components/form/NumericInput.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cloudflare Pages: interwovenkit-testnet
🔇 Additional comments (4)
packages/interwovenkit-react/src/components/form/NumericInput.tsx (4)
11-15: LGTM!The sanitization logic correctly handles multiple decimal points and truncates decimals to the specified precision.
30-30: LGTM!Local display state correctly separates the visual representation from the form value, enabling intermediate typing states.
47-50: LGTM!The
parsedDisplayValuelogic correctly preserves user formatting (like trailing decimals or zeros) while respecting external form value updates.
69-74: LGTM!The prop spread order correctly allows callers to override defaults, and ref merging with
mergeRefsproperly combines the field ref with the auto-focus ref.
Summary by CodeRabbit
New Features
UI/UX Improvements
✏️ Tip: You can customize this high-level summary in your review settings.