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.

Sign up for GitHub

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

Jump to bottom

feat: revamp example app, implement prelim content types #81

Merged
merged 1 commit into from
Aug 5, 2023
Merged

feat: revamp example app, implement prelim content types #81

merged 1 commit into from
Aug 5, 2023

Conversation

dmccartney
Copy link
Contributor

@dmccartney dmccartney commented Aug 3, 2023 •
edited
Loading

This revamps the example app and adds support and demo of standard content types.
Instead of attempting to use the JS codecs, this takes the approach of relying on the native SDK codecs and then passing a JSON edition of the decoded content back/forth to the JS layer.

This means that adding a new content type looks something like:

  1. design the content type
  2. implement the codec in the native iOS and Android SDKs
  3. add some glue code to this RN SDK to make use of it

This means there will always be a native codec for supported content types. This is important for native-only contexts like handling push notifications.

Here's my rough TODO list for tracking this WIP:

  • add and run prettier
  • revamp the example app UI
  • introduce RN abstraction over content types
    • js
    • android
    • ios
  • add RN bindings for standard content types
    • Text
      • android
      • iOS
    • Reaction
      • android
      • iOS
    • Attachment
      • android
      • iOS
    • Reply
      • android
      • iOS

These will come in a later PR:

  • Composite
    • android
    • iOS
  • RemoteAttachment
    • android
    • iOS

dmccartney
dmccartney commented Aug 3, 2023
View reviewed changes
src/lib/Conversation.ts
Comment on lines -51 to +54
async send(content: any): Promise<string> {
async send(content: string | MessageContent): Promise<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.

gist of the send(...) signature change.

dmccartney
dmccartney commented Aug 3, 2023
View reviewed changes
src/XMTP.types.ts
Comment on lines +36 to +46
// This contains the contents of a message.
// Each of these corresponds to a codec supported by the native libraries.
// This is a one-of or union type: only one of these fields will be present.
export type MessageContent = {
text?: string;
unknown?: UnknownContent;
reply?: ReplyContent;
reaction?: ReactionContent;
attachment?: AttachmentContent;
remoteAttachment?: RemoteAttachmentContent;
};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

gist of the new message content.
We pass one of these to .send(...) and we also receive one whenever we receive a DecodedMessage.

@nplasterer
Copy link
Contributor

I'm not sure if you need the group chat codecs just yet. I think @nakajima might be doing a revamp on how group chat works. Not sure if these will still be relevant.

@dmccartney dmccartney mentioned this pull request Aug 3, 2023
Merged
@dmccartney dmccartney force-pushed the daniel-revamp-example branch 3 times, most recently from b0ccc5f to a048214 Compare August 4, 2023 00:37
@peterferguson
Copy link
Contributor

I'm not sure if you need the group chat codecs just yet. I think @nakajima might be doing a revamp on how group chat works. Not sure if these will still be relevant.

Hey wondering if there is any timeline for group chat features in RN lib?

@nakajima
Copy link
Contributor

nakajima commented Aug 4, 2023

Hey wondering if there is any timeline for group chat features in RN lib?

@peterferguson Not yet.

nplasterer
nplasterer approved these changes Aug 4, 2023
View reviewed changes
Copy link
Contributor

@nplasterer nplasterer left a comment

Choose a reason for hiding this comment

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

All these changes look great. Could be fine to do the rest of the codecs in a follow up PR. Thanks for all the tweaks to the example app as well. :shipit:

@dmccartney dmccartney mentioned this pull request Aug 5, 2023
Merged
@dmccartney dmccartney marked this pull request as ready for review August 5, 2023 04:55
@dmccartney dmccartney requested a review from a team as a code owner August 5, 2023 04:55
@dmccartney dmccartney merged commit 49fe3be into main Aug 5, 2023
1 check failed
@dmccartney dmccartney deleted the daniel-revamp-example branch August 5, 2023 06:08
@dmccartney dmccartney changed the title feat: revamp example app, implement prelim android content types feat: revamp example app, implement prelim content types Aug 5, 2023
@dmccartney
Copy link
Contributor Author

Here are some rough screenshots of the overhauled app UIs:

Screenshot 2023-08-05 at 2 10 19 AM Screenshot 2023-08-05 at 2 09 11 AM

@github-actions
Copy link

🎉 This PR is included in version 1.8.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@dmccartney dmccartney mentioned this pull request Aug 16, 2023
Merged
10 tasks
@daria-github daria-github mentioned this pull request Dec 5, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nplasterer nplasterer nplasterer approved these changes

Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@dmccartney @nplasterer @peterferguson @nakajima