Use component factory when rendering message attachments#6133
Conversation
SDK Size Comparison 📏
|
gpunto
force-pushed
the
redesign/message-restructure-4
branch
from
6591c42 to
6ca3134
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughThis PR reorganizes attachment rendering by removing the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/theme/ChatComponentFactory.kt (1)
2892-2998:⚠️ Potential issue | 🟡 MinorAdd thread/state notes to the new public KDocs.
The new attachment content entry points are public APIs and should include thread expectations and state notes.
📝 Example update (apply similarly to the other new methods)
/** * Factory method for creating the content of audio recording attachments in a message. * * `@param` state State providing the context needed to render and handle interactions for the attachment. * `@param` modifier Modifier for styling. + * + * Threading: Called from the Compose UI thread. + * State: [state] represents the current message/attachment snapshot. */As per coding guidelines, "Document public APIs with KDoc, including thread expectations and state notes".
🤖 Fix all issues with AI agents
In
`@stream-chat-android-docs/src/main/kotlin/io/getstream/chat/docs/kotlin/compose/guides/AddingCustomAttachments.kt`:
- Around line 171-178: The CustomAttachmentContent override in
CustomComponentFactory currently returns nothing when no "date" attachments are
present, suppressing the base behavior; update CustomAttachmentContent to call
the base implementation (super.CustomAttachmentContent(state, modifier)) when
the date-type check fails (e.g., in an else branch or after the if) so
UnsupportedAttachmentContent from the parent is rendered for non-date
attachments.
In
`@stream-chat-android-ui-guides/src/main/java/io/getstream/chat/android/guides/catalog/compose/customattachments/CustomChatComponentFactory.kt`:
- Around line 45-54: Update the KDoc for CustomChatComponentFactory to reference
ChatComponentFactory (not AttachmentFactory) and include the required public-API
notes about thread expectations and state handling: document that
CustomAttachmentContent is a composable override of
ChatComponentFactory.CustomAttachmentContent, describe the thread (UI/main) and
composable invocation expectations, and add a short note about how
AttachmentState is used/observed (e.g., immutable snapshot, do not mutate).
Ensure references to CustomChatComponentFactory, CustomAttachmentContent,
ChatComponentFactory, AttachmentState and DateAttachmentContent are present in
the KDoc and that the comment sits directly above the object declaration.
🧹 Nitpick comments (3)
stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/components/messages/MessageContent.kt (1)
176-180: Route upload rendering throughChatTheme.componentFactoryfor consistency.Right now uploads bypass the component factory, which makes upload rendering non-customizable compared to the other attachment types wired here. Consider delegating through
componentFactory.FileUploadContent(...)instead.♻️ Suggested change
- FileUploadContent( - modifier = Modifier, - attachmentState = attachmentState, - ) + componentFactory.FileUploadContent( + modifier = Modifier, + attachmentState = attachmentState, + onItemClick = ::onFileUploadContentItemClick, + )-import io.getstream.chat.android.compose.ui.attachments.content.FileUploadContent +import io.getstream.chat.android.compose.ui.attachments.content.onFileUploadContentItemClickstream-chat-android-docs/src/main/kotlin/io/getstream/chat/docs/kotlin/compose/guides/AddingCustomAttachments.kt (2)
199-200: ConsiderfirstOrNullfor defensive access in example code.
attachments.first { it.type == "date" }will throwNoSuchElementExceptionif the guard check in the caller is ever removed or bypassed. Since this is documentation that users will copy, a defensive pattern would be friendlier:Suggested change
- val attachment = attachmentState.message.attachments.first { it.type == "date" } - val formattedDate = attachment.extraData["payload"].toString() + val attachment = attachmentState.message.attachments.firstOrNull { it.type == "date" } ?: return + val formattedDate = attachment.extraData["payload"]?.toString() ?: returnThe same applies to line 237 in
DateAttachmentPreviewContent.
171-171: Public declarations in docs module lack KDoc.
CustomComponentFactory,dateAttachmentFactory,DateAttachmentContent, andDateAttachmentPreviewContentare public and undocumented. For a docs/example module this is likely acceptable, but since users copy-paste these snippets, a brief comment on each would help discoverability. As per coding guidelines,**/*.ktfiles should "Document public APIs with KDoc, including thread expectations and state notes."
...docs/src/main/kotlin/io/getstream/chat/docs/kotlin/compose/guides/AddingCustomAttachments.kt
Show resolved
Hide resolved
| /** | ||
| * A custom [AttachmentFactory] that adds support for date attachments. | ||
| */ | ||
| object CustomChatComponentFactory : ChatComponentFactory { | ||
| @Composable | ||
| override fun CustomAttachmentContent(state: AttachmentState, modifier: Modifier) { | ||
| if (state.message.attachments.any { it.type == "date" }) { | ||
| DateAttachmentContent(state, modifier) | ||
| } | ||
| } |
There was a problem hiding this comment.
Fix KDoc to reference ChatComponentFactory and add state/thread notes.
The current KDoc mentions AttachmentFactory and omits the required thread/state notes for a public API. Please align the KDoc with the actual type and include the notes.
📝 Suggested KDoc update
-/**
- * A custom [AttachmentFactory] that adds support for date attachments.
- */
+/**
+ * A custom [ChatComponentFactory] that adds support for date attachments.
+ *
+ * Threading: Called from the Compose UI thread.
+ * State: [state] represents the current message/attachment snapshot.
+ */
object CustomChatComponentFactory : ChatComponentFactory {
+ /**
+ * Renders custom attachment content for date attachments.
+ *
+ * Threading: Called from the Compose UI thread.
+ * State: [state] contains the message/attachment snapshot for this item.
+ */
`@Composable`
override fun CustomAttachmentContent(state: AttachmentState, modifier: Modifier) {As per coding guidelines, "Document public APIs with KDoc, including thread expectations and state notes".
🤖 Prompt for AI Agents
In
`@stream-chat-android-ui-guides/src/main/java/io/getstream/chat/android/guides/catalog/compose/customattachments/CustomChatComponentFactory.kt`
around lines 45 - 54, Update the KDoc for CustomChatComponentFactory to
reference ChatComponentFactory (not AttachmentFactory) and include the required
public-API notes about thread expectations and state handling: document that
CustomAttachmentContent is a composable override of
ChatComponentFactory.CustomAttachmentContent, describe the thread (UI/main) and
composable invocation expectations, and add a short note about how
AttachmentState is used/observed (e.g., immutable snapshot, do not mutate).
Ensure references to CustomChatComponentFactory, CustomAttachmentContent,
ChatComponentFactory, AttachmentState and DateAttachmentContent are present in
the KDoc and that the comment sits directly above the object declaration.
...ose/src/main/java/io/getstream/chat/android/compose/ui/components/messages/MessageContent.kt
Show resolved
Hide resolved
The video doesn't load for me, but I checked and I imagine you mean the play button is completely white, right? Good catch, I'll fix it in my next PR 👍 
|
|
3 participants
Goal
This PR finishes the restructuring of the message content, where instead of using attachment factories, we just delegate to the component factory.
Implementation
MessageAttachmentsContent(which was using the attachment factories) and directly call the component factory for the remaining attachment types: links, recordings, unknown/customcontentparameter fromFileAttachmentFactoryand updated examples🎨 UI Changes
There should be no UI changes here. The redesign of audio & file attachments is coming in the next PR.
Testing
Can be tested in the sample by checking messages with different attachments
Summary by CodeRabbit
New Features
Breaking Changes
Improvements