-
Notifications
You must be signed in to change notification settings - Fork 572
[SDK] Refactor: Only show user balances in payment selection screens #7827
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
🦋 Changeset detectedLatest commit: a3b2cf6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 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 ↗︎
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughForwards an optional fiat currency prop through payment selection UI, adds fiat price rendering using formatted currency amounts in TokenSelection and FiatProviderSelection, updates TokenSelection props and rows to accept Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant PaymentSelection
participant TokenSelection
participant PaymentMethodTokenRow
participant PriceMap as originToken.prices
User->>PaymentSelection: Open payment selection (optional currency)
PaymentSelection->>TokenSelection: Render with props including currency
TokenSelection->>PriceMap: Lookup price by (currency || "USD")
PriceMap-->>TokenSelection: Return currencyPrice (if available)
TokenSelection->>PaymentMethodTokenRow: Render rows with currency and price
PaymentMethodTokenRow-->>User: Display fiat price (if present) and token balance
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 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 (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (2)**/*.{ts,tsx}📄 CodeRabbit Inference Engine (CLAUDE.md)
Files:
**/*.{ts,tsx,js,jsx}📄 CodeRabbit Inference Engine (CLAUDE.md)
Files:
🧠 Learnings (1)📚 Learning: 2025-06-26T19:46:04.024Z
Applied to files:
🧬 Code Graph Analysis (1)packages/thirdweb/src/react/web/ui/Bridge/payment-selection/TokenSelection.tsx (2)
⏰ 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)
🔇 Additional comments (3)
✨ 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 comments)
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. |
packages/thirdweb/src/react/web/ui/Bridge/payment-selection/TokenSelection.tsx
Outdated
Show resolved
Hide resolved
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: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.changeset/warm-ways-decide.md
(1 hunks)packages/thirdweb/src/react/web/ui/Bridge/payment-selection/PaymentSelection.tsx
(1 hunks)packages/thirdweb/src/react/web/ui/Bridge/payment-selection/TokenSelection.tsx
(6 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{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:
packages/thirdweb/src/react/web/ui/Bridge/payment-selection/TokenSelection.tsx
packages/thirdweb/src/react/web/ui/Bridge/payment-selection/PaymentSelection.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Load heavy dependencies inside async paths to keep initial bundle lean (lazy loading)
Files:
packages/thirdweb/src/react/web/ui/Bridge/payment-selection/TokenSelection.tsx
packages/thirdweb/src/react/web/ui/Bridge/payment-selection/PaymentSelection.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). (8)
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: E2E Tests (pnpm, vite)
- GitHub Check: E2E Tests (pnpm, webpack)
- GitHub Check: Size
- GitHub Check: Unit Tests
- GitHub Check: Lint Packages
- GitHub Check: Build Packages
- GitHub Check: Analyze (javascript)
🔇 Additional comments (6)
.changeset/warm-ways-decide.md (1)
1-5
: Changeset looks finePatch note is concise and appropriate for the scope.
packages/thirdweb/src/react/web/ui/Bridge/payment-selection/TokenSelection.tsx (5)
4-4
: Type import is appropriateImporting
SupportedFiatCurrency
here is correct and keeps prop typing explicit.
25-26
: Prop addition looks goodAdding
currency?: SupportedFiatCurrency
toTokenSelectionProps
is correct.
36-37
: Prop threading acknowledgedIncluding
currency?: SupportedFiatCurrency
inPaymentMethodTokenRowProps
is consistent with the parent prop.
43-44
: Parameter plumb-through LGTMThreading
currency
intoPaymentMethodTokenRow
keeps data localized to where it’s rendered.
250-251
: Prop forwarding LGTMPassing
currency
down to eachPaymentMethodTokenRow
is correct.
packages/thirdweb/src/react/web/ui/Bridge/payment-selection/PaymentSelection.tsx
Show resolved
Hide resolved
packages/thirdweb/src/react/web/ui/Bridge/payment-selection/TokenSelection.tsx
Show resolved
Hide resolved
packages/thirdweb/src/react/web/ui/Bridge/payment-selection/TokenSelection.tsx
Outdated
Show resolved
Hide resolved
size-limit report 📦
|
06eb526
to
d6ad0b1
Compare
d6ad0b1
to
727171c
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)
packages/thirdweb/src/react/web/ui/Bridge/payment-selection/PaymentSelection.tsx (1)
295-295
: Prop threading looks good; align onramp PaymentMethod currency to avoid mismatchesForwarding
currency
toTokenSelection
is correct. However,handleOnrampProviderSelected
still setscurrency: "USD"
, which can diverge from the selected/displayed currency.Update the onramp path to use the provided currency or default:
// In handleOnrampProviderSelected const fiatPaymentMethod: PaymentMethod = { currency: currency ?? "USD", onramp: provider, payerWallet, type: "fiat", };Optionally standardize the fallback at the component boundary:
export function PaymentSelection({ /* ... */, currency = "USD", }: PaymentSelectionProps) { // ... }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.changeset/warm-ways-decide.md
(1 hunks)packages/thirdweb/src/react/web/ui/Bridge/payment-selection/FiatProviderSelection.tsx
(1 hunks)packages/thirdweb/src/react/web/ui/Bridge/payment-selection/PaymentSelection.tsx
(1 hunks)packages/thirdweb/src/react/web/ui/Bridge/payment-selection/TokenSelection.tsx
(7 hunks)
✅ Files skipped from review due to trivial changes (1)
- .changeset/warm-ways-decide.md
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/thirdweb/src/react/web/ui/Bridge/payment-selection/TokenSelection.tsx
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{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:
packages/thirdweb/src/react/web/ui/Bridge/payment-selection/PaymentSelection.tsx
packages/thirdweb/src/react/web/ui/Bridge/payment-selection/FiatProviderSelection.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Load heavy dependencies inside async paths to keep initial bundle lean (lazy loading)
Files:
packages/thirdweb/src/react/web/ui/Bridge/payment-selection/PaymentSelection.tsx
packages/thirdweb/src/react/web/ui/Bridge/payment-selection/FiatProviderSelection.tsx
🧠 Learnings (1)
📚 Learning: 2025-06-26T19:46:04.024Z
Learnt from: gregfromstl
PR: thirdweb-dev/js#7450
File: packages/thirdweb/src/bridge/Webhook.ts:57-81
Timestamp: 2025-06-26T19:46:04.024Z
Learning: In the onramp webhook schema (`packages/thirdweb/src/bridge/Webhook.ts`), the `currencyAmount` field is intentionally typed as `z.number()` while other amount fields use `z.string()` because `currencyAmount` represents fiat currency amounts in decimals (like $10.50), whereas other amount fields represent token amounts in wei (very large integers that benefit from bigint representation). The different naming convention (`currencyAmount` vs `amount`) reflects this intentional distinction.
Applied to files:
packages/thirdweb/src/react/web/ui/Bridge/payment-selection/PaymentSelection.tsx
packages/thirdweb/src/react/web/ui/Bridge/payment-selection/FiatProviderSelection.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). (8)
- GitHub Check: Size
- GitHub Check: E2E Tests (pnpm, webpack)
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: Unit Tests
- GitHub Check: E2E Tests (pnpm, vite)
- GitHub Check: Build Packages
- GitHub Check: Lint Packages
- GitHub Check: Analyze (javascript)
packages/thirdweb/src/react/web/ui/Bridge/payment-selection/FiatProviderSelection.tsx
Outdated
Show resolved
Hide resolved
packages/thirdweb/src/react/web/ui/Bridge/payment-selection/FiatProviderSelection.tsx
Outdated
Show resolved
Hide resolved
Codecov Report❌ Patch coverage is ❌ Your patch status has failed because the patch coverage (7.14%) 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 #7827 +/- ##
=======================================
Coverage ? 56.32%
=======================================
Files ? 905
Lines ? 58820
Branches ? 4148
=======================================
Hits ? 33133
Misses ? 25581
Partials ? 106
🚀 New features to boost your workflow:
|
PR-Codex overview
This PR focuses on enhancing the payment selection UI in the
thirdweb
package by improving how currency values are displayed and formatted, particularly in payment widgets. It introduces a new method for formatting currency amounts and updates various components to utilize this method.Detailed summary
formatCurrencyAmount
function to format currency values.FiatProviderSelection
to useformatCurrencyAmount
for displaying quote values.PaymentMethodTokenRow
to calculate and display the currency price based on the selected currency.TokenSelection
from "Select payment token" to "Your token balances".currency
prop toPaymentMethodTokenRow
andTokenSelection
components for better currency management.Summary by CodeRabbit
New Features
Chores