-
Notifications
You must be signed in to change notification settings - Fork 124
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
feat: [Pay
] components
#1349
base: main
Are you sure you want to change the base?
feat: [Pay
] components
#1349
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
src/pay/components/Pay.tsx
Outdated
onStatus={onStatus} | ||
productId={productId} | ||
> | ||
<div className={cn(className, 'flex w-full flex-col gap-2')}> |
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.
className last
'noopener,noreferrer', | ||
); | ||
return; | ||
} |
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.
may be more straightforward to have separate callbacks for handleFundingFlow and handleViewReceipt and then call different callbacks in the button based on the status
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.
I'll refactor this as a fast follow once we have USDC magic spend
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'll delete the funding-related code in a few weeks I believe
src/pay/components/PayProvider.tsx
Outdated
if ( | ||
(error as Error).message?.includes('User denied connection request') | ||
) { | ||
setErrorMessage(USER_REJECTED_ERROR); |
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.
shouldn't this also updateLifecycleStatus?
<div className={cn(text.label2, className)}> | ||
<p className={cn(labelClassName, className)}>{label}</p> | ||
</div> | ||
</div> |
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.
pass className into all three seems like it could make some styling difficult
What changed? Why?
PayButton
,Pay
, andPayStatus
useGetPayStatus
useCommerceContracts
callback to take anaddress
as an argumentNotes to reviewers
How has it been tested?
playground PR next