-
Notifications
You must be signed in to change notification settings - Fork 1
feat: add proper multi network management to drawer #136
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 GitHub.
|
|
Warning Rate limit exceeded@premiumjibles has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 4 minutes and 25 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughIntroduces a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/agentic-chat/src/components/Portfolio/NetworkWalletRow.tsx (1)
35-35: Unusedgroupclass.The
groupclass is applied but nogroup-*utilities are used in the child elements. If this was intended for hover state styling, consider adding the relevant group utilities or removing the unused class.apps/agentic-chat/src/components/Portfolio/PortfolioDrawer.tsx (1)
90-92: Consider memoizing blockie generation.
makeBlockieis called on every render. While likely fast, the blockies could be memoized since they only change when addresses change.+import { useMemo } from 'react' + // In component: -const evmBlockie = evmAccount.address ? makeBlockie(evmAccount.address) : undefined -const solanaBlockie = solanaAccount.address ? makeBlockie(solanaAccount.address) : undefined +const evmBlockie = useMemo( + () => evmAccount.address ? makeBlockie(evmAccount.address) : undefined, + [evmAccount.address] +) +const solanaBlockie = useMemo( + () => solanaAccount.address ? makeBlockie(solanaAccount.address) : undefined, + [solanaAccount.address] +)
📜 Review details
Configuration used: CodeRabbit 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 ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
apps/agentic-chat/package.json(1 hunks)apps/agentic-chat/src/components/Portfolio/NetworkWalletRow.tsx(1 hunks)apps/agentic-chat/src/components/Portfolio/PortfolioDrawer.tsx(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/agentic-chat/src/components/Portfolio/PortfolioDrawer.tsx (4)
apps/agentic-chat/src/lib/utils.ts (1)
truncateAddress(9-12)apps/agentic-chat/src/components/ui/sheet.tsx (2)
Sheet(103-103)SheetContent(103-103)apps/agentic-chat/src/components/ui/dropdown-menu.tsx (3)
DropdownMenu(204-204)DropdownMenuTrigger(206-206)DropdownMenuContent(207-207)apps/agentic-chat/src/components/Portfolio/NetworkWalletRow.tsx (1)
NetworkWalletRow(20-68)
⏰ 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: main
🔇 Additional comments (5)
apps/agentic-chat/src/components/Portfolio/NetworkWalletRow.tsx (1)
20-67: Well-structured reusable component.The
NetworkWalletRowcomponent cleanly encapsulates the per-network wallet display logic with proper conditional rendering for connected/disconnected states. The defensive check on line 46 (address && onCopy(address)) correctly handles the optional address.apps/agentic-chat/src/components/Portfolio/PortfolioDrawer.tsx (3)
36-38: Verify which address is displayed when both networks are connected.The
addressfrom the defaultuseAppKitAccount()(line 36) is used fortruncatedAddressdisplay (line 88, shown on line 121). When both EVM and Solana wallets are connected, which address is shown may be ambiguous or depend on the last active namespace.Consider showing the primary/active network address explicitly or displaying both addresses in the trigger button.
Also applies to: 88-88, 121-121
64-80: Good UX pattern for network switching on disconnect.The disconnect handlers correctly capture the other network's connection state before the async disconnect, then switch to the remaining connected network. This provides a smooth user experience when managing multiple networks.
100-147: Clean dropdown implementation for multi-network wallet management.The conditional UI logic for displaying stacked blockies when both networks are connected vs. a single wallet icon is a nice UX touch. The
NetworkWalletRowcomponents are correctly wired with all required props.apps/agentic-chat/package.json (1)
38-38: New dependency for blockie generation is appropriate and secure.The
ethereum-blockies-base64package (v1.0.2) is a mature, lightweight library with MIT license, a single dependency, and no reported security vulnerabilities. The caret versioning constraint is safe for this stable package.
c111880 to
fc2b2a6
Compare
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
♻️ Duplicate comments (1)
apps/agentic-chat/src/components/Portfolio/PortfolioDrawer.tsx (1)
82-86: Missing error handling for clipboard operation.The
navigator.clipboard.writeTextcan reject if clipboard permissions are denied. This was already flagged in a previous review.
🧹 Nitpick comments (2)
apps/agentic-chat/src/components/Portfolio/PortfolioDrawer.tsx (2)
64-80: Consider adding error handling for disconnect and network switch operations.Both handlers ignore potential errors from
disconnect()andmodal?.switchNetwork(). If these operations fail, the UI may not reflect the actual wallet state.const handleDisconnectEvm = () => { const solanaWasConnected = solanaAccount.isConnected - void disconnect({ namespace: 'eip155' }).then(() => { - if (solanaWasConnected) { - void modal?.switchNetwork(solana) - } - }) + void disconnect({ namespace: 'eip155' }) + .then(() => { + if (solanaWasConnected) { + void modal?.switchNetwork(solana) + } + }) + .catch(() => { + toast.error('Failed to disconnect EVM wallet') + }) } const handleDisconnectSolana = () => { const evmWasConnected = evmAccount.isConnected - void disconnect({ namespace: 'solana' }).then(() => { - if (evmWasConnected) { - void modal?.switchNetwork(mainnet) - } - }) + void disconnect({ namespace: 'solana' }) + .then(() => { + if (evmWasConnected) { + void modal?.switchNetwork(mainnet) + } + }) + .catch(() => { + toast.error('Failed to disconnect Solana wallet') + }) }
121-121: Consider clarifying which address is displayed when both networks are connected.When both EVM and Solana are connected,
truncatedAddressshows the address from the genericuseAppKitAccount()hook (line 36), but it's unclear to users which network's address this represents. Consider either:
- Showing both truncated addresses
- Adding a network indicator (e.g., "EVM: 0x1234...")
- Showing a count like "2 wallets connected"
📜 Review details
Configuration used: CodeRabbit 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 ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
apps/agentic-chat/package.json(1 hunks)apps/agentic-chat/src/components/Portfolio/NetworkWalletRow.tsx(1 hunks)apps/agentic-chat/src/components/Portfolio/PortfolioDrawer.tsx(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/agentic-chat/src/components/Portfolio/NetworkWalletRow.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
apps/agentic-chat/src/components/Portfolio/PortfolioDrawer.tsx (4)
apps/agentic-chat/src/lib/utils.ts (1)
truncateAddress(9-12)apps/agentic-chat/src/components/ui/sheet.tsx (2)
Sheet(103-103)SheetContent(103-103)apps/agentic-chat/src/components/ui/dropdown-menu.tsx (3)
DropdownMenu(204-204)DropdownMenuTrigger(206-206)DropdownMenuContent(207-207)apps/agentic-chat/src/components/Portfolio/NetworkWalletRow.tsx (1)
NetworkWalletRow(20-68)
⏰ 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: main
🔇 Additional comments (1)
apps/agentic-chat/src/components/Portfolio/PortfolioDrawer.tsx (1)
127-145: LGTM!Clean integration of
NetworkWalletRowcomponents with proper props for each network. TheshowSeparatoron the Solana row provides good visual distinction between network sections.
fc2b2a6 to
085827a
Compare
085827a to
7042da8
Compare
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.