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

Push notifications #27

Merged
merged 16 commits into from
May 5, 2023
Merged

Push notifications #27

merged 16 commits into from
May 5, 2023

Conversation

nplasterer
Copy link
Contributor

@nplasterer nplasterer commented May 4, 2023

Android part of #22

Rewatch.Screen.Recording.-.2023-05-04.at.4.07.09.PM.mp4

Will do iOS in a follow up.

@nplasterer nplasterer self-assigned this May 4, 2023
@nplasterer nplasterer requested a review from a team as a code owner May 4, 2023 15:34
example/src/PushController.tsx Outdated Show resolved Hide resolved
example/android/app/google-services.json Outdated Show resolved Hide resolved
@nplasterer nplasterer marked this pull request as draft May 4, 2023 15:50
@nplasterer nplasterer marked this pull request as ready for review May 5, 2023 00:01
@nplasterer nplasterer requested a review from jhaaaa as a code owner May 5, 2023 00:01
@nplasterer nplasterer requested a review from a team May 5, 2023 00:01
Comment on lines +212 to +223

Function("registerPushToken") { (pushServer: String, token: String) in
// TODO
}

Function("subscribePushTopics") { (topics: [String]) in
// TODO
}

AsyncFunction("decodeMessage") { (topic: String, encryptedMessage: String, conversationID: String?) in
// TODO
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The spacing looks normal on in my VSCode and XCode so not sure why it looks like this here.

Copy link
Contributor

Choose a reason for hiding this comment

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

tabs v spaces

@@ -92,7 +95,7 @@ class XMTPModule : Module() {
Name("XMTP")
Events("sign", "authed", "conversation", "message")

Function("address") {
Function("address") { clientAddress: String ->
Copy link
Contributor Author

Choose a reason for hiding this comment

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

App wouldn't compile unless I added these but haven't hooked up the testing stuff to actually use them.

Copy link
Contributor

@jhaaaa jhaaaa left a comment

Choose a reason for hiding this comment

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

Thank you for this amazing engineering, @nplasterer - and also for taking the time to update the README! So grateful -- LGTM!

@@ -18,6 +18,7 @@ export default function ConversationListView({

async function refreshConversations() {
const conversations = await client.conversations.list();
XMTPPush.subscribe(conversations.map((c) => c.topic));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It only subscribes once you refresh the list view. Not super great at react couldn't figure out how to get it to subscribe on streaming conversations but this is a minor detail and we can improve it later.

Comment on lines +212 to +223

Function("registerPushToken") { (pushServer: String, token: String) in
// TODO
}

Function("subscribePushTopics") { (topics: [String]) in
// TODO
}

AsyncFunction("decodeMessage") { (topic: String, encryptedMessage: String, conversationID: String?) in
// TODO
}
Copy link
Contributor

Choose a reason for hiding this comment

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

tabs v spaces

@nplasterer nplasterer merged commit cc2f177 into main May 5, 2023
@nplasterer nplasterer deleted the np/expo-push-test branch May 5, 2023 17:18
@github-actions
Copy link
Contributor

🎉 This PR is included in version 1.0.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants