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

qml invoice dialog, onchain payment, cannot click Pay while editing amount #8252

Closed
accumulator opened this issue Mar 16, 2023 · 9 comments
Closed
Assignees
Labels

Comments

@accumulator
Copy link
Member

allow click Pay while editing amount (if valid)

@ecdsa
Copy link
Member

ecdsa commented Mar 17, 2023

Another issue with the current buttons is that the user is allowed to edit the amount, even if the invoice contains an amount.
With the current GUI, I can receive a lightning invoice for a certain amount and modify that amount. I have not tested what happens if I click pay, but I guess nothing good :-)

The amount should be frozen if the invoice contains an amount, editable otherwise. There is no need for buttons that enable and disable editing.

@accumulator
Copy link
Member Author

accumulator commented Mar 17, 2023

The problem is, once the amount is edited, it is stored in the invoice. So after setting an amount, it would become uneditable, even if the original invoice had no amount specified (e.g. address only 'invoice', no-amount LN invoice, etc).

We can work around it and keep the original amount around, but only as long as the wallet is open.

@ecdsa
Copy link
Member

ecdsa commented Mar 17, 2023

I don't understand what you mean...
I can still edit non-zero lightning amount after this commit: a571451
it does not seem to have an effect on the saved invoice, but that is not the point.
the point is: why would we want to let users edit that amount at all?

@accumulator
Copy link
Member Author

I can still edit non-zero lightning amount after this commit: a571451

Yes that commit is not blocking edits, it is a fix for this issue. We should create a separate issue for your additional comment.

it does not seem to have an effect on the saved invoice, but that is not the point. the point is: why would we want to let users edit that amount at all?

scan address, amount is unspecified. Edit amount, save invoice -> amount is fixed.

@ecdsa
Copy link
Member

ecdsa commented Mar 17, 2023

Maybe I was not clear, but I do understand that it is useful to edit amounts when no amount is specified. this is not the point...

The reason why I did not open a separate issue is, I do not see why the editing widgets are useful at all. Their purpose is to switch between editing mode and non-editing mode. But my point is that there is no reason to ever switch between those modes. Thus, I think a deeper rework of InvoiceDialog is needed, that would remove all those buttons, and probably fix all issues at once.

@accumulator
Copy link
Member Author

I can remove the edit mode toggle and just present the invoice amount OR the edit mode if no amount is specified.
However my point is what to do with the Save button. Do we just save the original invoice, or do we want to save with the user-entered amount?

@ecdsa
Copy link
Member

ecdsa commented Mar 17, 2023

we should save the entered amount if the original invoice did not have an amount.

@accumulator
Copy link
Member Author

see 24a3d6e
not yet saving user entered amount

@ecdsa
Copy link
Member

ecdsa commented Mar 18, 2023

not yet saving user entered amount

I will try to fix that. any issue I should know?

@ecdsa ecdsa closed this as completed in d7c5c40 Mar 18, 2023
@ecdsa ecdsa self-assigned this Mar 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants