Skip to content
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 functionality to share Contact via QR code #1924

Merged
merged 6 commits into from
Jan 12, 2024

Conversation

shubhamkmr04
Copy link
Contributor

No description provided.

@shubhamkmr04 shubhamkmr04 force-pushed the shubham/Contact_sharing branch 2 times, most recently from 490c0a4 to d6881d8 Compare January 3, 2024 14:18
components/CollapsedQR.tsx Outdated Show resolved Hide resolved
utils/handleAnything.ts Outdated Show resolved Hide resolved
@kaloudis
Copy link
Contributor

kaloudis commented Jan 5, 2024

Looking pretty good right now. Are there any backwards compatibility issues we created moving from id to contactId?

@shubhamkmr04
Copy link
Contributor Author

Looking pretty good right now. Are there any backwards compatibility issues we created moving from id to contactId?

Not in my opinion. Tested all features of contacts after this change

@kaloudis
Copy link
Contributor

kaloudis commented Jan 6, 2024

I found some breaking changes moving from id to contactId. You need to either add checks that moves the id to contactId on the contacts or find a different approach here. I think using a zeuscontact: or lncontact: prefix instead will be a better approach.

To see the bug, create contacts with beta2 then switch to this branch. Then try to load a contact. It will crash trying to load the banner because the contact hasn't been loaded at all. If you want to keep those old approach you need changes on Contacts.tsx and ContactDetails.tsx.

Furthermore, the other QRs are still missing prefixes. there should be nostr:, lightning:, and bitcoin: prefixes on the QRs where applicable

…edit remote contacts before saving + layout fix in AddContact
utils/handleAnything.ts Outdated Show resolved Hide resolved
@kaloudis
Copy link
Contributor

Screenshot 2024-01-12 at 00 04 00

let's add more spacing between favorite and qr code here

views/ContactDetails.tsx Outdated Show resolved Hide resolved
views/ContactDetails.tsx Outdated Show resolved Hide resolved
views/ContactDetails.tsx Outdated Show resolved Hide resolved
@@ -0,0 +1,167 @@
import React, { useEffect, useState } from 'react';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like the name of this file ContactInfo can be confused with ContactDetails - something like ContactQRs would be more specific, more appropriate

However, I'm unsure if we're better off transforming the QR view and modifying it to handle multiple QRs than creating a new one here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll convert the name to ContactQR. Would that be okay? And that's a great idea to change the QR view. It definitely might be helpful!

views/ContactDetails.tsx Outdated Show resolved Hide resolved
@@ -14,15 +14,15 @@ import Header from '../components/Header';
import { themeColor } from '../utils/ThemeUtils';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can delete this file now


interface IntroProps {
navigation: any;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you don't need this second interface

@kaloudis kaloudis merged commit 3abb16d into ZeusLN:master Jan 12, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants