-
Notifications
You must be signed in to change notification settings - Fork 1
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
implement offramp summary popup #394
base: polygon-prototype-staging
Are you sure you want to change the base?
implement offramp summary popup #394
Conversation
✅ Deploy Preview for pendulum-pay ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
Really nice job @Sharqiewicz, it looks amazing 🙏
It's not defined in the ticket but I would suggest we don't close the offramp dialog when the user clicks on 'Continue to partner'. This has some advantages:
- The user doesn't see that the exchange rates/quotes still continuously change/refresh in the base form.
- The user can switch back and forth between the anchor page and vortex to double check the expected values
I would continue showing the info dialog all the way until we switch over to the progress page. WDYT @pendulum-chain/devs and also @pendulum-chain/product?
window.open(anchorUrl, '_blank'); | ||
}} | ||
> | ||
Continue with Partner |
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 should add the 'external' icon that indicates that clicking this button/link will open a new tab.
const FeeDetails = ({ network, feesCost, fiatSymbol, tokenOutAmount, fromToken, anchorUrl }: FeeDetailsProps) => ( | ||
<section className="mt-6"> | ||
<div className="flex justify-between mb-2"> | ||
<p>Offramp fees</p> |
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 should add in the brackets how this offramp fee is comprised. (The code I suggested is not enough, we still need to transform the values to something easy to read for the user)
<p>Offramp fees</p> | |
<p>Offramp fee ({`${outputToken.offrampFeeBasisPoints}%` + outputToken.offrampFeesFixedComponent > 0 ? ` + {outputToken.offrampFeesFixedComponent}` : ""})</p> |
<p> | ||
<strong> | ||
<ExchangeRate tokenOutData={tokenOutAmount} fromToken={fromToken} toTokenSymbol={fiatSymbol} /> | ||
($1.00) |
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.
While it's in the reference image, I think we should remove this. As far as I understand, the idea of this '($1.00)' would be to indicate the dollar value of the resulting quote. This means, in order to make this value make sense, we would need to fetch the ARS or EUR to USD exchange rate from somewhere else and compare our output token amount to that. I think this is too complex and we should rather remove it for now. If this becomes a hard requirement later, we can still add it in the future.
($1.00) |
…es-after-the-confirm-button
); | ||
|
||
return ( | ||
<Dialog content={content} visible={visible} actions={actions} headerText="You're offramping" onClose={onClose} /> |
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 just realized that we should change this title since we don't want to call it 'offramp' but use the term 'sell' everywhere.
<Dialog content={content} visible={visible} actions={actions} headerText="You're offramping" onClose={onClose} /> | |
<Dialog content={content} visible={visible} actions={actions} headerText="You're selling" onClose={onClose} /> |
i agree to this suggestion |
couple of comments
|
No description provided.