Skip to content

Validate sendToken address when component updates#9907

Merged
tmashuang merged 2 commits intodevelopfrom
comp-update-error-token-address
Nov 19, 2020
Merged

Validate sendToken address when component updates#9907
tmashuang merged 2 commits intodevelopfrom
comp-update-error-token-address

Conversation

@tmashuang
Copy link
Copy Markdown
Contributor

@tmashuang tmashuang commented Nov 18, 2020

On a reproduction of trying to get the Known contract address error message it seems that if an token address is set as the recipient along with asset ETH selected then changing the asset to a token address, the error message won't show.

This will validate the sendtoken address on prop change to properly update the warning message.

Manual testing steps:

  1. On the home screen, Click Send button.
  2. Add a token contract to recipient address. i.e. 0x0F5D2fB29fb7d3CFeE444a200298f468908cC942 (MANA)
  3. On the Screen, the Asset should be ETH. Change to any Token.
  4. The warning Known contract address should not show.

Additional feedback:
Is the Known contract address error sufficient description? Maybe it should be clearer, like Warning: sending token to token address?

On a reproduction of trying to get the error message it seems that if an token address is provided in the address form with ETH selected then changing to a token address, the error message won't show.

This will validate the sendtoken address on prop change to properly update the warning message.
@tmashuang tmashuang requested a review from a team as a code owner November 18, 2020 19:41
@tmashuang tmashuang requested a review from Gudahtt November 18, 2020 19:41
@github-actions
Copy link
Copy Markdown
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.

@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [5043697]
Page Load Metrics (396 ± 67 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint317539105
domContentLoaded25764039413766
load25865539613967
domInteractive25663939413766

Copy link
Copy Markdown
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.

Good catch! LGTM

@Gudahtt
Copy link
Copy Markdown
Member

Gudahtt commented Nov 19, 2020

Is the Known contract address error sufficient description? Maybe it should be clearer, like Warning: sending token to token address?

I agree - "Known contract address" could still be pretty confusing for a lot of users. That doesn't seem great.

@tmashuang tmashuang merged commit 4444846 into develop Nov 19, 2020
@tmashuang tmashuang deleted the comp-update-error-token-address branch November 19, 2020 15:35
@github-actions github-actions bot locked and limited conversation to collaborators Nov 19, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants