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

Provide the balance dynamically for Confirm (issue 401) #449

Merged
merged 2 commits into from
Sep 10, 2022

Conversation

Yonben
Copy link
Contributor

@Yonben Yonben commented Sep 5, 2022

It seems to be the only thing, but I don't know your codebase like you do ;) Let me now if anything else is needed.

It's a fix for #401 :)

@cstenglein cstenglein linked an issue Sep 5, 2022 that may be closed by this pull request
@cstenglein
Copy link
Collaborator

Thank you very much! Will look into it in the coming days :)

@escapedcat
Copy link
Collaborator

@Yonben
Copy link
Contributor Author

Yonben commented Sep 7, 2022

@escapedcat I added tests, thanks for highlighting that :)
@cstenglein I'm not sure I understand why the ConfirmSendModal uses both amount and invoiceAmount. The former isn't used at all but invoiceAmount is just a reference to amount. Since it supports both invoices and onchain, it seems the more generic name amount is a better fit here.

What do you think?

@cstenglein
Copy link
Collaborator

@Yonben yeah, looks like I started it sometime but didn't finish it. Just using amount would be the better idea here.

Anyways, the PR looks good & you even wrote tests, thank you very much 👍

@cstenglein cstenglein merged commit f5ca753 into raspiblitz:master Sep 10, 2022
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.

ConfirmModal with on-chain: always "amount too high" error message
4 participants