Skip to content
This repository has been archived by the owner on May 10, 2024. It is now read-only.

Fix #6606: added UI to handle tx different statuses #6960

Merged
merged 8 commits into from
Feb 22, 2023

Conversation

nuo-xu
Copy link
Contributor

@nuo-xu nuo-xu commented Feb 16, 2023

Summary of Changes

For each transaction user clicks confirm in the transaction confirmation screen, it will always have UI to handle the changing of the states. It includes when tx is in loading/submitting, tx has been submitted or signed, tx has been confirmed/completed.

This pull request fixes #6606 #6861

Submitter Checklist:

  • Unit Tests are updated to cover new or changed functionality
  • User-facing strings use NSLocalizableString()
  • New or updated UI has been tested across:
    • Light & dark mode
    • Different size classes (iPhone, landscape, iPad)
    • Different dynamic type sizes

Test Plan:

Test regular transaction status changes:

  1. Make either a send a swap transaction via Brave Wallet
  2. A transaction confirmation screen will pop up
  3. click confirm
  4. observe a loading screen will show up
  5. observe after the loading, view will become transaction submitted state which has icon, text and buttons
  6. click the ok button
  7. observe it will dismiss the whole screen
  8. repeat step 1 to 5
  9. click the explorer button
  10. observe it will open transaction url
  11. repeat step 1 to 5
  12. stay at this screen for few seconds
  13. observe the view will change to another state indicates the tx has been completed
  14. click the receipt button
  15. observe a transaction detail screen show up
  16. close the detail screen and click close button
  17. observe the whole screen will be dismissed
  18. repeat step 1 to 2
  19. click advanced setting in the confirmation screen and edit nonce to be 0. save it
  20. click confirm
  21. observe a loading screen will show up
  22. observe after loading, view will become transaction failed with an error message
  23. click ok button
  24. observe it will dismiss the whole screen.

Test signed transaction:

  1. Setup https://github.com/bbondy/eth-manual-tests
  2. Change to any test network and visit http://localhost:8081/request.html
  3. setup safara debug console to check any output from http://localhost:8081/request.html
  4. Click ethereum.enable and grant permission
  5. Fill to address next to eth_signTransaction and click it
  6. observe a notification will show up and click the notification to open brave wallet
  7. once the wallet is unlocked, a send pending transaction will show up
  8. click confirm
  9. observe a loading state will apear
  10. observe after loading, the tx status will change to signed.
  11. click ok to close the screen
  12. check account activities to see the transaction appears with status Signed
  13. Paste the return signed transaction in console to eth_sendRawTransaction and click
  14. Check transactions in account activity and previously Signed transaction should be Confirmed after a while

Screenshots:

Simulator.Screen.Recording.-.iPhone.14.Pro.-.2023-02-15.at.23.58.11.mp4

Reviewer Checklist:

  • Issues include necessary QA labels:
    • QA/(Yes|No)
    • bug / enhancement
  • Necessary security reviews have taken place.
  • Adequate unit test coverage exists to prevent regressions.
  • Adequate test plan exists for QA to validate (if applicable).
  • Issue and pull request is assigned to a milestone (should happen at merge time).

@nuo-xu nuo-xu requested a review from StephenHeaps February 16, 2023 18:02
@nuo-xu nuo-xu self-assigned this Feb 16, 2023
@nuo-xu nuo-xu requested a review from a team as a code owner February 16, 2023 18:02
@nuo-xu nuo-xu requested a review from kylehickinson February 16, 2023 18:07
Copy link
Contributor

@StephenHeaps StephenHeaps left a comment

Choose a reason for hiding this comment

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

A couple initial comments

@nuo-xu nuo-xu force-pushed the wallet/eth_signTransaction branch 2 times, most recently from 3f68f77 to 29dc327 Compare February 18, 2023 20:53
@StephenHeaps
Copy link
Contributor

There's a bug if you have a pending request that is lower priority than eth_signTransaction that causes the modal to present again but showing the error state:

Simulator.Screen.Recording.-.iPad.Pro.11-inch.3rd.generation.-.2023-02-21.at.09.51.52.mp4

STR:

  1. create non-transaction dapp request (ex. personal_sign)
  2. dismiss / minimize the request so it's left pending
  3. create eth_signTransaction tx
  4. confirm the eth_signTransaction
  5. cancel/sign the dapp request from step 1

We may need a special check when pendingRequest contains empty transactions (ex after confirming eth_signTransaction here) to verify the completion state is shown (submitted/completed/signed/error views) before we show the dapp request?

@StephenHeaps
Copy link
Contributor

Some state issues if the transaction modal is dismissed when showing the transaction status with the latest changes, previous commit it would re-open and show the error state... In earlier testing of this PR would show the transaction status as it was previously shown 🤔.

Simulator.Screen.Recording.-.iPad.Pro.11-inch.3rd.generation.-.2023-02-21.at.13.08.21.mp4

@StephenHeaps StephenHeaps added this to the 1.48 milestone Feb 22, 2023
Copy link
Contributor

@StephenHeaps StephenHeaps left a comment

Choose a reason for hiding this comment

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

Lgtm, nice work on getting through bugs with the changes here 🎉

@nuo-xu nuo-xu requested a review from kylehickinson February 22, 2023 18:00
@nuo-xu nuo-xu force-pushed the wallet/eth_signTransaction branch from 8d59b95 to 6d1a4dc Compare February 22, 2023 19:56
@nuo-xu nuo-xu merged commit dccca81 into development Feb 22, 2023
@nuo-xu nuo-xu deleted the wallet/eth_signTransaction branch February 22, 2023 20:36
@nuo-xu nuo-xu modified the milestones: 1.48, 1.49 Feb 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants