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.

Sign up for GitHub

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

Jump to bottom

Fix for #5898 - Converting native ETH to fiat and fiat to native ETH results in wrong values beign displayed on the Amount screen #5961

Merged
merged 4 commits into from
Mar 23, 2023
Merged

Fix for #5898 - Converting native ETH to fiat and fiat to native ETH results in wrong values beign displayed on the Amount screen #5961

merged 4 commits into from
Mar 23, 2023

Conversation

segun
Copy link
Contributor

@segun segun commented Mar 13, 2023

Description

This PR fixes #5898, a bug that shows wrong results during etc/fiat conversion

Checklist

  • There is a related GitHub issue
  • Tests are included if applicable
  • Any added code is fully documented

@segun
use setState callback to call onInputChange
299267b 
Signed-off-by: Olusegun Akintayo <akintayo.segun@gmail.com>
@segun segun requested a review from a team as a code owner March 13, 2023 20:21
@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.

@segun segun changed the title use setState callback to call onInputChange Fix for #5898 - Converting native ETH to fiat and fiat to native ETH results in wrong values beign displayed on the Amount screen Mar 14, 2023
@segun segun marked this pull request as draft March 14, 2023 16:33
@segun
E2E for currency switch
43504e1 
Signed-off-by: Olusegun Akintayo <akintayo.segun@gmail.com>
@segun segun self-assigned this Mar 14, 2023
@segun segun marked this pull request as ready for review March 14, 2023 17:36
@segun
Lint fix
83ec12b 
Signed-off-by: Olusegun Akintayo <akintayo.segun@gmail.com>
@segun
Update snapshot
2af31c0 
Signed-off-by: Olusegun Akintayo <akintayo.segun@gmail.com>
@segun segun merged commit 7b5cce5 into main Mar 23, 2023
@segun segun deleted the fix-conversion-bug-5898 branch March 23, 2023 23:08
@github-actions github-actions bot locked and limited conversation to collaborators Mar 23, 2023
@bschorchit
Copy link

Has this been QAed? @seaona

@seaona
Copy link
Contributor

seaona commented Apr 4, 2023

@bschorchit this has not been QAd.
@segun notice that after having 2 dev reviews, the developer should add the needs-qa label, so the QA engineer can test it. Once QA has passed, the QA engineer adds the label QA-passed and merges the PR to main. Finally a release label is added

@seaona seaona added the needs-qa Any New Features that needs a full manual QA prior to being added to a release. label Apr 4, 2023
@seaona
Copy link
Contributor

seaona commented Apr 13, 2023

I can see how the issue is fixed. I add the QA Pass label.

  • Conversion is correctly performed on Mainnet
Screencast.from.13-04-23.14.28.26.webm
  • Conversion is correctly performed on another networks
Screencast.from.13-04-23.14.29.31.webm
  • Conversion is correctly performed with ERC20 tokens
Screencast.from.13-04-23.14.32.09.webm

@seaona seaona added QA Passed A successful QA run through has been done team-confirmations-secure-ux-PR PR from the confirmations team release-6.5.0 and removed needs-qa Any New Features that needs a full manual QA prior to being added to a release. labels Apr 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jpuri jpuri jpuri approved these changes

@blackdevelopa blackdevelopa blackdevelopa approved these changes

Assignees

@segun segun

Labels
QA Passed A successful QA run through has been done release-6.5.0 team-confirmations-secure-ux-PR PR from the confirmations team unit test coverage confirmed
Projects
None yet
Milestone
No milestone
6 participants
@segun @bschorchit @seaona @jpuri @blackdevelopa @hilvmason