feat: deeplinks for wallets #378
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
kilrau
requested changes
Jan 9, 2024
There was a problem hiding this comment.
Desktop:
- For Reverse Swaps on Desktop I don't think it makes sense to have the "Open in Wallet" button. It opens Alby, which already has the
Pay invoice via WebLNbutton. https://github.com/LN-Zap/zap-desktop is archived, I don't think there is any other popular desktop lightning wallets that could make use of that button. - For Normal Swaps, on my ubuntu system the handler couldn't find my Green wallet to open
liquidnetwork:, I openen an issue on green side. For bitcoin mainchain the handler listed others
but then send dialogue and amounts couldn't be prefilled.
Summary: not very useful on Desktop, if you insist let's keep it for normal swaps only and we hope that Green & other wallets improve in future. I am also fine removing it completely for desktop.
Mobile:
Works beautifully in all wallets I tested. Everything prefilled as it should be, even in Green.
kilrau
requested changes
Jan 10, 2024
There was a problem hiding this comment.
Reverse Swaps:
- So now
Pay invoice via WebLNonly applies to Desktop,Open in Walletonly applies for Mobile. I think we should unify style! Blue button on top vs both yellow next to each other. Don't care which, but it should be the same.Scan QR Codeon mobile is blue on top, same style asPay invoice via WebLNon Desktop.
Normal Swaps:
- Mobile is missing
Copy Addressnow. Which is only acceptable if we add a copy symbol into the address filed so it's discoverable that the address can be copied by tapping on it. I just really don't like that there are 3 equally looking buttons on mobile and desktop doing different things.
The "blue button on top" style for Open in Wallet can work if you agree doing #444
|
Since we are now free to do #444, we can use this button style for "open in wallet" on mobile. |
add links on SwapCreated and InvoiceSet on the QR code and as button closes #299 change copy addres to copy bip21
|
closes #444 |
michael1011
approved these changes
Jan 23, 2024
kilrau
approved these changes
Jan 23, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
add links on SwapCreated and InvoiceSet on the QR code and as button.
its working for alby on browser for me i did not test bip21 yet.
closes #299
i replaced the copy bip21 for the open in wallet button, usually you don't really want to copy bip21 right? else we get a small ui issue because we have to less room to make it look good.