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

CONTACTS #1618

Merged
merged 30 commits into from
Sep 18, 2023
Merged

CONTACTS #1618

merged 30 commits into from
Sep 18, 2023

Conversation

shubhamkmr04
Copy link
Contributor

No description provided.

@shubhamkmr04 shubhamkmr04 force-pushed the shubham/zeus-contacts branch 2 times, most recently from c986d83 to 33991ce Compare August 25, 2023 08:28
@shubhamkmr04 shubhamkmr04 force-pushed the shubham/zeus-contacts branch 2 times, most recently from f31ca0c to 2131fc1 Compare August 30, 2023 15:29

### Other:

- [ ] Changes were made that require an update to the README
- [ ] Changes were made that require an update to onboarding
Copy link
Contributor

Choose a reason for hiding this comment

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

why'd this file get changed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe when I fixed some conflicts, I ran prettier so it is changed

Copy link
Contributor

Choose a reason for hiding this comment

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

Please undo the change

@kaloudis
Copy link
Contributor

kaloudis commented Sep 7, 2023

nit: change AddContacts.tsx -> AddContact.tsx

@kaloudis
Copy link
Contributor

kaloudis commented Sep 7, 2023

is ContactSettings more accurately named Contacts or ContactList?

@shubhamkmr04
Copy link
Contributor Author

is ContactSettings more accurately named Contacts or ContactList?

Maybe Contacts would be better

@@ -2022,4 +2022,4 @@
/* End XCConfigurationList section */
};
rootObject = 83CBB9F71A601CBA00E9B192 /* Project object */;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

this file shouldn't change

"views.Settings.Contacts.searchBar2": "Search",
"views.Settings.Contacts.favorites": "Favorites",
"views.Settings.Contacts.contacts": "Contacts",
"views.Settings.Contacts.noContacts": "No Contacts",
Copy link
Contributor

Choose a reason for hiding this comment

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

this file shouldn't remove the LSP strings

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ahh. didn't see that, might have removed them by mistake

underlayColor="transparent"
/>
);
const AddPhotos = () => (
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: space before this line

color: themeColor('text')
}}
>
Contacts
Copy link
Contributor

Choose a reason for hiding this comment

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

this should be a locale string

{localeString(
'views.Settings.Contacts.favorites'
).toUpperCase()}{' '}
({favoriteContacts.length})
Copy link
Contributor

Choose a reason for hiding this comment

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

let's format this with custom function string formatting

${localeString('views.Settings.Contacts.favorites').toUpperCase() (${favoriteContacts.length})

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Giving me syntax error on this one

{localeString(
'views.Settings.Contacts.contacts'
).toUpperCase()}{' '}
({nonFavoriteContacts.length})
Copy link
Contributor

Choose a reason for hiding this comment

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

same here

console.error(localeString('views.Wallet.Wallet.error'), err)
);
handleDeepLink = (url: string, navigation: any) => {
if (url.startsWith('nostr:')) {
Copy link
Contributor

Choose a reason for hiding this comment

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

why not add nostr: handling to handleAnything?

@shubhamkmr04 shubhamkmr04 force-pushed the shubham/zeus-contacts branch 8 times, most recently from 8d0a366 to 531ab1f Compare September 12, 2023 02:17
@kaloudis
Copy link
Contributor

Let's add Nostr deeplinking functionality to NIP-05s too

@@ -333,6 +335,50 @@ export default class ContactDetails extends React.Component<
)}
</View>
)}
{contact.pubkey.length >= 1 &&
Copy link
Contributor

Choose a reason for hiding this comment

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

on all these checks we also need to see if contact.x exists. otherwise, things will break if/when we add new fields to the contacts

@kaloudis
Copy link
Contributor

tACK

@kaloudis kaloudis merged commit 63cb739 into ZeusLN:master Sep 18, 2023
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.

2 participants