-
Notifications
You must be signed in to change notification settings - Fork 33
Add Max button to send the whole balance #302
Conversation
Happy to get some more ideas about the look&feel.
|
TODO: |
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.
Works well, some code needs change.
At first the amount not being in the middle also felt a bit weird. Idea: put the max button smaller, cf #38? So that amount still seems in the middle.
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.
I like the small button design
|
||
if (token.address === 'ETH') { | ||
output = fromWei( | ||
toWei(balance).minus(gasBn.mul(toWei(gasPriceBn, 'shannon'))) |
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.
I would prefer to merge #274 first, because the syntax is changed to .multipliedBy
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.
Sure go ahead
on ice until we merge #274 |
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.
lgtm!
Will rebase #274 on top of this. |
closes #254
TODO:
1. Understand why the max button actually submits the form2. Understand this
3. Style the button. More on this later (using this would be great but I couldn't manage to make it work so far)
4. Test with ERC20
I need help regarding 1. and 2. @axelchalon, @amaurymartiny or @yjkimjunior whoever has time :)Looks like this:
it acts as a toggle button, the gas and address can be changed and the amount will update accordingly, if max is selected, the amount field is disabled.