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

Set max amount - Closes #1234 #1377

Merged
merged 22 commits into from
Nov 2, 2018
Merged

Set max amount - Closes #1234 #1377

merged 22 commits into from
Nov 2, 2018

Conversation

bmaggi-lisk
Copy link
Contributor

What issue have I solved?

-- #1234

How have I implemented/fixed it?

Add a link (behaves like a button) that sets the input to max amount minus the transaction fee

How has this been tested?

  1. Go to wallet send
  2. Locate the amount input (bottom one)
  3. Click the Set max. amount link
  4. Input should have value of total LISK in wallet minus the transaction fee

Review checklist

Copy link
Contributor

@slaweet slaweet left a comment

Choose a reason for hiding this comment

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

  • "Set max. amount" shouldn't be inside the Converter component, because that one is used not just in "Send" but also in "Request". "Set Max Amount" can be directly in the "SendWrtitable"
  • "Set max. amount" should have more bottom margin
  • "Set max. amount" should not be displayed if the amount input is not focused (as described in the issue).

wrapper.find('.set-max-amount').simulate('click');
/* eslint-disable no-unused-expressions */
expect(props.onSetMaxAmount).to.have.been.calledOnce;
/* eslint-enable no-unused-expressions */
Copy link
Contributor

Choose a reason for hiding this comment

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

You can use just one comment before the statement

    // eslint-disable-next no-unused-expressions

instead of one comment before and one after

@slaweet
Copy link
Contributor

slaweet commented Oct 29, 2018

merge conflict...

slaweet
slaweet previously approved these changes Oct 29, 2018
Copy link
Contributor

@slaweet slaweet left a comment

Choose a reason for hiding this comment

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

Thank you, Ben.

Copy link
Contributor

@Efefefef Efefefef left a comment

Choose a reason for hiding this comment

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

🐛 It rarely works.. try to click this button in a different manner

slaweet
slaweet previously approved these changes Oct 30, 2018
Copy link
Contributor

@Efefefef Efefefef left a comment

Choose a reason for hiding this comment

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

🐛 If you make account balance <0.1 and click Set Max Amount, you have Send button enabled.
Which should be disabled if we dont have enough money to compensate the network fee

Copy link
Contributor

@Efefefef Efefefef left a comment

Choose a reason for hiding this comment

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

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants