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

Send/Receive Encoded Content #63

Merged
merged 21 commits into from
Jun 30, 2023
Merged

Send/Receive Encoded Content #63

merged 21 commits into from
Jun 30, 2023

Conversation

daria-github
Copy link
Contributor

@daria-github daria-github commented Jun 23, 2023 •
edited
Loading

This PR adds the ability to send and receive encoded content on iOS and Android.
Android -
Screenshot 2023-06-29 at 5 52 24 PM

iOS -

@nplasterer
Copy link
Contributor

I don't think you can do the kotlin modules in the next PR. It needs to be done in this one. This is a breaking change. Sending messages won't work at all in android now because you removed the sendMessage function. Since other people are cutting npm releases in this repo you can't guarantee that a release won't get cut with this change making the android experience entirely broken.

@daria-github
Copy link
Contributor Author

Makes sense, will do this in 1 go — just updated the overview. Thanks!

nakajima
nakajima approved these changes Jun 26, 2023
View reviewed changes
Copy link
Contributor

@nakajima nakajima left a comment

Choose a reason for hiding this comment

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

Left a couple small questions/comments, I think we can punt on the formatting for a follow up PR though to keep this one focused.

example/src/ConversationView.tsx Outdated Show resolved Hide resolved
example/src/test_utils.ts Outdated Show resolved Hide resolved
ios/Wrappers/BinaryDataWrapper.swift Show resolved Hide resolved
ios/Wrappers/BinaryDataWrapper.swift Show resolved Hide resolved
ios/Wrappers/EncodedMessageWrapper.swift Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
@daria-github daria-github marked this pull request as ready for review June 30, 2023 01:02
@daria-github daria-github requested a review from a team as a code owner June 30, 2023 01:02
nplasterer
nplasterer approved these changes Jun 30, 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.

LGTM! Great work on this. I did a quick lint on the android files (removing unused imports and reformatting) and just pushed that in a commit.

@daria-github daria-github merged commit 9fdad14 into main Jun 30, 2023
@daria-github daria-github deleted the dj/messages branch June 30, 2023 15:13
@daria-github daria-github mentioned this pull request Jun 30, 2023
Closed
3 tasks
@github-actions
Copy link
Contributor

🎉 This PR is included in version 1.6.1 🎉

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
Reviewers

@nplasterer nplasterer nplasterer approved these changes

@nakajima nakajima nakajima 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.
3 participants
@daria-github @nplasterer @nakajima