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

Refactors so TableInvoice takes the total fee #2785

Merged

Conversation

thebkr7
Copy link
Contributor

@thebkr7 thebkr7 commented Jun 27, 2019

Closes #28 for the Extension Repo

Description:

Refactors so TableInvoice takes the total fee rather than taking two numbers as props and calculating it there.

Thank you! 🚀


For contributor:

  • Added changes entries. Run yarn changelog for a guided process.
  • Reviewed Files changed in the github PR explorer
  • Attach screenshots of the UI components on the PR description (if applicable)
  • Scope of work approved for big PRs

For reviewer:

  • Manually tested the changes on the UI

jbibla
jbibla previously approved these changes Jun 28, 2019
@thebkr7 thebkr7 marked this pull request as ready for review July 2, 2019 20:38
@thebkr7 thebkr7 requested review from colw and faboweb as code owners July 2, 2019 20:38
@thebkr7
Copy link
Contributor Author

thebkr7 commented Jul 2, 2019

Will change destination branch to develop once the chrome-extension branch has been merged.

@thebkr7
Copy link
Contributor Author

thebkr7 commented Jul 2, 2019

Refactoring TableInvoice accepting BondDenom as a prop based on comment: https://github.com/luniehq/lunie-browser-extension/issues/30#issuecomment-507854375

@thebkr7
Copy link
Contributor Author

thebkr7 commented Jul 3, 2019

The above refactor of TableInvoice was completed in PR #2802

@thebkr7 thebkr7 changed the base branch from chrome-extension to develop July 3, 2019 18:25
@@ -98,8 +98,7 @@ exports[`ActionModal should show the action modal when user has logged in with e

<tableinvoice-stub
amount="0"
gasestimate="0"
gasprice="2.5e-8"
estimatedfee="0"
Copy link
Collaborator

Choose a reason for hiding this comment

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

should this be 0?

Copy link
Collaborator

Choose a reason for hiding this comment

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

when the estimate was 0 before then yes ;)

@faboweb faboweb merged commit 0be13cc into develop Jul 4, 2019
@faboweb faboweb deleted the Benji/Ext-Issue-28-TableInvoice-receive-final-amount branch July 4, 2019 08:46
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