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

Feature: ERC20 approve #997

Merged
merged 11 commits into from
Aug 16, 2019
Merged

Feature: ERC20 approve #997

merged 11 commits into from
Aug 16, 2019

Conversation

estebanmino
Copy link
Contributor

Description

  • This PR adds the ability to render adhoc information for erc20 approve transactions showing here https://recordit.co/IGgdpGmTR5
  • Now we render the token information in transaction review
    image
  • Made some changes to avoid doing math calculation on render methods.
  • Also we have a small bug with a missing locale transaction.optional

Checklist

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

Issue

Resolves #809

@estebanmino estebanmino added the needs-qa Any New Features that needs a full manual QA prior to being added to a release. label Aug 14, 2019
Copy link
Contributor

@brunobar79 brunobar79 left a comment

Choose a reason for hiding this comment

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

💯

@@ -461,7 +461,8 @@
"cancel_tx_title": "Attempt to cancel?",
"cancel_tx_message": "Submitting this attempt does not guarantee your original transaction will be cancelled. If the canellation attempt is successful, you will be charged the transaction fee above.",
"nevermind": "Nevermind",
"gasCancelFee": "Gas Cancellation Fee"
"gasCancelFee": "Gas Cancellation Fee",
"approve_warning": "By approving this action, you grant permission for this contract to spend up to"
Copy link
Contributor

Choose a reason for hiding this comment

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

you will grant?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is what we're saying on the extension 🤷‍♂

@@ -453,7 +453,8 @@
"value_not_available": "No disponible",
"rate_not_available": "Conversión no disponible",
"optional": "Opcional",
"no_address_for_ens": "No existe dirección para nombre ENS"
"no_address_for_ens": "No existe dirección para nombre ENS",
"approve_warning": "Al aprobar esta acción, otorgas permiso para que este contrato gaste hasta"
Copy link
Contributor

Choose a reason for hiding this comment

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

otorgarás

@ibrahimtaveras00
Copy link
Contributor

Perhaps make the warning text align left instead of justified

Smaller screen view:

image

Copy link
Contributor

@ibrahimtaveras00 ibrahimtaveras00 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 now, QA Passed 👍

@estebanmino estebanmino merged commit 6b44dcc into develop Aug 16, 2019
@estebanmino estebanmino deleted the feature/tokens-approve branch August 20, 2019 18:04
rickycodes pushed a commit that referenced this pull request Jan 31, 2022
* rm unnecessary code

* mv calculations to didmount instead of render

* render approve info

* render alert

* fix missing locales

* render contract information

* delete empty

* snaps

* left
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-qa Any New Features that needs a full manual QA prior to being added to a release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ERC20 approve transactions
3 participants