[SDK] Feature: Remove Switch Network button #7191
Conversation
🦋 Changeset detectedLatest commit: 7303d90 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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. |
WalkthroughThe changes remove the "Switch Network" button from the PayEmbed feature in the "thirdweb" component. The logic for switching networks, previously handled by a separate button, is now integrated into the main "Continue" button's flow, streamlining the user interface and consolidating the network-switching process. Changes
Suggested labels
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
packages/thirdweb/src/react/web/ui/ConnectWallet/screens/Buy/swap/SwapScreenContent.tsx (1)
320-356: 🛠️ Refactor suggestionConsider adding loading state during chain switching.
The Continue button doesn't provide visual feedback during the chain switching operation, which could leave users confused about what's happening, especially if the wallet prompts for network switching approval.
Consider adding a loading state for better UX:
+ const [isSwitchingChain, setIsSwitchingChain] = useState(false); <Button variant={disableContinue ? "outline" : "accent"} fullWidth data-disabled={disableContinue} - disabled={disableContinue} + disabled={disableContinue || isSwitchingChain} onClick={async () => { if (!disableContinue) { if (props.payer.wallet.getChain()?.id !== fromChain?.id) { + setIsSwitchingChain(true); try { await props.payer.wallet.switchChain(fromChain); } catch (error) { console.error("Failed to switch network:", error); return; } finally { + setIsSwitchingChain(false); } } showSwapFlow(); // ... rest of the logic } }} gap="xs" > - {quoteQuery.isLoading ? ( + {quoteQuery.isLoading || isSwitchingChain ? ( <> - Getting price quote + {isSwitchingChain ? "Switching network..." : "Getting price quote"} <Spinner size="sm" /> </> ) : ( "Continue" )} </Button>
🧹 Nitpick comments (1)
.changeset/tricky-points-tease.md (1)
5-5: Consider minor grammatical improvement (optional).The changeset description accurately captures the change. The static analysis tool suggests adding "the" before "PayEmbed" for better grammar, though the current phrasing is commonly used in changelog entries.
Optional improvement:
- Remove unnecessary Switch Network button in PayEmbed + Remove unnecessary Switch Network button in the PayEmbed🧰 Tools
🪛 LanguageTool
[uncategorized] ~5-~5: You might be missing the article “the” here.
Context: --- "thirdweb": patch --- Remove unnecessary Switch Network button in PayEmbed(AI_EN_LECTOR_MISSING_DETERMINER_THE)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.changeset/tricky-points-tease.md(1 hunks)packages/thirdweb/src/react/web/ui/ConnectWallet/screens/Buy/swap/SwapScreenContent.tsx(1 hunks)
🧰 Additional context used
🪛 LanguageTool
.changeset/tricky-points-tease.md
[uncategorized] ~5-~5: You might be missing the article “the” here.
Context: --- "thirdweb": patch --- Remove unnecessary Switch Network button in PayEmbed
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: E2E Tests (pnpm, esbuild)
- 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: Unit Tests
- GitHub Check: Analyze (javascript)
🔇 Additional comments (2)
packages/thirdweb/src/react/web/ui/ConnectWallet/screens/Buy/swap/SwapScreenContent.tsx (1)
325-344: Approve the consolidation of network switching logic.The integration of network switching directly into the Continue button's flow is a good UX improvement that eliminates the need for a separate "Switch Network" button. This streamlines the user experience by reducing the number of steps required.
.changeset/tricky-points-tease.md (1)
1-5: Changeset accurately documents the changes.The changeset properly categorizes this as a patch update and provides a clear, concise description of the removed functionality.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~5-~5: You might be missing the article “the” here.
Context: --- "thirdweb": patch --- Remove unnecessary Switch Network button in PayEmbed(AI_EN_LECTOR_MISSING_DETERMINER_THE)
| if (props.payer.wallet.getChain()?.id !== fromChain?.id) { | ||
| await props.payer.wallet.switchChain(fromChain); | ||
| } |
There was a problem hiding this comment.
Add error handling for the chain switching operation.
The network switching logic is correctly integrated into the Continue button, but the switchChain operation lacks error handling. If the chain switch fails (e.g., user rejects the request, RPC issues), it will throw an unhandled exception that could crash the component.
Consider adding error handling and user feedback:
onClick={async () => {
if (!disableContinue) {
if (props.payer.wallet.getChain()?.id !== fromChain?.id) {
- await props.payer.wallet.switchChain(fromChain);
+ try {
+ await props.payer.wallet.switchChain(fromChain);
+ } catch (error) {
+ console.error("Failed to switch network:", error);
+ // Consider showing an error message to the user
+ return;
+ }
}
showSwapFlow();
// ... rest of the logic
}
}}📝 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.
| if (props.payer.wallet.getChain()?.id !== fromChain?.id) { | |
| await props.payer.wallet.switchChain(fromChain); | |
| } | |
| onClick={async () => { | |
| if (!disableContinue) { | |
| if (props.payer.wallet.getChain()?.id !== fromChain?.id) { | |
| try { | |
| await props.payer.wallet.switchChain(fromChain); | |
| } catch (error) { | |
| console.error("Failed to switch network:", error); | |
| // Consider showing an error message to the user | |
| return; | |
| } | |
| } | |
| showSwapFlow(); | |
| // ... rest of the logic | |
| } | |
| }} |
🤖 Prompt for AI Agents
In
packages/thirdweb/src/react/web/ui/ConnectWallet/screens/Buy/swap/SwapScreenContent.tsx
around lines 327 to 329, the call to switchChain lacks error handling, which can
cause unhandled exceptions if the chain switch fails. Wrap the switchChain call
in a try-catch block to catch any errors, handle them gracefully, and provide
user feedback such as an error message or notification to inform the user about
the failure.
size-limit report 📦
|
Codecov ReportAttention: Patch coverage is
❌ Your patch status has failed because the patch coverage (0.00%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #7191 +/- ##
==========================================
+ Coverage 55.66% 55.67% +0.01%
==========================================
Files 904 904
Lines 58404 58391 -13
Branches 4118 4119 +1
==========================================
- Hits 32511 32510 -1
+ Misses 25788 25776 -12
Partials 105 105
🚀 New features to boost your workflow:
|
PR-Codex overview
This PR focuses on improving the
SwapScreenContentcomponent by removing the unnecessarySwitchNetworkButton, streamlining the code and enhancing the user experience during the token swap process.Detailed summary
SwitchNetworkButtonfrom the component.SwitchNetworkButton.switchChainwithin the button click handler.Summary by CodeRabbit
New Features
Refactor