-
Notifications
You must be signed in to change notification settings - Fork 984
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
[#10105] Add set max to Send transaction #10877
Conversation
Jenkins BuildsClick to see older builds (12)
|
ade084e
to
cceddbb
Compare
{:on-press | ||
#(when token | ||
(re-frame/dispatch [:wallet.send/set-max-amount token]))} | ||
[react/view {:height 35 :border-radius 40 :background-color colors/blue-light |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks unreadable to me, do you mind to add comas here or keep one pair per line?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
cceddbb
to
62871f0
Compare
Tested with iOS13, Android8, Android 9 3rd build (
|
thanks @Serhy |
actually this set max is tricky for eth, if you set max and then change fee, it won't be max anymore |
How do I get the NaN again? I couldn't seem to replicate it, anyway that shouldn't be displayed under any circumstances, should be a 0.0 |
The way Trust Wallet works is: Set Max = sets all available ETH -> Next -> and on the next screen it becomes (Max - Fee). Further changes of fee deducts/increases this If I'm correct we have a fixate ETH amount on the |
62871f0
to
4b40b4c
Compare
thanks @Serhy 2 and 3 fixed |
Small request - could you please rebase to latest master so this commit b5fda12 gets included? This means we aren't running E2E tests on eth.prod cluster anymore, and since this happens on every push, the impact is quite big. This ensures we have more accurate metrics going forward from Jul 1 onward, which would be awesome. See https://discuss.status.im/t/user-growth-and-retention/1782 for more |
4b40b4c
to
8be3c75
Compare
Tested on iOS13, Android8.1, Android 9 - all above issues fixed, can set max value for ETH/token. @flexsurfer I just noticed the currency in newly introduced fiat value always set to |
8be3c75
to
c7712eb
Compare
@Serhy fixed, hided it for all cases |
99% of end-end tests have passed
Failed tests (1)Click to expand
Passed tests (93)Click to expand
|
Looks good now! Meaningful currency code everywhere. |
Signed-off-by: andrey <motor4ik@gmail.com>
c7712eb
to
24114f7
Compare
fixes #10105
Also added fiat value