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
[redesign][ln] Receive tab #3537
Conversation
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.
This is going in the right track, but I see some UX issues:
- You can't create a 0 amount invoice anymore. This is needed for "donations" (a 0 amount invoice indicates the sender can send any amount)
- Both canceled and timed out invoices are appearing as "canceled" (not sure that's the intention)
- Maybe we should disable copying the payment request if the payment was canceled/timed out? It will fail if someone attempts to pay for it, so this could make it less prone to cause this error
- Amount in the individual invoice box (in the invoices listing) is showing a "+" sign
- I'd like to get an additional expandable view (either in the details modal or directly in the listing) with the extended info for the invoice (hash, description, htlcs, etc).
- enable 0 amount invoice - disable copying the payment request if the payment was canceled/timed out - remove "+" from the invoice box - show canceled status until the expiry time. - add expandable details part to invoice modal
095768a
to
30f383f
Compare
@matheusd Thanks for your review. I've pushed the suggested changes and updated the screenshots. |
Update: implemented the polished details part of the invoice modal thanks to @MariaPleshkova. (screenshots are updated) |
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.
Also, it's probably useful to have the payment hash be copyable, since that's used in the command line to verify things.
@matheusd @MariaPleshkova Thanks, guys. I pushed the changes and updated the screenshots. |
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.
Working nicely now!
Toward issue #3296
Some notes:
Updated screenshots: