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

Fixes display of token amount on the confirm transaction screen #6763

Closed
wants to merge 2 commits into from

Conversation

danjm
Copy link
Contributor

@danjm danjm commented Jun 28, 2019

Fixes #6762

As discovered by @Gudahtt, 748801f#diff-18388ebb04a4abd10f886dfe385e77d8L391 removed a call to updateTokenProps, which would have set the correct decimals in state.confirmTransaction.tokenProps.tokenDecimals. Without that data in state, calculations of token amounts were accounting for the token's decimal values.

This PR fixes that issue by retrieving the necessary data for correct rendering of token amounts on mount of the confirm-transaction.component and then setting it in state with the other token data. It discontinues use of the confirmTransaction.tokenProps for token transaction confirm screen rendering. Necessary values are now calculated within confirm-token-transaction-base.component. This aligns with our eventual desire to discontinue use of that part of state altogether.

The first commit of this PR adds an e2e test that currently fails on develop and passes with the fixes of this PR.

Peek 2019-06-28 01-24

@danjm danjm requested a review from whymarrh as a code owner June 28, 2019 04:01
@danjm danjm requested a review from Gudahtt June 28, 2019 04:03
@metamaskbot
Copy link
Collaborator

Builds ready [a6fc002]: chrome, firefox, edge, opera

Copy link
Member

@Gudahtt Gudahtt left a comment

Choose a reason for hiding this comment

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

Looks good! This seems like a good solution, and I tested it locally as well.

@Gudahtt Gudahtt mentioned this pull request Jun 28, 2019
danfinlay added a commit that referenced this pull request Jul 1, 2019
Using admin privilege to bypass code owner review because it has already [been reviewed](#6763) and those code owners are out today.
@Gudahtt
Copy link
Member

Gudahtt commented Jul 3, 2019

These two commits have already landed in develop via the Version-6.7.1 release branch, (789fc8b, 0e108db)

@Gudahtt Gudahtt closed this Jul 3, 2019
@whymarrh whymarrh deleted the i6762-fix-token-decimals branch July 3, 2019 01:26
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.

Extra zeros in token value on send confirmation screen
5 participants