Skip to content

Replace FundError with InvoiceModal#455

Merged
huumn merged 1 commit into
stackernews:masterfrom
ekzyis:405-combine-fund-wallet-and-pay-invoice
Aug 31, 2023
Merged

Replace FundError with InvoiceModal#455
huumn merged 1 commit into
stackernews:masterfrom
ekzyis:405-combine-fund-wallet-and-pay-invoice

Conversation

@ekzyis
Copy link
Copy Markdown
Member

@ekzyis ekzyis commented Aug 29, 2023

  • invoices are no longer deleted to prevent double-spends but marked as confirmed.
    therefore, during checkInvoice, we also check if the invoice is already confirmed.
  • instead of showing FundError (with "fund wallet" and "pay invoice" as options), we now always immediately show an invoice
  • since flagging, paying bounties and poll voting used FundError but only allowed spending from balance, they now also support paying per invoice

Based upon #432, so #432 should be merged first

Closes #405

@ekzyis ekzyis added the enhancement improvements to existing features label Aug 29, 2023
@ekzyis
Copy link
Copy Markdown
Member Author

ekzyis commented Aug 31, 2023

I'll rebase this now since #432 was merged

* invoices are no longer deleted to prevent double-spends but marked as confirmed.
  therefore, during checkInvoice, we also check if the invoice is already confirmed.
* instead of showing FundError (with "fund wallet" and "pay invoice" as options), we now always immediately show an invoice
* since flagging, paying bounties and poll voting used FundError but only allowed spending from balance, they now also support paying per invoice
@ekzyis ekzyis force-pushed the 405-combine-fund-wallet-and-pay-invoice branch from 367f183 to 6033f2d Compare August 31, 2023 04:30
@huumn huumn merged commit 803acd1 into stackernews:master Aug 31, 2023
@huumn
Copy link
Copy Markdown
Member

huumn commented Aug 31, 2023

I'm not sure the info message actually scratches the issue's itch. It's just kind of telling people they can overpay an invoice which most wallets don't support and most people aren't aware of (not to mention there are limitations on how much you can overpay an invoice). So I suspect this message is more confusing than helpful.

After seeing this invoicable stuff live for a bit, I don't think we need the ability to fund an account from these modals. It might not be worth the UX bloat to even do it how I imagined it and people can always put sats in their wallet if they want to.

I'm going to remove the info message for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement improvements to existing features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Combine "fund wallet" + "pay invoice"

2 participants