-
Notifications
You must be signed in to change notification settings - Fork 5k
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
Swaps: Create a new swap #11124
Swaps: Create a new swap #11124
Conversation
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
776086a
to
746f96f
Compare
746f96f
to
94b9927
Compare
} else { | ||
history.push(`${ASSET_ROUTE}/${destinationTokenInfo?.address}`); |
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 is this line deleted? history.push(
${ASSET_ROUTE}/${destinationTokenInfo?.address});
I believe the purpose of this was to redirect the user to the screen for the specific token that they are swapping to. Do we not want to do that any more?
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 question! We synced up yesterday and here is a summary: the requirement is to show the Close
button after a swap is done and that will redirect to the main page. However, before the swap is done, a user can click on View in activity
:
Which redirects to the Activity tab, where a user can see progress of their transaction.
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.
const MakeAnotherSwap = () => { | ||
return ( | ||
<Box marginBottom={3}> | ||
<a |
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 think it would be best to use a <button>
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.
Thanks for the suggestion! We had a quick chat about when to use a button or a link and since we do page redirection on click here, a link seems fine.
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.
Code looks good, but I left a question about a functionality change.
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!
* origin/develop: (227 commits) Improve UI + content for price difference notifications (#11145) Swaps: Create a new swap (#11124) Bump @metamask/controllers from 9.0.0 to 9.1.0 (#11150) Capture exception instead of throw error in useTransactionDisplayData (#11153) Fixing jest component test output errors (#11139) Avoid showing "Gas price extremely low" warning in advanced tab for testnets (#11111) @metamask/[email protected] (#11140) Migrate to new CurrencyRateController (#11005) bump allow scripts (#11134) Show Sentry CLI output when uploading artifacts (#11100) use etherscan-link customBlockExplorer methods with customNetwork usage tracking (#11017) Adding notification for updated seed phrase wording (#11131) Bumping package.json Fix a condition for checking if a token should be added (#11127) Removing support survey notification from What's New (#11118) Handling custom token decimal fetch failure due to network error (#10956) Hide basic tab in advanced gas modal for speedup and cancel when on testnets (#11115) Migrate Sentry settings to environment variables (#11085) Update eth-ledger-bridge-keyring to v0.5.0 (#11064) fix metaRPCClientFactory id handling (#11116) ...
Explanation
As a user, I want to submit multiple swaps in a row without clicking on multiple buttons to reset the flow.
We've also changed the button to
Close
after swapping is completed, which redirects a user to the main page.Manual testing steps
Make another swap:
Create a new swap
Build Quote
page is displayed againClose button:
Close
buttonScreenshots
Success page with the
Create a new swap
link andClose
button: