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

Link Channel to the stored contacts #2299

Merged

Conversation

shubhamkmr04
Copy link
Contributor

@shubhamkmr04 shubhamkmr04 commented Jul 24, 2024

This PR addresses #2082

It adds functionality to link the remotePubkey in the Channel view to a contact from our stored contacts.

It retrieves contacts from the encrypted storage, matches the remotePubkey with contact pubkey, and displays the contact name next to remotePubkey if a match is found.

Check out the demo picture below addressing the change

Simulator Screenshot - iPhone 15 Pro - 2024-07-29 at 14 43 20

@shubhamkmr04 shubhamkmr04 changed the title Link remote Pubkey from Channel view to the stored contacts Link Channel to the stored contacts Jul 24, 2024
@kaloudis kaloudis added this to the v0.9.1 milestone Jul 24, 2024
@kaloudis
Copy link
Contributor

Can you please add a description of the change and some screenshots or video illustrating the change?

@shubhamkmr04
Copy link
Contributor Author

shubhamkmr04 commented Jul 24, 2024

Can you please add a description of the change and some screenshots or video illustrating the change?

added a description and a demo picture

@@ -295,6 +364,17 @@ export default class ChannelView extends React.Component<
</Text>
</TouchableOpacity>
)}
<Text
Copy link
Contributor

Choose a reason for hiding this comment

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

I really like the display component here but i don't think we need the label. Let's remove Matching Contact Found

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure

locales/en.json Outdated
@@ -432,6 +432,7 @@
"views.Channel.Total.offline": "Total offline",
"views.Channel.zeroConf": "Zero conf",
"views.Channel.commitmentType": "Commitment Type",
"views.Channel.matchingContactFound": "Matching Contact Found",
Copy link
Contributor

Choose a reason for hiding this comment

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

we're no longer using this locale now

@shubhamkmr04 shubhamkmr04 force-pushed the shubham/link-channel-pubkey-to-contact branch from 02b441f to 6997503 Compare August 28, 2024 01:42
@kaloudis
Copy link
Contributor

kaloudis commented Aug 28, 2024

Initial tests look good. We're gonna have to refactor this with the ContactsStore changes though. We'll try to get that in soon.

this.loadContacts();
}

loadContacts = async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please rebase and refactor to use the newly added ContactStore

componentDidMount() {
const { ContactStore } = this.props;
const { loadContacts } = ContactStore;
loadContacts();
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we have to load contacts again on page load? Aren't they accessible in the store already?

Copy link
Contributor Author

@shubhamkmr04 shubhamkmr04 Aug 29, 2024

Choose a reason for hiding this comment

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

yeahh. you're right. lemme change that

@shubhamkmr04 shubhamkmr04 force-pushed the shubham/link-channel-pubkey-to-contact branch from 5b7977c to e82d2c0 Compare August 29, 2024 15:24
Copy link
Contributor

@kaloudis kaloudis left a comment

Choose a reason for hiding this comment

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

tACK

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

Successfully merging this pull request may close these issues.

2 participants