Draft
Conversation
Contributor
PR checklist ✅All required conditions are satisfied:
🎉 Great job! This PR is ready for review. |
Contributor
SDK Size Comparison 📏
|
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughThis PR parallelizes attachment uploads in the worker and refactors the Compose UI to integrate upload progress indicators into existing attachment content components, removing dedicated upload-specific UI components (FileUploadContent, UploadAttachmentFactory, UploadingFooter) and their related APIs. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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: 1
🧹 Nitpick comments (2)
stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/components/LoadingIndicator.kt (1)
47-54: Clarify the new progress lambda contract in KDoc.The new overload takes a lambda, but the KDoc still reads like
progressis a plain value. Please document that it must return0f..1fand is read during composition so SDK consumers know it needs Compose-backed state.📝 Suggested KDoc tweak
- * `@param` progress The current progress value between 0f and 1f. + * `@param` progress Lambda returning the current progress in the `0f..1f` range. + * It is read during composition and should be backed by Compose state.As per coding guidelines, "Document public APIs with KDoc, including thread expectations and state notes".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/components/LoadingIndicator.kt` around lines 47 - 54, Update the KDoc for the public Composable LoadingIndicator to describe that the parameter progress is a lambda (progress: () -> Float) that must return a value in the 0f..1f range and that it is invoked during composition, so callers should supply Compose-backed state (e.g., remember/State<Float>) or other thread-safe state to provide updates; reference the LoadingIndicator(progress: () -> Float, modifier: Modifier = Modifier) signature and note the composition/read expectation.stream-chat-android-compose/src/test/kotlin/io/getstream/chat/android/compose/ui/attachments/content/AttachmentsContentTest.kt (1)
81-84: Add a Paparazzi case for uploading media as well.Line 83 updates the file-upload snapshot, but this PR also changes the upload state UI for media attachments. Please add a matching
MediaAttachmentContent/message-level snapshot so the new image/video upload treatment is covered too.As per coding guidelines, "Applies to /stream-chat-android-compose//*Test.kt: Add Paparazzi snapshots for Compose UI regressions and run
verifyPaparazziDebug".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-chat-android-compose/src/test/kotlin/io/getstream/chat/android/compose/ui/attachments/content/AttachmentsContentTest.kt` around lines 81 - 84, Add a Paparazzi snapshot test for the media upload UI analogous to the existing file-upload test: create a new test function (e.g., `media upload content`) that calls `snapshotWithDarkModeRow { MediaAttachmentContent(isMine = true, isUploading = true) }` so the new image/video upload treatment is covered; place it alongside the `file upload content` test in `AttachmentsContentTest.kt` and follow the same naming/structure conventions so it runs under `verifyPaparazziDebug`.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/attachments/content/MediaAttachmentContent.kt`:
- Around line 409-423: The overflow shimmer calculation currently includes the
visible last-slot attachment because anyOverflowUploading is computed with
attachments.drop(maximumNumberOfPreviewedItems - 1); change the scan to exclude
the visible tile by starting the drop at maximumNumberOfPreviewedItems (or
alternatively filter out the attachment at attachmentIndex) so only hidden
attachments are considered; update the anyOverflowUploading computation used
when creating MediaAttachmentContentItem (in MediaAttachmentContent.kt near the
MediaAttachmentContentItem call) to reflect this exclusion.
---
Nitpick comments:
In
`@stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/components/LoadingIndicator.kt`:
- Around line 47-54: Update the KDoc for the public Composable LoadingIndicator
to describe that the parameter progress is a lambda (progress: () -> Float) that
must return a value in the 0f..1f range and that it is invoked during
composition, so callers should supply Compose-backed state (e.g.,
remember/State<Float>) or other thread-safe state to provide updates; reference
the LoadingIndicator(progress: () -> Float, modifier: Modifier = Modifier)
signature and note the composition/read expectation.
In
`@stream-chat-android-compose/src/test/kotlin/io/getstream/chat/android/compose/ui/attachments/content/AttachmentsContentTest.kt`:
- Around line 81-84: Add a Paparazzi snapshot test for the media upload UI
analogous to the existing file-upload test: create a new test function (e.g.,
`media upload content`) that calls `snapshotWithDarkModeRow {
MediaAttachmentContent(isMine = true, isUploading = true) }` so the new
image/video upload treatment is covered; place it alongside the `file upload
content` test in `AttachmentsContentTest.kt` and follow the same
naming/structure conventions so it runs under `verifyPaparazziDebug`.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 77397de3-18bf-49dc-8145-6dfffed391d3
⛔ Files ignored due to path filters (26)
stream-chat-android-compose/src/test/snapshots/images/io.getstream.chat.android.compose.ui.attachments.content_AttachmentsContentTest_file_attachment_content.pngis excluded by!**/*.pngstream-chat-android-compose/src/test/snapshots/images/io.getstream.chat.android.compose.ui.attachments.content_AttachmentsContentTest_file_upload_content.pngis excluded by!**/*.pngstream-chat-android-compose/src/test/snapshots/images/io.getstream.chat.android.compose.ui.channel.attachments_ChannelFilesAttachmentsContentTest_loading.pngis excluded by!**/*.pngstream-chat-android-compose/src/test/snapshots/images/io.getstream.chat.android.compose.ui.channel.attachments_ChannelFilesAttachmentsContentTest_loading_in_dark_mode.pngis excluded by!**/*.pngstream-chat-android-compose/src/test/snapshots/images/io.getstream.chat.android.compose.ui.channel.attachments_ChannelFilesAttachmentsContentTest_loading_more.pngis excluded by!**/*.pngstream-chat-android-compose/src/test/snapshots/images/io.getstream.chat.android.compose.ui.channel.attachments_ChannelFilesAttachmentsContentTest_loading_more_in_dark_mode.pngis excluded by!**/*.pngstream-chat-android-compose/src/test/snapshots/images/io.getstream.chat.android.compose.ui.channel.attachments_ChannelMediaAttachmentsContentTest_loading.pngis excluded by!**/*.pngstream-chat-android-compose/src/test/snapshots/images/io.getstream.chat.android.compose.ui.channel.attachments_ChannelMediaAttachmentsContentTest_loading_in_dark_mode.pngis excluded by!**/*.pngstream-chat-android-compose/src/test/snapshots/images/io.getstream.chat.android.compose.ui.channel.info_DirectChannelInfoContentTest_content.pngis excluded by!**/*.pngstream-chat-android-compose/src/test/snapshots/images/io.getstream.chat.android.compose.ui.channel.info_DirectChannelInfoContentTest_content_in_dark_mode.pngis excluded by!**/*.pngstream-chat-android-compose/src/test/snapshots/images/io.getstream.chat.android.compose.ui.channel.info_DirectChannelInfoContentTest_loading.pngis excluded by!**/*.pngstream-chat-android-compose/src/test/snapshots/images/io.getstream.chat.android.compose.ui.channel.info_DirectChannelInfoContentTest_loading_in_dark_mode.pngis excluded by!**/*.pngstream-chat-android-compose/src/test/snapshots/images/io.getstream.chat.android.compose.ui.channel.info_GroupChannelInfoContentTest_loading.pngis excluded by!**/*.pngstream-chat-android-compose/src/test/snapshots/images/io.getstream.chat.android.compose.ui.channel.info_GroupChannelInfoContentTest_loading_in_dark_mode.pngis excluded by!**/*.pngstream-chat-android-compose/src/test/snapshots/images/io.getstream.chat.android.compose.ui.components.poll_PollOptionVotesDialogTest_loading_in_dark_mode.pngis excluded by!**/*.pngstream-chat-android-compose/src/test/snapshots/images/io.getstream.chat.android.compose.ui.components.poll_PollOptionVotesDialogTest_loading_in_light_mode.pngis excluded by!**/*.pngstream-chat-android-compose/src/test/snapshots/images/io.getstream.chat.android.compose.ui.components.poll_PollOptionVotesDialogTest_loading_more_in_dark_mode.pngis excluded by!**/*.pngstream-chat-android-compose/src/test/snapshots/images/io.getstream.chat.android.compose.ui.components.poll_PollOptionVotesDialogTest_loading_more_in_light_mode.pngis excluded by!**/*.pngstream-chat-android-compose/src/test/snapshots/images/io.getstream.chat.android.compose.ui.mentions_MentionListTest_loading_mention_list.pngis excluded by!**/*.pngstream-chat-android-compose/src/test/snapshots/images/io.getstream.chat.android.compose.ui.mentions_MentionListTest_loading_more_mention_list.pngis excluded by!**/*.pngstream-chat-android-compose/src/test/snapshots/images/io.getstream.chat.android.compose.ui.messages_MessageListTest_loading_messages.pngis excluded by!**/*.pngstream-chat-android-compose/src/test/snapshots/images/io.getstream.chat.android.compose.ui.messages_MessageListTest_loading_older_messages.pngis excluded by!**/*.pngstream-chat-android-compose/src/test/snapshots/images/io.getstream.chat.android.compose.ui.pinned_PinnedMessageListTest_loading_more_pinned_messages.pngis excluded by!**/*.pngstream-chat-android-compose/src/test/snapshots/images/io.getstream.chat.android.compose.ui.pinned_PinnedMessageListTest_loading_pinned_messages.pngis excluded by!**/*.pngstream-chat-android-compose/src/test/snapshots/images/io.getstream.chat.android.compose.ui.threads_ThreadListTest_loading_more_threads.pngis excluded by!**/*.pngstream-chat-android-ui-components/src/test/snapshots/images/io.getstream.chat.android.ui.feature.channels.list_ChannelListViewTest_loaded_channels.pngis excluded by!**/*.png
📒 Files selected for processing (14)
stream-chat-android-client/src/main/java/io/getstream/chat/android/client/attachment/worker/UploadAttachmentsWorker.ktstream-chat-android-compose/api/stream-chat-android-compose.apistream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/attachments/StreamAttachmentFactories.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/attachments/content/FileAttachmentContent.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/attachments/content/FileUploadContent.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/attachments/content/MediaAttachmentContent.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/attachments/factory/UploadAttachmentFactory.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/components/LoadingIndicator.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/components/messages/MessageContent.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/components/messages/UploadingFooter.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/theme/ChatComponentFactory.ktstream-chat-android-compose/src/test/kotlin/io/getstream/chat/android/compose/ui/attachments/content/AttachmentsContentTest.ktstream-chat-android-docs/src/main/kotlin/io/getstream/chat/docs/kotlin/compose/guides/CustomizingImageAndVideoPreviews.ktstream-chat-android-ui-guides/src/main/java/io/getstream/chat/android/guides/catalog/compose/customizingimageandvideoattachments/MessagesActivity.kt
💤 Files with no reviewable changes (5)
- stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/components/messages/UploadingFooter.kt
- stream-chat-android-ui-guides/src/main/java/io/getstream/chat/android/guides/catalog/compose/customizingimageandvideoattachments/MessagesActivity.kt
- stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/attachments/StreamAttachmentFactories.kt
- stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/attachments/content/FileUploadContent.kt
- stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/attachments/factory/UploadAttachmentFactory.kt
...main/java/io/getstream/chat/android/compose/ui/attachments/content/MediaAttachmentContent.kt
Show resolved
Hide resolved
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Goal
Update design of attachment uploading state
Implementation
FileUploadContentFileAttachmentContent&MediaAttachmentContentto show the new upload state🎨 UI Changes
Screen_recording_20260309_105046.webm
Note, in the video you see the PDF file having size 0. It seems it comes like that from the backend:
Testing
Try uploading attachments in the sample
Summary by CodeRabbit
Release Notes
New Features
Refactor