-
-
Notifications
You must be signed in to change notification settings - Fork 152
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
Add share feature #2728
base: master
Are you sure you want to change the base?
Add share feature #2728
Conversation
68f7439
to
e437b1e
Compare
e437b1e
to
8439520
Compare
Rebased on #2733 (should be merged first). |
…QR with icon-only mode
8439520
to
1421d1b
Compare
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.
Interesting. No idea why that could be as usage is pretty simple:
It works elsewhere on iOS though? (e.g. via Receive->Unified Invoice) |
No It happens at all the places where QR can be shared, android worked perfect though |
No I mean, is the "Z" logo displayed in the normal QR codes on iOS? (e.g. Receive->Unified Invoice) I'm asking because I have no idea what could be the root cause for this, if it is only happening for the shared QR. |
yup its displayed in the normal QR codes generated in iOS |
:/ Ok. Can you maybe help me find out what the difference is? I cannot debug on iOS... |
Sure, I'll take a look at the code and let you know what we can do. |
} | ||
|
||
const svgElement = qrRef.current; | ||
const base64Data = await captureRef(svgElement, { |
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.
@myxmaster It looks like the QR logo issue on iOS happens because the capture runs before the logo fully loads. Try adding a 500ms delay here — should fix it!
await new Promise((resolve) => setTimeout(resolve, 500));
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.
Ah ok, thanks for looking into it! I will try to fix it, if possible without a static timeout.
Description
This fixes #1285.
This adds the ability to share invoices (and on-chain addresses and lightning-addresses) via OS share dialog. The shared content includes both QR and text representation. Unfortunately it depends on the app which is used for sharing if both is actually used.
Implementation details:
I was playing around a lot, and ended up with this current approach. It looks kind of complicated, but to be able to share the QR it actually has to be rendered and captured (at least I didn't find another way). I wanted to avoid rendering a 2nd hidden black/white version of the QR whenever an invoice is generated, so I ended up with this logic:
This pull request is categorized as a:
Checklist
yarn run tsc
and made sure my code compiles correctlyyarn run lint
and made sure my code didn’t contain any problematic patternsyarn run prettier
and made sure my code is formatted correctlyyarn run test
and made sure all of the tests passTesting
If you modified or added a utility file, did you add new unit tests?
I have tested this PR on the following platforms (please specify OS version and phone model/VM):
I have tested this PR with the following types of nodes (please specify node version and API version where appropriate):
Locales
Third Party Dependencies and Packages
yarn
after this PR is merged inpackage.json
andyarn.lock
have been properly updatedOther: