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

Error states for new Tx overview: contract & network fees #9037

Closed
rachelhamlin opened this issue Sep 25, 2019 · 7 comments · Fixed by #10857
Closed

Error states for new Tx overview: contract & network fees #9037

rachelhamlin opened this issue Sep 25, 2019 · 7 comments · Fixed by #10857
Assignees
Labels
feature feature requests
Projects

Comments

@rachelhamlin
Copy link
Contributor

rachelhamlin commented Sep 25, 2019

Problem

We redesigned the transaction overview screen in #8026, but error states have not been updated to match.

There are two key error states we need to cover:
(1) When the user does not have enough ETH to pay for network fees (gas).
(2) Contract-related errors that are special cases arising from an inability of the contract to accurately calculate gas.

Initially we spec'd out these error states in #8026 and #8691, respectively—but as there is overlap between them, this issue merges them into one.

Implementation

Figma screens for insufficient network fee
image

Figma screens for contract errors
image

Acceptance Criteria

The terminology to be used in app is now network fee rather than gas.

There is a loading state utilizing spinner animation as the network fee is being calculated.

For the network fee error

When the user is initiating the transaction (from wallet, or in future, chat):

  • They will not make it past this screen if they have an insufficient number of tokens for the transaction. They will never see both errors at once on the final overview.
  • They can, however, bump into the insufficient network fee error, if they are low on ETH.

When a DApp initiates the transaction:

  • Either or BOTH errors can occur at once:
  • The user may have enough tokens, but not enough ETH for gas;
  • Or the user may have enough ETH for gas, but not enough tokens;
  • Or the user may be lacking in both.

The error states should accurately reflect this.

Note that if both error states are displayed, tapping on either info bubble should display the Not enough [token] (do not hard code ETH!) or Not enough ETH tool tip. If one is open already and the other is tapped, the first tool tip is minimized, and the second is opened.

For the contract error

The user is gated from signing the Tx unless they manually Set custom fee. Then they may continue with an additional warning about the likelihood of failure.

@rachelhamlin
Copy link
Contributor Author

rachelhamlin commented Sep 25, 2019

cc @andmironov for your review and also @flexsurfer - do you know if this bottom sheet is implemented yet? If not, I will create a separate issue for it.

EDIT: the old version of the screen exists. We can continue to use that for this PR, with added error state for insufficient network fee. To be updated on Figma, please, Andrei 🙏

I will create a separate issue for this future version, post v1 -
image

@3esmit
Copy link
Member

3esmit commented Sep 26, 2019

Please, don't totally hide 'data' field from contract calls.

@flexsurfer
Copy link
Member

WIP #9236

@rachelhamlin
Copy link
Contributor Author

Reviewing this issue again and I'm no longer thinking that it's strictly required for v1. I'm happy to finish the WIP PR, but want to comment that some of the open issues in keycard are probably more urgent to fix for launch.

@StatusSceptre StatusSceptre moved this from Doing to To Do - P1 in Core Nov 5, 2019
@rachelhamlin rachelhamlin moved this from To Do - P1 to To Do - P2 in Core Jan 27, 2020
@flexsurfer flexsurfer moved this from To Do - P2 to To Do - P0 in Core Feb 14, 2020
@flexsurfer
Copy link
Member

moved to P0 because currently users think there is an error with gas and set gas manually and spent their funds but tx fails

@rachelhamlin
Copy link
Contributor Author

@flexsurfer you mean when they don't have any tokens for an ENS name?

@flexsurfer
Copy link
Member

it can be any contract interaction but ens name as well yes

@rachelhamlin rachelhamlin moved this from To Do - P0 to To Do - P2 in Core Apr 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature feature requests
Projects
Core
  
Done
4 participants