-
Notifications
You must be signed in to change notification settings - Fork 567
Dashboard: Add fallback to native token balance on contract/split page when insight is not supported #7847
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
WalkthroughAdds a try/catch fallback in the split token-balance hook to return native balance on error; updates ContractSplitPage to consume full recipients query object and refactors balances/recipients UI/markup; adjusts DistributeButton sizing and transaction count behavior. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as ContractSplitPage
participant Hook as useSplit / getTokenBalancesQuery
participant TokenSvc as getOwnedTokens
participant Wallet as getWalletBalance
UI->>Hook: request token balances
Hook->>TokenSvc: getOwnedTokens()
alt token fetch succeeds
TokenSvc-->>Hook: tokens[]
Hook-->>UI: tokens[]
else token fetch fails
TokenSvc-->>Hook: error
Hook->>Wallet: getWalletBalance()
Wallet-->>Hook: nativeBalance
Hook-->>UI: [GetBalanceResult(native token)]
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 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). (7)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7847 +/- ##
=======================================
Coverage 56.34% 56.34%
=======================================
Files 905 905
Lines 58834 58834
Branches 4150 4150
=======================================
Hits 33151 33151
Misses 25577 25577
Partials 106 106
🚀 New features to boost your workflow:
|
size-limit report 📦
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🔭 Outside diff range comments (3)
apps/dashboard/src/@/hooks/useSplit.ts (3)
101-106
: Use tokenAddress to detect native token (name is unreliable)The fallback uses walletBalance.name (e.g., “Ether”), so name === "Native Token" will misclassify the native balance path and call distributeByToken incorrectly. Compare tokenAddress to NATIVE_TOKEN_ADDRESS instead.
Apply this diff:
- const transaction = - currency.name === "Native Token" - ? distribute({ contract }) - : distributeByToken({ - contract, - tokenAddress: currency.tokenAddress, - }); + const transaction = + currency.tokenAddress === NATIVE_TOKEN_ADDRESS + ? distribute({ contract }) + : distributeByToken({ + contract, + tokenAddress: currency.tokenAddress, + });
97-121
: Mutation resolves early; return the transaction promisesThe map callback doesn’t return the promise, so Promise.all resolves immediately, marking the mutation done before transactions finish. Return the promise to ensure correct lifecycle and toasts timing.
Apply this diff:
- const distributions = balances - .filter((token) => token.value !== 0n) - .map(async (currency) => { + const distributions = balances + .filter((token) => token.value !== 0n) + .map((currency) => { const transaction = - currency.name === "Native Token" + currency.tokenAddress === NATIVE_TOKEN_ADDRESS ? distribute({ contract }) : distributeByToken({ contract, tokenAddress: currency.tokenAddress, }); const promise = sendAndConfirmTransaction({ account, transaction, }); toast.promise(promise, { error: (err) => ({ message: `Error distributing ${currency.name}`, description: parseError(err), }), loading: `Distributing ${currency.name}`, success: `Successfully distributed ${currency.name}`, }); + return promise; });
124-125
: Fix invalidateQueries usage with queryKeyinvalidateQueries expects a queryKey/filter, not a full queryOptions object.
Apply this diff:
- queryClient.invalidateQueries(getTokenBalancesQuery(params)); + queryClient.invalidateQueries({ + queryKey: getTokenBalancesQuery(params).queryKey, + });
🧹 Nitpick comments (6)
apps/dashboard/src/app/(app)/(dashboard)/(chain)/[chain_id]/[contractAddress]/split/components/distribute-button.tsx (3)
28-39
: Simplify numTransactions logic (remove redundancy)The early “Native Token” check and second zero-filter are redundant given validBalances is already filtered.
Apply this diff to simplify:
- const numTransactions = useMemo(() => { - if ( - validBalances.length === 1 && - validBalances[0]?.name === "Native Token" - ) { - return 1; - } - if (!validBalances || balancesIsPending) { - return 0; - } - return validBalances?.filter((b) => b.value !== 0n).length; - }, [validBalances, balancesIsPending]); + const numTransactions = useMemo(() => { + if (balancesIsPending) { + return 0; + } + return validBalances.length; + }, [validBalances, balancesIsPending]);
14-26
: Add explicit return type to component per guidelinesDeclare a JSX return type for the component.
Apply this diff:
-export const DistributeButton = ({ +export const DistributeButton = ({ contract, balances, balancesIsPending, balancesIsError, isLoggedIn, }: { contract: ThirdwebContract; balances: GetBalanceResult[]; balancesIsPending: boolean; balancesIsError: boolean; isLoggedIn: boolean; -}) => { +}): JSX.Element => {
43-55
: Type the helper function returnAnnotate distributeFunds to return void for clarity.
Apply this diff:
- const distributeFunds = () => { + const distributeFunds = (): void => {apps/dashboard/src/@/hooks/useSplit.ts (2)
29-68
: Set a sensible staleTime for balances to reduce refetch churnPer guidelines, configure staleTime/cacheTime. Recommend staleTime ≥ 60s for balances.
Apply this diff:
return queryOptions({ queryFn: async () => { const ownedTokenBalancePromise = getOwnedTokens({ client: params.client, chains: [params.chain], ownerAddress: params.ownerAddress, queryOptions: { include_native: "true", }, }); const result = await tryCatch(ownedTokenBalancePromise); // fallback to fetch native token balance with rpc if (result.error) { const walletBalance = await getWalletBalance({ address: params.ownerAddress, client: params.client, chain: params.chain, }); const nativeTokenBalance: GetBalanceResult = { name: walletBalance.name, value: walletBalance.value, decimals: walletBalance.decimals, displayValue: walletBalance.displayValue, symbol: walletBalance.symbol, chainId: params.chain.id, tokenAddress: NATIVE_TOKEN_ADDRESS, }; return [nativeTokenBalance]; } return result.data; }, queryKey: ["getOwnedTokens", params.chain.id, params.ownerAddress], retry: false, + staleTime: 60_000, });
29-31
: Optional: Add explicit generics to queryOptions for clearer typingThis clarifies the intended data shape.
Apply this diff:
- return queryOptions({ + return queryOptions<GetBalanceResult[]>({apps/dashboard/src/app/(app)/(dashboard)/(chain)/[chain_id]/[contractAddress]/split/ContractSplitPage.tsx (1)
23-29
: Add explicit return type to component per guidelinesDeclare the JSX return type on the page component.
Apply this diff:
-export function ContractSplitPage({ +export function ContractSplitPage({ contract, isLoggedIn, }: { contract: ThirdwebContract; isLoggedIn: boolean; -}) { +}): JSX.Element {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/dashboard/src/@/hooks/useSplit.ts
(2 hunks)apps/dashboard/src/app/(app)/(dashboard)/(chain)/[chain_id]/[contractAddress]/split/ContractSplitPage.tsx
(2 hunks)apps/dashboard/src/app/(app)/(dashboard)/(chain)/[chain_id]/[contractAddress]/split/components/distribute-button.tsx
(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.{ts,tsx}
: Write idiomatic TypeScript with explicit function declarations and return types
Limit each file to one stateless, single-responsibility function for clarity
Re-use shared types from@/types
or localtypes.ts
barrels
Prefer type aliases over interface except for nominal shapes
Avoidany
andunknown
unless unavoidable; narrow generics when possible
Choose composition over inheritance; leverage utility types (Partial
,Pick
, etc.)
Comment only ambiguous logic; avoid restating TypeScript in prose
Files:
apps/dashboard/src/app/(app)/(dashboard)/(chain)/[chain_id]/[contractAddress]/split/components/distribute-button.tsx
apps/dashboard/src/@/hooks/useSplit.ts
apps/dashboard/src/app/(app)/(dashboard)/(chain)/[chain_id]/[contractAddress]/split/ContractSplitPage.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Load heavy dependencies inside async paths to keep initial bundle lean (lazy loading)
Files:
apps/dashboard/src/app/(app)/(dashboard)/(chain)/[chain_id]/[contractAddress]/split/components/distribute-button.tsx
apps/dashboard/src/@/hooks/useSplit.ts
apps/dashboard/src/app/(app)/(dashboard)/(chain)/[chain_id]/[contractAddress]/split/ContractSplitPage.tsx
apps/{dashboard,playground-web}/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
apps/{dashboard,playground-web}/**/*.{ts,tsx}
: Import UI primitives from@/components/ui/*
(Button, Input, Select, Tabs, Card, Sidebar, Badge, Separator) in dashboard and playground apps
UseNavLink
for internal navigation with automatic active states in dashboard and playground apps
Use Tailwind CSS only – no inline styles or CSS modules
Usecn()
from@/lib/utils
for conditional class logic
Use design system tokens (e.g.,bg-card
,border-border
,text-muted-foreground
)
Server Components (Node edge): Start files withimport "server-only";
Client Components (browser): Begin files with'use client';
Always callgetAuthToken()
to retrieve JWT from cookies on server side
UseAuthorization: Bearer
header – never embed tokens in URLs
Return typed results (e.g.,Project[]
,User[]
) – avoidany
Wrap client-side data fetching calls in React Query (@tanstack/react-query
)
Use descriptive, stablequeryKeys
for React Query cache hits
ConfigurestaleTime
/cacheTime
in React Query based on freshness (default ≥ 60s)
Keep tokens secret via internal API routes or server actions
Never importposthog-js
in server components
Files:
apps/dashboard/src/app/(app)/(dashboard)/(chain)/[chain_id]/[contractAddress]/split/components/distribute-button.tsx
apps/dashboard/src/@/hooks/useSplit.ts
apps/dashboard/src/app/(app)/(dashboard)/(chain)/[chain_id]/[contractAddress]/split/ContractSplitPage.tsx
🧠 Learnings (3)
📚 Learning: 2025-05-29T00:46:09.063Z
Learnt from: jnsdls
PR: thirdweb-dev/js#7188
File: apps/dashboard/src/app/(app)/(dashboard)/(chain)/[chain_id]/[contractAddress]/accounts/components/accounts-count.tsx:15-15
Timestamp: 2025-05-29T00:46:09.063Z
Learning: In the accounts component at apps/dashboard/src/app/(app)/(dashboard)/(chain)/[chain_id]/[contractAddress]/accounts/components/accounts-count.tsx, the 3-column grid layout (md:grid-cols-3) is intentionally maintained even when rendering only one StatCard, as part of the design structure for this component.
Applied to files:
apps/dashboard/src/app/(app)/(dashboard)/(chain)/[chain_id]/[contractAddress]/split/components/distribute-button.tsx
apps/dashboard/src/app/(app)/(dashboard)/(chain)/[chain_id]/[contractAddress]/split/ContractSplitPage.tsx
📚 Learning: 2025-07-18T19:20:32.530Z
Learnt from: CR
PR: thirdweb-dev/js#0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-07-18T19:20:32.530Z
Learning: Applies to dashboard/**/*client.tsx : Interactive UI that relies on hooks (`useState`, `useEffect`, React Query, wallet hooks).
Applied to files:
apps/dashboard/src/app/(app)/(dashboard)/(chain)/[chain_id]/[contractAddress]/split/components/distribute-button.tsx
apps/dashboard/src/app/(app)/(dashboard)/(chain)/[chain_id]/[contractAddress]/split/ContractSplitPage.tsx
📚 Learning: 2025-07-18T19:20:32.530Z
Learnt from: CR
PR: thirdweb-dev/js#0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-07-18T19:20:32.530Z
Learning: Applies to dashboard/**/*.{tsx,jsx} : Prefer composable primitives over custom markup: `Button`, `Input`, `Select`, `Tabs`, `Card`, `Sidebar`, `Separator`, `Badge`.
Applied to files:
apps/dashboard/src/app/(app)/(dashboard)/(chain)/[chain_id]/[contractAddress]/split/components/distribute-button.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). (7)
- GitHub Check: E2E Tests (pnpm, vite)
- GitHub Check: E2E Tests (pnpm, webpack)
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: Size
- GitHub Check: Lint Packages
- GitHub Check: Unit Tests
- GitHub Check: Analyze (javascript)
🔇 Additional comments (8)
apps/dashboard/src/app/(app)/(dashboard)/(chain)/[chain_id]/[contractAddress]/split/components/distribute-button.tsx (3)
64-64
: Button sizing in error state aligns with UI updatesAdding size="sm" keeps the error-state TransactionButton visually consistent with the rest of the page.
91-91
: Hide transactionCount when it equals 1This improves UX by avoiding redundant “1 tx” labels while still showing the badge for multi-tx cases.
57-72
: Potentially unreachable error branch due to parent gatingParent only renders DistributeButton when balanceQuery.data?.length > 0, so this balancesIsError branch likely won’t render when the query errors (unless stale data exists). Confirm intended behavior.
If you want to always allow “Distribute” even when the balance query errors, consider moving the error-state button rendering into the parent alongside the error UI.
apps/dashboard/src/@/hooks/useSplit.ts (1)
31-64
: Solid fallback to native balance when insights are unsupportedWrapping getOwnedTokens with tryCatch and falling back to getWalletBalance provides resilience and keeps the UI usable on chains without Insight support.
apps/dashboard/src/app/(app)/(dashboard)/(chain)/[chain_id]/[contractAddress]/split/ContractSplitPage.tsx (4)
32-35
: Good consolidation of recipients into a single query objectUsing allRecipientsPercentagesQuery directly reduces state duplication and clarifies loading/error states.
42-44
: Recipient detection is robustLowercasing both sides avoids checksum/format issues. LGTM.
137-147
: Consider rendering DistributeButton even when balances query errors (if intended)Currently, the button is only shown when data.length > 0, so the DistributeButton’s internal balancesIsError handling won’t surface. If you want to allow distribution attempts despite balance insight failures (given your new native fallback), loosen this gating.
If desired, I can propose a small layout tweak to always render the footer row and mount DistributeButton with error/pending states reflected by the query.
56-66
: Table container and headers look consistent with design tokensUse of bg-card, border, and responsive widths reads well and matches dashboard UI guidance.
{balanceQuery.isError && ( | ||
<div className="text-red-500 px-4 flex justify-center items-center py-20 text-muted-foreground"> | ||
{balanceQuery.error.message} | ||
</div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Conflicting text colors override error state
"text-red-500" is overridden by "text-muted-foreground" in the same className. Remove the muted class to ensure the error appears red.
Apply this diff:
- <div className="text-red-500 px-4 flex justify-center items-center py-20 text-muted-foreground">
+ <div className="text-red-500 px-4 flex justify-center items-center py-20">
{balanceQuery.error.message}
</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.
{balanceQuery.isError && ( | |
<div className="text-red-500 px-4 flex justify-center items-center py-20 text-muted-foreground"> | |
{balanceQuery.error.message} | |
</div> | |
{balanceQuery.isError && ( | |
<div className="text-red-500 px-4 flex justify-center items-center py-20"> | |
{balanceQuery.error.message} | |
</div> |
🤖 Prompt for AI Agents
In
apps/dashboard/src/app/(app)/(dashboard)/(chain)/[chain_id]/[contractAddress]/split/ContractSplitPage.tsx
around lines 124 to 127, the error div mixes "text-red-500" and
"text-muted-foreground" which conflicts and prevents the error text from
appearing red; remove "text-muted-foreground" from the className so the error
uses the red styling (leave the other layout classes intact).
Merge activity
|
…e when insight is not supported (#7847) <!-- ## title your PR with this format: "[SDK/Dashboard/Portal] Feature/Fix: Concise title for the changes" If you did not copy the branch name from Linear, paste the issue tag here (format is TEAM-0000): ## Notes for the reviewer Anything important to call out? Be sure to also clarify these in your comments. ## How to test Unit tests, playground, etc. --> <!-- start pr-codex --> --- ## PR-Codex overview This PR focuses on improving the handling of token balances and recipient percentages in the `ContractSplitPage` and related components, enhancing user experience and error handling. ### Detailed summary - Updated `transactionCount` logic in `DistributeButton` to handle undefined cases. - Added error handling for fetching token balances using `tryCatch`. - Improved UI structure and styling in `ContractSplitPage`. - Changed variable naming for clarity in `useReadContract`. - Enhanced loading states for recipient percentages and balances. > ✨ Ask PR-Codex anything about this PR by commenting with `/codex {your question}` <!-- end pr-codex --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved reliability of token balances by falling back to native balance if token data fails to load. * **Style** * Refreshed Split page layout: simplified headers, refined spacing, and updated table containers. * Adjusted column widths for Tokens and Addresses for better readability. * Enhanced loading skeletons and maintained clear empty/error states. * **Refactor** * Streamlined data flow for recipients and balances to use a unified query object. * **Chores** * Button tweaks: small size on error; hide transaction count when only one transaction. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
24e4802
to
74d4074
Compare
PR-Codex overview
This PR focuses on improving the
DistributeButton
component and enhancing theContractSplitPage
by refining the handling of transaction counts and token balances, as well as updating the UI for better clarity and usability.Detailed summary
distribute-button.tsx
, updatedtransactionCount
logic to handle undefined values.useSplit.ts
, added fallback logic for fetching wallet balance when token balance retrieval fails.ContractSplitPage.tsx
, improved variable naming and UI structure for better readability.Summary by CodeRabbit