DO NOT MERGE - Add Your tokens tab #8620
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThis PR refactors the Bridge/Swap widget UI by introducing tab-based navigation, renaming wallet connection components for clarity, and expanding token selection capabilities with new theming and wallet configuration options. Multiple components are restructured to support a multi-tab selection flow instead of single-action interfaces. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes 🚥 Pre-merge checks | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/thirdweb/src/react/web/ui/ConnectWallet/screens/WalletSwitcherConnectionScreen.tsx (1)
47-51: Potential inconsistency:useSetupScreenuses hardcoded"compact"instead ofprops.size.The
sizeprop is now configurable viaprops.size, butuseSetupScreenstill hardcodessize: "compact". This could cause layout inconsistencies whensize="wide"is passed to the component.Proposed fix
const screenSetup = useSetupScreen({ - size: "compact", + size: props.size, wallets: wallets, welcomeScreen: undefined, });
🤖 Fix all issues with AI agents
In @packages/thirdweb/src/react/web/ui/Bridge/swap-widget/select-token-ui.tsx:
- Line 150: The prop binding for showAllWallets currently uses a truthy-or
fallback (props.connectOptions?.showAllWallets || true) which forces true even
when showAllWallets is explicitly false; change the fallback to a
nullish-coalescing fallback (props.connectOptions?.showAllWallets ?? true) so
false is preserved and only undefined/null fall back to true, updating the
expression where showAllWallets is passed into the component.
In @packages/thirdweb/src/react/web/ui/Bridge/swap-widget/use-tokens.ts:
- Around line 80-104: The TODO in select-token-ui.tsx about multi-chainId
filtering must be resolved: either remove the broken multi-chainId feature or
make use-tokens.ts and the consumer compatible with the API. Update
use-tokens.ts (the query building in the hook that sets chainId params) to send
chain IDs in the format the API actually accepts (e.g., a single comma-separated
chainId query value instead of multiple chainId params) and adjust
select-token-ui.tsx to consume the new shape (and delete the `// TODO - this is
not working!!` comment). Ensure the unique symbols to edit are the hook that
builds the URL in use-tokens.ts (the code appending chainId params) and the
component logic in select-token-ui.tsx around the TODO, then test the flow
end-to-end to verify filtering works and remove the TODO before merging.
🧹 Nitpick comments (6)
packages/thirdweb/src/react/web/ui/ConnectWallet/icons/WalletDotIcon.tsx (1)
4-7: Add explicit return type to match TypeScript guidelines and sibling icon patterns.The function is missing an explicit return type annotation. While the suggested
React.ReactElementis valid, other icons in this directory use theIconFCtype alias for consistency. However, sinceWalletDotIconhas different props (withstyleinstead ofcolor), define a proper type alias or useReact.JSX.Elementfor clarity:Suggested approach
+import type { IconFC } from "./types.js"; + +type WalletDotIconProps = { + size?: string; + style?: React.CSSProperties; +}; + -export const WalletDotIcon = (props: { +export const WalletDotIcon = (props: WalletDotIconProps): React.JSX.Element => { - size?: string; - style?: React.CSSProperties; -}) => {Alternatively, inline the return type as
: React.JSX.Elementwithout extracting the props type, but ensure consistency with sibling icons where possible.packages/thirdweb/src/react/web/ui/Bridge/swap-widget/swap-ui.tsx (1)
254-303: LGTM!Consistent with the buy token modal, the sell token modal now also receives
themeandconnectOptionsprops. The prop ordering differs slightly from the buy modal (props appear beforeonClosevs aftertype), but this is a minor stylistic inconsistency.Consider aligning prop order for consistency
For maintainability, consider aligning the prop order between buy and sell token modals:
{modalState.screen === "select-sell-token" && ( <SelectToken + type="sell" theme={props.theme} connectOptions={props.connectOptions} + selections={{ + buyChainId: props.buyToken?.chainId, + sellChainId: props.sellToken?.chainId, + }} + activeWalletInfo={props.activeWalletInfo} onClose={() => { setModalState((v) => ({ ...v, isOpen: false, })); }} client={props.client} selectedToken={props.sellToken} currency={props.currency} setSelectedToken={(token) => { // ... }} - activeWalletInfo={props.activeWalletInfo} - type="sell" - selections={{ - buyChainId: props.buyToken?.chainId, - sellChainId: props.sellToken?.chainId, - }} /> )}packages/thirdweb/src/react/web/ui/ConnectWallet/screens/Details/WalletManagerScreen.tsx (1)
89-89: Typo in component name:WalletManangerButtonshould beWalletManagerButton.Minor typo in the internal component name.
Suggested fix
-function WalletManangerButton(props: { +function WalletManagerButton(props: {And update usage at line 89:
- <WalletManangerButton + <WalletManagerButtonpackages/thirdweb/src/react/web/ui/Bridge/swap-widget/select-chain.tsx (1)
173-177: Missing key attribute on Skeleton elements.The biome-ignore comment suppresses the lint warning, but using array index as key (or no key at all here) can cause rendering issues if the list changes. Consider using the array index explicitly since these are static placeholders.
Suggested fix
{props.isPending && - new Array(20).fill(0).map(() => ( - // biome-ignore lint/correctness/useJsxKeyInIterable: ok - <ChainButtonSkeleton /> + new Array(20).fill(0).map((_, i) => ( + // biome-ignore lint/suspicious/noArrayIndexKey: static placeholder list + <ChainButtonSkeleton key={i} /> ))}packages/thirdweb/src/react/web/ui/Bridge/swap-widget/SelectChainButton.tsx (1)
68-92: Consider extracting tab options to a constant for maintainability.The tab labels "Your Tokens" and "All Tokens" are hardcoded in both
TabsandYourTokensButton(in select-chain.tsx). Consider extracting to a shared constant for consistency.packages/thirdweb/src/react/web/ui/Bridge/swap-widget/select-token-ui.tsx (1)
934-1012: Consider extracting duplicate sorting logic into a helper function.The icon-based sorting logic (tokens with icons first) is duplicated in
ChainTokenSelectionScreen(lines 1001-1009, 1028-1036) andYourTokenSelectionScreen(lines 1277-1285). Consider extracting to a shared utility.Suggested helper
function sortByIconPresence<T extends { icon_uri?: string } | { iconUri?: string }>(tokens: T[]): T[] { return [...tokens].sort((a, b) => { const aHasIcon = 'icon_uri' in a ? a.icon_uri : a.iconUri; const bHasIcon = 'icon_uri' in b ? b.icon_uri : b.iconUri; if (aHasIcon && !bHasIcon) return -1; if (!aHasIcon && bHasIcon) return 1; return 0; }); }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (14)
packages/thirdweb/src/react/web/ui/Bridge/FundWallet.tsxpackages/thirdweb/src/react/web/ui/Bridge/payment-selection/PaymentSelection.tsxpackages/thirdweb/src/react/web/ui/Bridge/swap-widget/SelectChainButton.tsxpackages/thirdweb/src/react/web/ui/Bridge/swap-widget/select-chain.tsxpackages/thirdweb/src/react/web/ui/Bridge/swap-widget/select-token-ui.tsxpackages/thirdweb/src/react/web/ui/Bridge/swap-widget/swap-ui.tsxpackages/thirdweb/src/react/web/ui/Bridge/swap-widget/types.tspackages/thirdweb/src/react/web/ui/Bridge/swap-widget/use-tokens.tspackages/thirdweb/src/react/web/ui/ConnectWallet/icons/WalletDotIcon.tsxpackages/thirdweb/src/react/web/ui/ConnectWallet/screens/Details/WalletManagerScreen.tsxpackages/thirdweb/src/react/web/ui/ConnectWallet/screens/WalletSwitcherConnectionScreen.tsxpackages/thirdweb/src/react/web/ui/components/DynamicHeight.tsxpackages/thirdweb/src/react/web/ui/components/Modal.tsxpackages/thirdweb/src/stories/Bridge/Swap/SelectChain.stories.tsx
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Write idiomatic TypeScript with explicit function declarations and return types
Limit each TypeScript file to one stateless, single-responsibility function for clarity
Re-use shared types from@/typesor localtypes.tsbarrels
Prefer type aliases over interface except for nominal shapes in TypeScript
Avoidanyandunknownin TypeScript unless unavoidable; narrow generics when possible
Choose composition over inheritance; leverage utility types (Partial,Pick, etc.) in TypeScript
**/*.{ts,tsx}: Write idiomatic TypeScript with explicit function declarations and return types
Limit each file to one stateless, single-responsibility function for clarity and testability
Re-use shared types from @/types or local types.ts barrel exports
Prefer type aliases over interface except for nominal shapes
Avoid any and unknown unless unavoidable; narrow generics whenever possible
Choose composition over inheritance; leverage utility types (Partial, Pick, etc.)
Comment only ambiguous logic in TypeScript files; avoid restating TypeScript types and signatures in prose
Files:
packages/thirdweb/src/react/web/ui/components/DynamicHeight.tsxpackages/thirdweb/src/react/web/ui/Bridge/swap-widget/types.tspackages/thirdweb/src/react/web/ui/Bridge/swap-widget/swap-ui.tsxpackages/thirdweb/src/react/web/ui/Bridge/swap-widget/use-tokens.tspackages/thirdweb/src/react/web/ui/Bridge/swap-widget/select-chain.tsxpackages/thirdweb/src/react/web/ui/Bridge/FundWallet.tsxpackages/thirdweb/src/react/web/ui/ConnectWallet/icons/WalletDotIcon.tsxpackages/thirdweb/src/react/web/ui/Bridge/swap-widget/SelectChainButton.tsxpackages/thirdweb/src/react/web/ui/components/Modal.tsxpackages/thirdweb/src/react/web/ui/ConnectWallet/screens/WalletSwitcherConnectionScreen.tsxpackages/thirdweb/src/react/web/ui/Bridge/payment-selection/PaymentSelection.tsxpackages/thirdweb/src/stories/Bridge/Swap/SelectChain.stories.tsxpackages/thirdweb/src/react/web/ui/ConnectWallet/screens/Details/WalletManagerScreen.tsxpackages/thirdweb/src/react/web/ui/Bridge/swap-widget/select-token-ui.tsx
packages/thirdweb/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
packages/thirdweb/src/**/*.{ts,tsx}: Comment only ambiguous logic in SDK code; avoid restating TypeScript in prose
Load heavy dependencies inside async paths to keep initial bundle lean (e.g.const { jsPDF } = await import("jspdf");)Lazy-load heavy dependencies inside async paths to keep the initial bundle lean (e.g., const { jsPDF } = await import('jspdf');)
Files:
packages/thirdweb/src/react/web/ui/components/DynamicHeight.tsxpackages/thirdweb/src/react/web/ui/Bridge/swap-widget/types.tspackages/thirdweb/src/react/web/ui/Bridge/swap-widget/swap-ui.tsxpackages/thirdweb/src/react/web/ui/Bridge/swap-widget/use-tokens.tspackages/thirdweb/src/react/web/ui/Bridge/swap-widget/select-chain.tsxpackages/thirdweb/src/react/web/ui/Bridge/FundWallet.tsxpackages/thirdweb/src/react/web/ui/ConnectWallet/icons/WalletDotIcon.tsxpackages/thirdweb/src/react/web/ui/Bridge/swap-widget/SelectChainButton.tsxpackages/thirdweb/src/react/web/ui/components/Modal.tsxpackages/thirdweb/src/react/web/ui/ConnectWallet/screens/WalletSwitcherConnectionScreen.tsxpackages/thirdweb/src/react/web/ui/Bridge/payment-selection/PaymentSelection.tsxpackages/thirdweb/src/stories/Bridge/Swap/SelectChain.stories.tsxpackages/thirdweb/src/react/web/ui/ConnectWallet/screens/Details/WalletManagerScreen.tsxpackages/thirdweb/src/react/web/ui/Bridge/swap-widget/select-token-ui.tsx
**/*.{js,jsx,ts,tsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Biome governs formatting and linting; its rules live in biome.json. Run
pnpm fix&pnpm lintbefore committing, ensure there are no linting errors
Files:
packages/thirdweb/src/react/web/ui/components/DynamicHeight.tsxpackages/thirdweb/src/react/web/ui/Bridge/swap-widget/types.tspackages/thirdweb/src/react/web/ui/Bridge/swap-widget/swap-ui.tsxpackages/thirdweb/src/react/web/ui/Bridge/swap-widget/use-tokens.tspackages/thirdweb/src/react/web/ui/Bridge/swap-widget/select-chain.tsxpackages/thirdweb/src/react/web/ui/Bridge/FundWallet.tsxpackages/thirdweb/src/react/web/ui/ConnectWallet/icons/WalletDotIcon.tsxpackages/thirdweb/src/react/web/ui/Bridge/swap-widget/SelectChainButton.tsxpackages/thirdweb/src/react/web/ui/components/Modal.tsxpackages/thirdweb/src/react/web/ui/ConnectWallet/screens/WalletSwitcherConnectionScreen.tsxpackages/thirdweb/src/react/web/ui/Bridge/payment-selection/PaymentSelection.tsxpackages/thirdweb/src/stories/Bridge/Swap/SelectChain.stories.tsxpackages/thirdweb/src/react/web/ui/ConnectWallet/screens/Details/WalletManagerScreen.tsxpackages/thirdweb/src/react/web/ui/Bridge/swap-widget/select-token-ui.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Lazy-import optional features; avoid top-level side-effects
Files:
packages/thirdweb/src/react/web/ui/components/DynamicHeight.tsxpackages/thirdweb/src/react/web/ui/Bridge/swap-widget/types.tspackages/thirdweb/src/react/web/ui/Bridge/swap-widget/swap-ui.tsxpackages/thirdweb/src/react/web/ui/Bridge/swap-widget/use-tokens.tspackages/thirdweb/src/react/web/ui/Bridge/swap-widget/select-chain.tsxpackages/thirdweb/src/react/web/ui/Bridge/FundWallet.tsxpackages/thirdweb/src/react/web/ui/ConnectWallet/icons/WalletDotIcon.tsxpackages/thirdweb/src/react/web/ui/Bridge/swap-widget/SelectChainButton.tsxpackages/thirdweb/src/react/web/ui/components/Modal.tsxpackages/thirdweb/src/react/web/ui/ConnectWallet/screens/WalletSwitcherConnectionScreen.tsxpackages/thirdweb/src/react/web/ui/Bridge/payment-selection/PaymentSelection.tsxpackages/thirdweb/src/stories/Bridge/Swap/SelectChain.stories.tsxpackages/thirdweb/src/react/web/ui/ConnectWallet/screens/Details/WalletManagerScreen.tsxpackages/thirdweb/src/react/web/ui/Bridge/swap-widget/select-token-ui.tsx
**/*.stories.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
Add Storybook stories (
*.stories.tsx) alongside new UI components for documentation
Files:
packages/thirdweb/src/stories/Bridge/Swap/SelectChain.stories.tsx
**/*.stories.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
For new UI components, add Storybook stories (*.stories.tsx) alongside the code
Files:
packages/thirdweb/src/stories/Bridge/Swap/SelectChain.stories.tsx
🧬 Code graph analysis (8)
packages/thirdweb/src/react/web/ui/Bridge/swap-widget/types.ts (1)
packages/thirdweb/src/bridge/types/Chain.ts (1)
BridgeChain(42-42)
packages/thirdweb/src/react/web/ui/Bridge/swap-widget/use-tokens.ts (1)
packages/thirdweb/src/utils/domains.ts (1)
getThirdwebBaseUrl(103-109)
packages/thirdweb/src/react/web/ui/Bridge/swap-widget/select-chain.tsx (2)
packages/thirdweb/src/exports/thirdweb.ts (1)
ThirdwebClient(25-25)packages/thirdweb/src/react/web/ui/Bridge/swap-widget/types.ts (1)
SelectedTab(160-162)
packages/thirdweb/src/react/web/ui/Bridge/FundWallet.tsx (2)
packages/thirdweb/src/react/web/ui/ConnectWallet/icons/WalletDotIcon.tsx (1)
WalletDotIcon(4-24)packages/thirdweb/src/react/core/design-system/index.ts (1)
iconSize(166-176)
packages/thirdweb/src/react/web/ui/components/Modal.tsx (2)
packages/thirdweb/src/react/web/ui/ConnectWallet/constants.ts (1)
compactModalMaxHeight(15-15)packages/thirdweb/src/react/web/ui/components/DynamicHeight.tsx (1)
DynamicHeight(8-23)
packages/thirdweb/src/react/web/ui/Bridge/payment-selection/PaymentSelection.tsx (1)
packages/thirdweb/src/react/web/ui/ConnectWallet/screens/WalletSwitcherConnectionScreen.tsx (1)
WalletConnectionScreen(37-86)
packages/thirdweb/src/stories/Bridge/Swap/SelectChain.stories.tsx (2)
packages/thirdweb/src/react/web/ui/Bridge/swap-widget/types.ts (1)
SelectedTab(160-162)packages/thirdweb/src/react/web/ui/Bridge/swap-widget/SwapWidget.tsx (1)
SwapWidgetContainer(276-295)
packages/thirdweb/src/react/web/ui/ConnectWallet/screens/Details/WalletManagerScreen.tsx (1)
packages/thirdweb/src/react/web/ui/ConnectWallet/screens/WalletSwitcherConnectionScreen.tsx (2)
WalletConnectionScreenProps(14-35)WalletConnectionScreen(37-86)
⏰ 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). (8)
- GitHub Check: Unit Tests
- GitHub Check: Size
- GitHub Check: Lint Packages
- GitHub Check: E2E Tests (pnpm, webpack)
- GitHub Check: Build Packages
- GitHub Check: E2E Tests (pnpm, vite)
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (19)
packages/thirdweb/src/react/web/ui/components/DynamicHeight.tsx (1)
8-22: Clean simplification of the component's responsibility.The removal of the
maxHeightprop and the extra wrapper div is a good refactor. The component now has a single responsibility—observing and animating height transitions—while height constraints are handled by the parent container. This reduces coupling and improves maintainability.packages/thirdweb/src/react/web/ui/components/Modal.tsx (2)
102-105: Constraint moved appropriately to the container level.The
maxHeightconstraint is now correctly applied at the DialogContent level for compact modals. This works well with the updatedDynamicHeightcomponent, which handles smooth height transitions while the container enforces the height limit.
131-135: No issues found — DynamicHeight usage is consistent with the updated API.All usages of
DynamicHeightwithin the thirdweb package are already aligned with the updated signature that accepts onlychildren. The Modal.tsx component correctly uses the component without themaxHeightprop, consistent with all other usages in the codebase.packages/thirdweb/src/react/web/ui/ConnectWallet/icons/WalletDotIcon.tsx (1)
8-23: LGTM!The SVG implementation follows best practices:
- Uses
fill="currentColor"to inherit color from CSS, eliminating the need for a dedicated color prop- Properly includes
role="presentation"for accessibility- The
viewBoxprovides intrinsic sizing when explicit dimensions aren't providedpackages/thirdweb/src/react/web/ui/Bridge/swap-widget/types.ts (1)
159-162: LGTM!The
SelectedTabdiscriminated union type is well-structured with a cleartypediscriminator. This cleanly models the tab-based navigation pattern introduced by this PR.packages/thirdweb/src/react/web/ui/Bridge/FundWallet.tsx (2)
214-231: LGTM!The
themeandconnectOptionsprops are correctly passed toSelectToken, aligning with the expanded component API. This enables themed styling and wallet-connection configuration within the token selection modal.
650-650: Verify the removal of thecolorprop fromWalletDotIcon.The
WalletDotIconcomponent no longer receives acolorprop. Based on the component's implementation which usesfill="currentColor", the icon will inherit color from the parent's CSScolorproperty. Ensure this is the intended behavior and that the parent container (Containerwithcolor="secondaryText") provides the correct color styling.packages/thirdweb/src/react/web/ui/Bridge/swap-widget/swap-ui.tsx (1)
207-252: LGTM!The
themeandconnectOptionsprops are correctly passed to theSelectTokencomponent for the buy token modal, enabling themed styling and wallet-connection configuration.packages/thirdweb/src/react/web/ui/Bridge/payment-selection/PaymentSelection.tsx (2)
16-16: LGTM!The import is correctly updated to use the renamed
WalletConnectionScreencomponent from the same module path.
241-264: LGTM!The component usage is correctly updated with the new required props:
shouldSetActive={false}- appropriate for payment selection where the connected wallet shouldn't automatically become the active walletsize="compact"- matches the modal size contextThe remaining props are passed through correctly.
packages/thirdweb/src/react/web/ui/ConnectWallet/screens/WalletSwitcherConnectionScreen.tsx (2)
14-35: LGTM!The type rename from
WalletSwitcherConnectionScreenPropstoWalletConnectionScreenPropsand the addition ofsizeandshouldSetActiveprops make the API more flexible and align with the broader wallet connection flow refactoring.
77-79: LGTM!The new props
shouldSetActiveandsizeare correctly passed through toConnectModalContent, completing the API extension.packages/thirdweb/src/stories/Bridge/Swap/SelectChain.stories.tsx (1)
19-39: LGTM - Stories correctly updated for tab-based API.The stories properly demonstrate the new
SelectedTabtype andonSelectTab/selectedTabprops. The initial state{ type: "your-tokens" }aligns with the type definition.packages/thirdweb/src/react/web/ui/ConnectWallet/screens/Details/WalletManagerScreen.tsx (1)
54-66: LGTM - WalletConnectionScreen integration is correct.The explicit
shouldSetActive={false}andsize="compact"props are appropriately omitted from the type and passed directly. The manualsetActive(w)call inonSelectensures proper wallet activation flow.packages/thirdweb/src/react/web/ui/Bridge/swap-widget/select-chain.tsx (1)
268-304: LGTM - YourTokensButton component is well-structured.The component correctly mirrors the ChainButton styling pattern and properly displays the WalletDotIcon with optical alignment adjustment.
packages/thirdweb/src/react/web/ui/Bridge/swap-widget/SelectChainButton.tsx (1)
17-66: LGTM - MobileTabSelector correctly handles tab states.The component properly branches rendering based on
selectedTab.typeand theonSelectcallback uses appropriate string literals for navigation actions. The "all-tokens" → "chain" semantic mapping in the Tabs component is correctly handled.packages/thirdweb/src/react/web/ui/Bridge/swap-widget/select-token-ui.tsx (3)
448-620: LGTM - YourTokenButton component with chain icon overlay.The component correctly displays token info with a chain icon overlay for multi-chain context. The positioning and styling are well-implemented.
1404-1424: LGTM - ScreenHeader component is a clean extraction.The component provides consistent header styling across screens. The conditional description rendering is correctly implemented.
1239-1245: Remove outdated TODO comment — chainId array parameter is properly supported.The
useTokenBalanceshook already supports array chainIds. The type signature declareschainId: number | number[] | undefined(use-tokens.ts:80), and the implementation correctly handles arrays by iterating and appending each chainId to the URL search parameters (lines 97–100). The code at line 1244 is functioning as intended.Likely an incorrect or invalid review comment.
| appMetadata={props.connectOptions?.appMetadata} | ||
| connectLocale={connectLocaleEn} | ||
| recommendedWallets={props.connectOptions?.recommendedWallets} | ||
| showAllWallets={props.connectOptions?.showAllWallets || true} |
There was a problem hiding this comment.
Bug: || true always evaluates to true, use ?? true instead.
The expression props.connectOptions?.showAllWallets || true will always be true because || performs a truthy check. If showAllWallets is explicitly false, it will still evaluate to true.
Proposed fix
- showAllWallets={props.connectOptions?.showAllWallets || true}
+ showAllWallets={props.connectOptions?.showAllWallets ?? true}📝 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.
| showAllWallets={props.connectOptions?.showAllWallets || true} | |
| showAllWallets={props.connectOptions?.showAllWallets ?? true} |
🤖 Prompt for AI Agents
In @packages/thirdweb/src/react/web/ui/Bridge/swap-widget/select-token-ui.tsx at
line 150, The prop binding for showAllWallets currently uses a truthy-or
fallback (props.connectOptions?.showAllWallets || true) which forces true even
when showAllWallets is explicitly false; change the fallback to a
nullish-coalescing fallback (props.connectOptions?.showAllWallets ?? true) so
false is preserved and only undefined/null fall back to true, updating the
expression where showAllWallets is passed into the component.
| chainId: number | number[] | undefined; | ||
| }) { | ||
| return useQuery({ | ||
| queryKey: ["bridge/v1/wallets", options], | ||
| enabled: !!options.chainId && !!options.walletAddress, | ||
| enabled: !!options.walletAddress, | ||
| queryFn: async () => { | ||
| if (!options.chainId || !options.walletAddress) { | ||
| if (!options.walletAddress) { | ||
| throw new Error("invalid options"); | ||
| } | ||
| const baseUrl = getThirdwebBaseUrl("bridge"); | ||
| const isDev = baseUrl.includes("thirdweb-dev"); | ||
| const url = new URL( | ||
| `https://api.${isDev ? "thirdweb-dev" : "thirdweb"}.com/v1/wallets/${options.walletAddress}/tokens`, | ||
| ); | ||
| url.searchParams.set("chainId", options.chainId.toString()); | ||
|
|
||
| // Set chainId param(s) if provided | ||
| if (options.chainId !== undefined) { | ||
| if (Array.isArray(options.chainId)) { | ||
| for (const id of options.chainId) { | ||
| url.searchParams.append("chainId", id.toString()); | ||
| } | ||
| } else { | ||
| url.searchParams.set("chainId", options.chainId.toString()); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n packages/thirdweb/src/react/web/ui/Bridge/swap-widget/use-tokens.ts | head -120Repository: thirdweb-dev/js
Length of output: 4506
🏁 Script executed:
rg "bridge/v1/wallets" -A 5 -B 5 packages/thirdweb/src/Repository: thirdweb-dev/js
Length of output: 2283
🏁 Script executed:
grep -r "chainId" packages/thirdweb/src/react/web/ui/Bridge/swap-widget/use-tokens.ts -B 2 -A 2Repository: thirdweb-dev/js
Length of output: 1022
🏁 Script executed:
rg "useTokenBalances" -A 3 -B 3 packages/thirdweb/src/react/web/ui/Bridge/ | head -100Repository: thirdweb-dev/js
Length of output: 2718
🏁 Script executed:
fd "use-tokens.ts" -x wc -l {}Repository: thirdweb-dev/js
Length of output: 132
🏁 Script executed:
cat -n packages/thirdweb/src/react/web/ui/Bridge/swap-widget/use-tokens.ts | sed -n '75,130p'Repository: thirdweb-dev/js
Length of output: 2396
🏁 Script executed:
cat -n packages/thirdweb/src/react/web/ui/Bridge/swap-widget/select-token-ui.tsx | grep -A 10 -B 2 "useTokenBalances"Repository: thirdweb-dev/js
Length of output: 1283
🏁 Script executed:
cat -n packages/thirdweb/src/react/web/ui/Bridge/swap-widget/use-tokens.ts | head -80Repository: thirdweb-dev/js
Length of output: 2690
🏁 Script executed:
ast-grep --pattern 'export function useTokenBalances' packages/thirdweb/src/react/web/ui/Bridge/swap-widget/use-tokens.tsRepository: thirdweb-dev/js
Length of output: 41
🏁 Script executed:
rg "TODO - this is not working" packages/thirdweb/src/react/web/ui/Bridge/ -B 5 -A 5Repository: thirdweb-dev/js
Length of output: 1256
🏁 Script executed:
rg "chainId.*map" packages/thirdweb/src/react/web/ui/Bridge/swap-widget/ -B 3 -A 3Repository: thirdweb-dev/js
Length of output: 802
🏁 Script executed:
git log -p --all -S "TODO - this is not working" -- packages/thirdweb/src/react/web/ui/Bridge/swap-widget/select-token-ui.tsx | head -100Repository: thirdweb-dev/js
Length of output: 41
🏁 Script executed:
rg "allTokensQuery" packages/thirdweb/src/react/web/ui/Bridge/swap-widget/select-token-ui.tsx -A 10 -B 2Repository: thirdweb-dev/js
Length of output: 3219
🏁 Script executed:
cat -n packages/thirdweb/src/react/web/ui/Bridge/swap-widget/select-token-ui.tsx | sed -n '1239,1280p'Repository: thirdweb-dev/js
Length of output: 1626
🏁 Script executed:
rg "v1/wallets.*tokens" packages/thirdweb/ -B 3 -A 3 | head -50Repository: thirdweb-dev/js
Length of output: 828
Remove or resolve the TODO comment in select-token-ui.tsx before merging.
The implementation in useTokenBalances appears syntactically correct for handling array chainIds (lines 96–104), but the consuming code in select-token-ui.tsx:1244 has an explicit TODO comment: // TODO - this is not working!! on the exact feature being reviewed. This indicates the multi-chainId filtering is known to be broken—either the API does not accept multiple chainId query parameters, or there is an unresolved bug in the flow.
Either investigate and fix the underlying issue, or remove the broken feature before shipping.
🤖 Prompt for AI Agents
In @packages/thirdweb/src/react/web/ui/Bridge/swap-widget/use-tokens.ts around
lines 80 - 104, The TODO in select-token-ui.tsx about multi-chainId filtering
must be resolved: either remove the broken multi-chainId feature or make
use-tokens.ts and the consumer compatible with the API. Update use-tokens.ts
(the query building in the hook that sets chainId params) to send chain IDs in
the format the API actually accepts (e.g., a single comma-separated chainId
query value instead of multiple chainId params) and adjust select-token-ui.tsx
to consume the new shape (and delete the `// TODO - this is not working!!`
comment). Ensure the unique symbols to edit are the hook that builds the URL in
use-tokens.ts (the code appending chainId params) and the component logic in
select-token-ui.tsx around the TODO, then test the flow end-to-end to verify
filtering works and remove the TODO before merging.
size-limit report 📦
|
PR-Codex overview
This PR introduces a new
SelectedTabtype for managing tab states in the swap widget and refines various components related to wallet connections and token selections, enhancing the user interface and functionality.Detailed summary
SelectedTabtype to manage tab states.WalletDotIconto acceptstyleprop.SwapUIandFundWalletto supportthemeandconnectOptions.DynamicHeightto removemaxHeightprop.WalletSwitcherConnectionScreentoWalletConnectionScreen.SelectTokento useselectedTab.YourTokenButtonfor improved token display.ScreenHeaderfor consistent header formatting.Summary by CodeRabbit
Release Notes
New Features
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.