-
Notifications
You must be signed in to change notification settings - Fork 46
chat docs: fix missing chat kotlin/swift/jetpack doc #3098
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
base: feature/chat-jetpack-compose-docs
Are you sure you want to change the base?
chat docs: fix missing chat kotlin/swift/jetpack doc #3098
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis PR adds comprehensive multi-language code examples for media sharing, message replies, and message reactions across the chat documentation, while correcting a Swift API parameter label in the message history section from Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
207d1c0 to
58c0968
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/pages/docs/chat/rooms/replies.mdx`:
- Around line 66-99: The React reply example uses "createdAt" for the reply
metadata while Swift/Kotlin/JS use "timestamp"; update the React example so the
metadata key is "reply.timestamp" (replace createdAt with timestamp) to match
other language examples and ensure cross-platform consistency, and while here
confirm that the Kotlin usage of replyToMessage.timestamp is in milliseconds (or
convert it to ms like Swift does) so all examples supply timestamp as epoch
milliseconds.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
src/pages/docs/chat/rooms/history.mdxsrc/pages/docs/chat/rooms/media.mdxsrc/pages/docs/chat/rooms/message-reactions.mdxsrc/pages/docs/chat/rooms/replies.mdx
🔇 Additional comments (11)
src/pages/docs/chat/rooms/history.mdx (1)
57-57: LGTM!The Swift parameter label correction from
withOptionstowithParamsaligns with the SDK documentation URL referenced on line 11 (history(withparams:)).src/pages/docs/chat/rooms/message-reactions.mdx (1)
318-352: LGTM!The Swift and Kotlin examples for removing message reactions are consistent with the existing send reaction patterns earlier in the file (lines 122-162) and mirror the JavaScript/React examples.
src/pages/docs/chat/rooms/replies.mdx (3)
170-202: LGTM!The
prepareReplyhelper functions are consistent across Swift, Kotlin, and Jetpack, properly extracting and formatting parent message data for reply metadata.
233-275: LGTM!The
fetchParentMessageimplementations properly handle serial extraction with appropriate error handling for each platform (Swift usesNSError, Kotlin throwsIllegalArgumentException).
337-427: LGTM!The display reply implementations correctly handle metadata extraction using platform-appropriate patterns (Swift pattern matching, Kotlin null-safe operators). The Jetpack Compose example properly manages subscription lifecycle with
DisposableEffect.src/pages/docs/chat/rooms/media.mdx (6)
80-148: LGTM!The
MediaDatastruct andonMediaAttachflow implementations are consistent across Swift, Kotlin, and Jetpack Compose, properly managing the media attachment state.Note: The
uploadMedia()function referenced in Swift/Kotlin/Jetpack is assumed to be a placeholder that users implement, matching the pattern established in the JavaScript example at lines 29-44.
214-329: LGTM!The send message implementations properly construct JSON metadata containing the media array using platform-appropriate patterns (Swift
JSONValueenums, KotlinbuildJsonObject/buildJsonArray).
366-450: LGTM!The
getValidMediaimplementations properly validate media IDs using regex and safely extract typed values from JSON metadata. The validation approach is consistent with security best practices mentioned in the documentation.
515-623: LGTM!The display implementations demonstrate platform-appropriate UI patterns. The Swift (UIKit) and Kotlin (Android Views) examples include helpful comments indicating where image loading libraries should be integrated, while the Jetpack Compose example shows a complete implementation using Coil's
AsyncImage.
669-780: LGTM!The
addMediaToMessageimplementations correctly handle metadata mutation by preserving existing metadata while adding new media entries. The Kotlin/Jetpack approach of rebuilding the metadata object maintains immutability.
829-947: LGTM!The
removeMediaFromMessageimplementations properly filter out the target media ID while preserving other metadata. Early returns for empty media arrays prevent unnecessary API calls.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
57c3e71 to
a72f25d
Compare
…lin snippets - Added uploadMedia method to swift/kotlin and jetpack
a72f25d to
520c5c6
Compare
maratal
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a few questions.
src/pages/docs/chat/rooms/media.mdx
Outdated
| func send(text: String, mediaToAttach: [MediaData]) async throws { | ||
| var metadata: MessageMetadata = [:] | ||
| if !mediaToAttach.isEmpty { | ||
| let mediaArray: [JSONValue] = mediaToAttach.map { media in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This snippet is complicated and not welcoming. I think a single object is enough as an example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, will check for simplifying this 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
src/pages/docs/chat/rooms/media.mdx
Outdated
| suspend fun send(text: String, mediaToAttach: List<MediaData>) { | ||
| val metadata = if (mediaToAttach.isNotEmpty()) { | ||
| buildJsonObject { | ||
| put("media", buildJsonArray { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
src/pages/docs/chat/rooms/media.mdx
Outdated
| ``` | ||
|
|
||
| ```swift | ||
| import UIKit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't it better to stick to SwiftUI in all new examples?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will check 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
6f59c3b to
bef8be6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds missing documentation for chat-swift, chat-kotlin, and Jetpack Compose SDKs, fixing incomplete code examples across multiple chat features. The changes also correct an API parameter naming inconsistency in Swift examples.
Changes:
- Updated Swift API parameter name from
withOptionstowithParamsfor consistency - Added comprehensive Swift, Kotlin, and Jetpack Compose code examples for media attachments
- Added Swift, Kotlin, and Jetpack Compose examples for message replies functionality
- Added Swift and Kotlin examples for removing message reactions
- Corrected Swift code examples to include
trykeyword for error handling
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| history.mdx | Updated Swift history query API from withOptions to withParams |
| message-reactions.mdx | Fixed Swift syntax, added Swift/Kotlin reaction deletion examples, corrected parameter names |
| media.mdx | Added comprehensive Swift, Kotlin, and Jetpack examples for media upload, validation, and display |
| replies.mdx | Added complete Swift, Kotlin, and Jetpack examples for sending and displaying message replies |
Comments suppressed due to low confidence (1)
src/pages/docs/chat/rooms/message-reactions.mdx:651
- This Swift code uses the outdated parameter name
withOptionsinstead ofwithParams. This is inconsistent with the fix made in history.mdx and other Swift examples in this PR. ChangewithOptionstowithParamsfor API consistency.
var messages = (await room.messages.history(withOptions: .init(limit: 50))).items
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…andards - Fixed swift snippet method signatuee for messages.history in message-reactions
bef8be6 to
a6181a1
Compare
maratal
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Few suggestions
| func send(text: String, mediaToAttach: [JSONObject]) async throws { | ||
| var metadata: MessageMetadata = [:] | ||
| if !mediaToAttach.isEmpty { | ||
| metadata["media"] = .array(mediaToAttach.map { .object($0) }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I beleive that some commentary of what .array(mediaToAttach.map { .object($0) }) means wouldn't be unuseful for reader.
| return nil | ||
| } | ||
|
|
||
| let range = NSRange(location: 0, length: id.utf16.count) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not just id.count?
| ```swift | ||
| import SwiftUI | ||
|
|
||
| struct MessageView: View { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally, this should be checked in Xcode, since someone will copy this example and try to run it.
| var newMetadata = message.metadata | ||
|
|
||
| var mediaArray: [JSONValue] | ||
| if case let .array(existingArray) = newMetadata["media"] { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it can be
var mediaArray: [JSONValue] = if case let .array(existingArray) = newMetadata["media"] {
existingArray
} else {
[]
}
| if let replyMetadata = message.metadata["reply"], | ||
| case let .object(replyData) = replyMetadata { | ||
| if case let .string(replySerial) = replyData["serial"] { | ||
| if let parentMessage = localMessages.first(where: { $0.serial == replySerial }) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This and four lines above are unreadable, better to move them into Message extension method message.getReplyToSerial() or something.
Summary by CodeRabbit
Documentation
withOptionsrenamed towithParamsfor chat history queries✏️ Tip: You can customize this high-level summary in your review settings.