-
Notifications
You must be signed in to change notification settings - Fork 89
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
refactor(swap): clean up Swap widget from isWrapOrUnwrap #2775
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
Looks and works good 👍
Only one behaviour change from prod which might be a problem, but not a big one.
It's no longer possible to insert the receive amount when wrapping/unwrapping.
Yes, the amounts are exactly the same, it has no actual difference from inputting it on sell or buy, but raising it just in case.
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.
Great to see so many deletions of lines of code!
Soft approve, cause i haven't tested it. I would suggest @elena-zh tests this, but not the previous one. Because we might need to do quite some tests
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.
@elena-zh thanks! Fixed everything |
I did it intentionally to keep the code simple. I don't see any concrete reasons for this functionality |
Hey @shoom3301 , all is good, The last issue that I see that there is no balance check on Wrap/unwrap transaction. Could you please fix this as well? Wrap/unwrap button should be disabled when 'could not load balances' and 'insufficient balance' errors are happening. |
@elena-zh thanks! Fixed |
Hey @shoom3301 , the last nitpick: amount is reset to 0 when press on the change tokens arrow on the Swap page: |
@elena-zh thanks, fixed! |
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.
LGTM!
… fix/2768-1 # Conflicts: # src/modules/tradeFormValidation/services/validateTradeForm.ts # src/pages/LimitOrders/index.tsx
<TradeFormBlankButton onClick={context.wrapNativeFlow}> | ||
<TradeFormBlankButton onClick={() => context.wrapNativeFlow()}> |
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.
Why do you need the wrapper fn?
Summary
Context: #2774
Removed
wrap/unwrap
checks from Swap page code, becausewrap/unwrap
flow is consolidated inTradeWidget
.To Test (Swap / Limit / Advanced)