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

ParaTimes: cancel transfer instead of navigating back to ParaTime selection #1352

Merged
merged 3 commits into from
Mar 30, 2023

Conversation

buberdds
Copy link
Contributor

@buberdds buberdds commented Mar 27, 2023

Currently user can navigate back to ParaTime selector step which can lead to weird experience as we keep data for next steps.

sample: fill in Cipher recipient go back to the first step, select emerald, click next and we have Oasis addr in recipient input where we need eth addr.

@github-actions
Copy link

github-actions bot commented Mar 27, 2023

🦙 MegaLinter status: ✅ SUCCESS

Descriptor Linter Files Fixed Errors Elapsed time
✅ EDITORCONFIG editorconfig-checker 4 0 0.02s
✅ REPOSITORY checkov yes no 18.93s
✅ REPOSITORY git_diff yes no 0.0s
✅ TSX eslint 2 0 0 4.94s
✅ TYPESCRIPT eslint 2 0 0 4.54s

See detailed report in MegaLinter reports
Set VALIDATE_ALL_CODEBASE: true in mega-linter.yml to validate all sources, not only the diff

MegaLinter is graciously provided by OX Security

@codecov
Copy link

codecov bot commented Mar 27, 2023

Codecov Report

Merging #1352 (a8a4760) into master (6fbc754) will decrease coverage by 0.21%.
The diff coverage is 100.00%.

❗ Current head a8a4760 differs from pull request most recent head 0c75a20. Consider uploading reports for the commit 0c75a20 to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1352      +/-   ##
==========================================
- Coverage   84.44%   84.24%   -0.21%     
==========================================
  Files         142      142              
  Lines        3652     3631      -21     
  Branches      668      666       -2     
==========================================
- Hits         3084     3059      -25     
- Misses        568      572       +4     
Flag Coverage Δ
cypress 51.17% <0.00%> (+0.21%) ⬆️
jest 79.47% <100.00%> (-0.23%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...pages/ParaTimesPage/TransactionRecipient/index.tsx 94.44% <100.00%> (-0.16%) ⬇️

... and 10 files with indirect coverage changes

@buberdds buberdds changed the title Cancel transfer instead of navigating back to ParaTime selection ParaTimes: cancel transfer instead of navigating back to ParaTime selection Mar 27, 2023
@buberdds buberdds requested a review from lukaw3d March 27, 2023 09:01

expect(navigateToWithdraw).toHaveBeenCalled()
expect(clearTransactionForm).toHaveBeenCalled()
Copy link
Member

Choose a reason for hiding this comment

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

could we instead have a test more like

sample: fill in Cipher recipient go back to the first step, select emerald, click next and we have Oasis addr in recipient input where we need eth addr.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That belongs to e2e test more. I can add one and comment it out temporary. Which one is now preferable in Wallet: cypress or playwright ?

Copy link
Member

Choose a reason for hiding this comment

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

playwright

@cloudflare-pages
Copy link

cloudflare-pages bot commented Mar 29, 2023

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 0c75a20
Status: ✅  Deploy successful!
Preview URL: https://d3f414eb.oasis-wallet.pages.dev
Branch Preview URL: https://mz-paratimeformreset.oasis-wallet.pages.dev

View logs

@buberdds buberdds merged commit 6cf2245 into master Mar 30, 2023
@buberdds buberdds deleted the mz/paraTimeFormReset branch March 30, 2023 07:56
@@ -0,0 +1,30 @@
// Uncomment when ParaTimes are released
Copy link
Member

Choose a reason for hiding this comment

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

I would have e2e test running already with e.g.

  const canAccessParaTimesRoute =
    process.env.REACT_APP_E2E_TEST && // Remove when we can officially show ParaTimes to users

Copy link
Contributor Author

Choose a reason for hiding this comment

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

totally forgot about this env, thx

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

2 participants