-
Notifications
You must be signed in to change notification settings - Fork 195
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
feat: lnurlpay from options #448
Conversation
hmm... now we have two different behaviors: |
I switched back to just using hashrouter to have more unified code. Besides that /confirmPayment will now also be opened within the popup or options page just like lnurlPay. |
I'm only not so happy with: https://github.com/getAlby/lightning-browser-extension/pull/448/files#diff-08ea01d483ec3b202dd5a87e72ff940500dff987205a9ba133502db72c7425e5R236 where I send originData as a search param. |
@dylancom any idea how it could make you happy? we also need to refactor that soon because in that payment flow we do not really have origin data - and it is just a payment to a ln invoice or lnurl. |
@bumi we only use name & icon, so we could just send those as a search param directly. Or store scraped twitter data on the window object and grab it from there, or call getLightingData again from the lnurlPay screen. I guess sending 2 extra search params (name & icon) is the easiest. |
not sure if I understand you correctly. that flow is used from several entrypoints: paying to an invoice, paying to a lightning address, tipping a twitter profile. The origin data holds the metadata that we add to the transactions, doesn't it? - which is more than just the name and icon. |
@bumi oops my bad, looked too quickly itm at what we could possibly do. |
Solves #445
Seemed not as easy as just adding a route.
This is because the extension is using MemoryRouter and the Options page is using HashRouter.
MemoryRouter can pass state to a new screen, HashRouter has to do this by querystring.
Then I found out that the LNURLDetails type was incorrect :)