-
Notifications
You must be signed in to change notification settings - Fork 46
chat: add Jetpack Compose support to documentation #2991
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
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 Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 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 |
- Added examples showcasing the use of Jetpack Compose APIs in the chat SDK. - Updated documentation to include Jetpack Compose methods such as `collectAsPagingMessagesState()`, `collectAsCurrentlyTyping()`, `collectAsStatus()`, and others. - Extended sections with Jetpack Compose-specific code snippets for message reactions, typing events, room management, and more.
d33713d to
3d7feab
Compare
| label: 'Kotlin', | ||
| syntaxHighlighterKey: 'kotlin', | ||
| }, | ||
| jetpack: { |
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 added these as a fixup so that it shows up in docs - could you source an icon for Jetpack so it shows in the language selector?
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 have added alias to kotlin, so it will use kotlin icon for now
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.
Created PR to add icon -> ably/ably-ui#1023
| ``` | ||
|
|
||
| ```jetpack | ||
| import com.ably.chat.ChatClient |
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.
The indentation on the code examples are different between Kotlin and Jetpack in many places (despite being the same code) - can we fix 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.
Fixed -> b8cf371
| </Code> | ||
| </If> | ||
|
|
||
| ## Handle connection discontinuity <a id="discontinuity"/> |
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.
We have discontinuityAsFlow - should that be mentioned here?
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.
Fixed -> b8cf371
| </If> | ||
|
|
||
| <If lang="jetpack"> | ||
| For Jetpack Compose, use the [`collectAsPagingMessagesState()`](https://sdk.ably.com/builds/ably/ably-chat-kotlin/main/jetpack/chat-extensions-compose/com.ably.chat.extensions.compose/collect-as-paging-messages-state.html) composable function to observe messages with automatic pagination support. Alternatively, you can use [`messages.subscribe()`](https://sdk.ably.com/builds/ably/ably-chat-kotlin/main/dokka/chat/com.ably.chat/-messages/subscribe.html) for a simple subscription: |
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.
We should avoid saying "For Jetpack Compose", as the language selector filters all this out. It should just be "Use the..."
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.
Fixed -> b8cf371
| <If lang="javascript,kotlin"> | ||
| Use the `unsubscribe()` function returned in the `subscribe()` response to remove a chat message listener: | ||
| <If lang="javascript,kotlin,jetpack"> | ||
| Use the `unsubscribe()` function returned in the `subscribe()` response to remove a chat message listener<If lang="jetpack">. When using Jetpack Compose's `collectAsPagingMessagesState()`, lifecycle and cleanup are handled automatically</If>: |
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 think we can remove this extra sentence, because someone using jetpack won't be expecting to use it? So we just say:
"collectAsPagingMessagesState() handles lifecycle and cleanup automatically"
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.
Fixed -> b8cf371
|
|
||
| ### Occupancy event structure | ||
|
|
||
| The following is the structure of an occupancy event: |
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.
We need to fix this for Kotlin and Swift
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.
added for kotlin,swift and jetpack
|
|
||
| <If lang="javascript,kotlin"> | ||
| Use the `unsubscribe()` function returned in the `subscribe()` response to remove a room occupancy listener: | ||
| <If lang="javascript,kotlin,jetpack"> |
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.
Ditto re Jetpack having its own if block rather than tacking on the end of kotlin
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.
jetpack block has been removed, since it automatically handles unsubscribe as a part of the component lifecycle.
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.
Removed the block
| ```jetpack | ||
| // Jetpack Compose handles cleanup automatically | ||
| // When using subscribe directly: | ||
| val (unsubscribe) = room.occupancy.subscribe { event -> |
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.
Ditto re whether we should be encouraging manual subscription with jetpack
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.
Removed the code, it's not needed similar to swift
|
|
||
| Note that if sending two identical reactions of type `Distinct`, the second one will be accepted and broadcast as a raw reaction, but it will be ignored in the summary (aggregate). Similarly, when removing a reaction that doesn't exist (of any type), the operation will be accepted and broadcast as a raw reaction, but it will have no effect on the summary. | ||
|
|
||
| ### Configure the default reaction type <a id="default-type"/> |
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.
Needs a jetpack
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.
Added -> 37360d5
|
|
||
| ## Messages and reactions <a id="messages-and-reactions"/> | ||
|
|
||
| The `Message` object contains a `reactions` property which is an object that looks like 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.
Needs Kotlin+Swift section
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.
Added -> 37360d5
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 pull request adds comprehensive Jetpack Compose support to the Ably Chat SDK documentation. It introduces a new "jetpack" language option alongside existing Kotlin examples, providing developers with Compose-specific APIs and patterns for building chat applications with reactive state management.
Changes:
- Added "jetpack" as a new language option in the documentation system with appropriate configuration and syntax highlighting
- Extended all major chat feature documentation pages with Jetpack Compose code examples showcasing composable functions and state management
- Fixed several existing typos and syntax errors in Kotlin code examples (spelling corrections and bracket issues)
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 33 comments.
Show a summary per file
| File | Description |
|---|---|
src/data/languages/types.ts |
Added 'jetpack' to the list of supported language keys |
src/data/languages/languageInfo.ts |
Configured Jetpack Compose with label and Kotlin syntax highlighting |
src/data/languages/languageData.ts |
Added version 1.0 for jetpack in chat SDK |
src/pages/docs/chat/setup.mdx |
Added Jetpack Compose dependency installation and client instantiation examples |
src/pages/docs/chat/rooms/typing.mdx |
Added composable examples for typing indicators using collectAsCurrentlyTyping() |
src/pages/docs/chat/rooms/reactions.mdx |
Added composable examples for room reactions and fixed "reaciton" typo |
src/pages/docs/chat/rooms/presence.mdx |
Added composable examples for presence management with collectAsPresenceMembers() |
src/pages/docs/chat/rooms/occupancy.mdx |
Added composable examples for occupancy tracking with collectAsOccupancy() |
src/pages/docs/chat/rooms/messages.mdx |
Added composable examples for messages with collectAsPagingMessagesState() and fixed broken markdown link |
src/pages/docs/chat/rooms/message-reactions.mdx |
Added composable examples for message reactions and fixed syntax errors with extra parentheses |
src/pages/docs/chat/rooms/index.mdx |
Added composable examples for room status monitoring with collectAsStatus() |
src/pages/docs/chat/rooms/history.mdx |
Added composable examples for message history with pagination support |
src/pages/docs/chat/connect.mdx |
Added composable examples for connection status monitoring |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
b8cf371 to
0293510
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
Copilot reviewed 13 out of 13 changed files in this pull request and generated 14 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
27b058c to
8e8d3bd
Compare
…be method - Added missing Room imports for occupancy and presence - Fixed android.mdx to use `historyBeforeSubscribe` method instead of `getPreviousMessages`
8e8d3bd to
1abfd27
Compare
# Conflicts: # src/pages/docs/chat/rooms/messages.mdx
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
Copilot reviewed 14 out of 14 changed files in this pull request and generated 14 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
rather than imports
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
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
… merged into ably-ui - Bumped up kotlin and jetpack version to 1.1
56f98c6 to
4e71d12
Compare
|
Closing in favor of #3097 |
Description
collectAsPagingMessagesState(),collectAsCurrentlyTyping(),collectAsStatus(), and others.Checklist