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

[#8026] Update not enough ETH warning #8485

Closed
hesterbruikman opened this issue Jun 25, 2019 · 26 comments
Closed

[#8026] Update not enough ETH warning #8485

hesterbruikman opened this issue Jun 25, 2019 · 26 comments

Comments

@hesterbruikman
Copy link
Contributor

hesterbruikman commented Jun 25, 2019 •
edited by rachelhamlin
Loading

Description

Type: Feature

Summary: Continuation of the work done in this PR for #8026.

New overview screen warning: "Not enough ETH for gas". This warning provides insufficient information to new users as the what the problem is and how to fix it and refers to gas which is inconsistent with language in the Overview screen (Network fee). Update includes a warning that provides more context.

Current behavior
IMG_0129

Expected behavior
image
Expected behavior was updated as of this comment
@hesterbruikman hesterbruikman mentioned this issue Jun 25, 2019
Closed
@gitcoinbot
Copy link

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

This issue now has a funding of 70.0 DAI (70.0 USD @ $1.0/DAI) attached to it.

@rachelhamlin
Copy link
Contributor

Happy to see you back @MajorTomSec! 🙌 I approved you for this bounty. Let us know if you need anything here.

@flexsurfer
Copy link
Member

@andmironov could you take a look if the design is up to date in the description? maybe we could implement it similar to #8691

@andmironov
Copy link

andmironov commented Sep 4, 2019 •
edited
Loading

yep, I suggest doing this

Screen Shot 2019-09-04 at 12 52 08

@flexsurfer
Copy link
Member

@andmironov but the point of this issue description, no?

@andmironov
Copy link

andmironov commented Sep 4, 2019 •
edited
Loading

when there are several errors, let it be like this:

Tap on the icon shows the tooltip and hides the other tooltip if it's shown already

Screen Shot 2019-09-04 at 16 08 56

@andmironov
Copy link

If the goal is to explicitly tell that the user must have ETH to pay the network fee, we can mix these approaches and for consistency's sake do something like the screens below.

But I believe the red (!) icon and a tooltip migh do the job!

Screen Shot 2019-09-04 at 16 22 00

@rachelhamlin
Copy link
Contributor

Okay, so to clarify this issue - let's use these screens, NOT the Expected behavior above?

I also think this new red error state is clear enough.

Can you add the Figma link to these versions @andmironov?

@MajorTomSec does this makes sense? I'll update the issue description. We could also roll #8691 into this one—it's an additional error state using the same styles. If you're willing, I'll double the bounty size for it (will pay out the extra amount in a tip once completed).

@andmironov
Copy link

Can you add the Figma link to these versions @andmironov?

https://www.figma.com/file/XUehMnhyD1FGcWzvGz6SXqvh/Mobile-wallet?node-id=3864%3A33253

@MajorTomSec
Copy link
Contributor

@MajorTomSec does this makes sense? I'll update the issue description. We could also roll #8691 into this one—it's an additional error state using the same styles. If you're willing, I'll double the bounty size for it (will pay out the extra amount in a tip once completed).

Sure, this seems fine to me, let's get this done !

@gitcoinbot
Copy link

@MajorTomSec Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

  • reminder (3 days)
  • escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

@MajorTomSec
Copy link
Contributor

Yes I'm still working on the issue, expect a PR in one or two days. Was just busy the last few days ! Sorry for the delay !

@rachelhamlin
Copy link
Contributor

Glad you're still around @MajorTomSec! Thanks for checking in.

@gitcoinbot
Copy link

@MajorTomSec Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

  • reminder (3 days)
  • escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

1 similar comment
@gitcoinbot
Copy link

@MajorTomSec Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

  • reminder (3 days)
  • escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

@MajorTomSec
Copy link
Contributor

Hello there,
I'm sorry I'll have to stop working on this issue.
I thought I'd have more free time than this, but life has kept me busy recently, so I'd better leave this issue to someone else.
I'll work on it when I get more free time eventually, if no one takes it !
Sorry for holding the issue !

@rachelhamlin
Copy link
Contributor

rachelhamlin commented Sep 16, 2019 via email

@gitcoinbot
Copy link

gitcoinbot commented Sep 17, 2019 •
edited
Loading

Issue Status: 1. Open 2. Cancelled

Work has been started.

These users each claimed they can complete the work by 8 months, 1 week ago.
Please review their action plans below:

1) igormuba has been approved to start work.

I have cloned the repository. Will explore it a bit and see if I can find where to apply the changes.

Learn more on the Gitcoin Issue Details page.

@rachelhamlin
Copy link
Contributor

@igormuba approved you for the bounty - welcome!

@igormuba igormuba mentioned this issue Sep 20, 2019
Closed
@igormuba
Copy link

I have changed the error tooltip to a red text below the field where the error occurs.
I didn't create a clickable tooltip because the whole square of the transaction fee is clickable (to edit the fee), and it is not a good idea to put a clickable button inside a clickable view.
How do you like this new error message?
Captura de Tela 2019-09-20 às 00 43 44

@gitcoinbot
Copy link

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

Work for 70.0 DAI (70.0 USD @ $1.0/DAI) has been submitted by:

  1. @igormuba

@StatusSceptre please take a look at the submitted work:

@andmironov
Copy link

Hey, @igormuba great job on improving the screen!

the whole square of the transaction fee is clickable (to edit the fee), and it is not a good idea to put a clickable button inside a clickable view.

Great catch, I agree with your point that if the whole row is clickable it's a bit weird to add a clickable error inside of it. But if you look at this problem from a different angle it might make more sense. This error message design is supposed to also help indicate several errors at once. For instance when there are both not enough tokens to send and not enough eth to pay for the network fee.

Screen Shot 2019-09-23 at 13 39 47

This is a more universal and compact way to visually indicate the error.

A solution to this problem might be increasing the clickable area of the (i) icon to prevent users from tapping the row instead.

Screen Shot 2019-09-23 at 13 42 43

Also, you helped me realize that if the user still wants to open the network fee settings popup on purpose there's no error indication there. I suggest disabling all the fields and links and showing the error like this:

Screen Shot 2019-09-23 at 13 46 31

But as far as I know, the network fee screens have not been implemented yet anyway. I wonder how it all will work together cc @rachelhamlin @flexsurfer

@flexsurfer
Copy link
Member

it's implemented in a simple form
image

@rachelhamlin rachelhamlin mentioned this issue Sep 25, 2019
Closed
@rachelhamlin
Copy link
Contributor

Thanks @flexsurfer! We can run with that for now. I will create a separate issue to handle the slider, but it can be done after v1 - agreed @andmironov?

Can you put the network fee error state on this design for the interim?

Let's continue within #9037. It replaces this issue.

@rachelhamlin rachelhamlin mentioned this issue Sep 25, 2019
Closed
@igormuba
Copy link

igormuba commented Oct 2, 2019

What about the bounty? It expires soon

@gitcoinbot
Copy link

Issue Status: 1. Open 2. Cancelled

The funding of 70.0 SAI (70.0 USD @ $1.0/SAI) attached to this issue has been cancelled by the bounty submitter

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
7 participants
@andmironov @MajorTomSec @flexsurfer @rachelhamlin @igormuba @gitcoinbot @hesterbruikman