feat: modify receive screen for wallets without ln address and set up #224
Conversation
| @@ -77,92 +35,11 @@ export function Receive() { | |||
| }); | |||
| } | |||
|
|
|||
| // TODO: move this somewhere else to have app-wide notifications of incoming payments | |||
There was a problem hiding this comment.
Maybe we can address this to show a toast whenever we get a payment? because received payments via lightning address will not show any more - we are removing a feature here.
There was a problem hiding this comment.
Then we can make this provider which keeps pushing notifications via toast in-app (everywhere, not just receive), will add a commit
There was a problem hiding this comment.
Removing this since notifications PR is now ready to merge
There was a problem hiding this comment.
@im-adithya isn't this something different? it's not related to push notifications is it?
There was a problem hiding this comment.
My thinking was, if someone wants to receive notifications, they would enable from settings. And if they already have push notifications it doesn't make sense to see two notifications (one in-app toast and one native notification) for the same payment.
Also for those who don't have push notifications, we would be showing them without being prompted if they want it or not. And react-native-toast-message can't show multiple toasts at once, so it might interfere with other toasts. Best to remove this completely and keep it simple. Wdyt?
There was a problem hiding this comment.
we would be showing them without being prompted if they want it or not
What do you mean by this?
Can we re-review this, I think at least on the page if you generate an invoice and are expecting a payment, you would do this and want to actively see a confirmation on Alby Hub (e.g. a PoS flow).
If you don't manually generate an invoice and only show the lightning address, this could be different.
I'm not sure about the toasts, but I think we should think this through properly before merging it.
There was a problem hiding this comment.
We are proceeding with removing these for now as we think notifications can bridge this gap.
There was a problem hiding this comment.
Opened a follow up: #266 cc @reneaaron
im-adithya
changed the title
Description
Fixes #255
Screenshots