Skip to content
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

Merged
merged 9 commits into from May 21, 2021
Merged

Conversation

dan437
Copy link
Contributor

@dan437 dan437 commented May 18, 2021

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:

  • Make a swap
  • On the success page click on Create a new swap
  • The Build Quote page is displayed again

Close button:

  • Make a swap
  • Click on the Close button
  • The main page is displayed

Screenshots

Success page with the Create a new swap link and Close button:
image

@dan437 dan437 requested a review from a team as a code owner May 18, 2021 10:01
@dan437 dan437 requested a review from ryanml May 18, 2021 10:01
@github-actions
Copy link
Contributor

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.

@dan437 dan437 changed the title Swaps: Make another swap Swaps: Create a new swap May 18, 2021
} else {
history.push(`${ASSET_ROUTE}/${destinationTokenInfo?.address}`);
Copy link
Contributor

@danjm danjm May 19, 2021

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?

Copy link
Contributor Author

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:
image

Which redirects to the Activity tab, where a user can see progress of their transaction.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An update.. I was redirecting to the Activity tab on the main page. I changed it back to the Activity tab on an asset's page, so it looks like this now:
image

const MakeAnotherSwap = () => {
return (
<Box marginBottom={3}>
<a
Copy link
Contributor

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

Copy link
Contributor Author

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.

Copy link
Contributor

@danjm danjm left a 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.

Copy link
Contributor

@danjm danjm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@brad-decker brad-decker merged commit df6cb42 into MetaMask:develop May 21, 2021
Gudahtt added a commit that referenced this pull request May 21, 2021
* 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/auto-changelog@2.0.1 (#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)
  ...
ryanml pushed a commit that referenced this pull request Jun 22, 2021
@dan437 dan437 deleted the swaps-make-another-swap branch July 24, 2023 11:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants