-
Notifications
You must be signed in to change notification settings - Fork 628
Add Fiat onramp, Add PayEmbed #2801
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: 35adf7a The changes in this PR will be included in the next version bump. This PR includes changesets to release 10 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 |
CodSpeed Performance ReportMerging #2801 will not alter performanceComparing Summary
|
size-limit report 📦
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2801 +/- ##
==========================================
- Coverage 65.12% 64.54% -0.58%
==========================================
Files 744 772 +28
Lines 52049 54159 +2110
Branches 3091 3092 +1
==========================================
+ Hits 33896 34957 +1061
- Misses 17488 18536 +1048
- Partials 665 666 +1
*This pull request uses carry forward flags. Click here to find out more.
|
|
/release-pr |
|
/release-pr |
|
/release-pr |
|
/release-pr |
|
/release-pr |
|
/release-pr |
|
/release-pr |
| // Must pass 0 otherwise it will throw on some chains | ||
| const gasCost = await estimateGasCost({ | ||
| transaction: { ...tx, value: 0n }, | ||
| transaction: tx, // NEED A PROPER FIX HERE !!! |
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.
what was the issue here?
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.
The value: 0n override was added because estimateGasCost was throwing errors on Optimism, Base, Arbitrum without it.
packages/thirdweb/src/react/core/hooks/pay/useBuyWithFiatQuote.ts
Outdated
Show resolved
Hide resolved
|
/release-pr |
jnsdls
left a 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.
review part(1/n)
packages/thirdweb/src/pay/buyWithFiat/isSwapRequiredPostOnramp.ts
Outdated
Show resolved
Hide resolved
|
PR is not ready for review yet - I'm still reviewing |
| client={data.tx.client} | ||
| localeId={payModal?.locale || "en_US"} | ||
| supportedTokens={payModal?.supportedTokens || defaultTokens} | ||
| supportedTokens={payModal?.supportedTokens} |
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.
we don't default to defaultTokens here anymore?
PR-Codex overview
This PR introduces minor improvements to the thirdweb Pay UI.
Detailed summary