Improve attachment picker selection and staging#6233
Merged
Conversation
Contributor
PR checklist ✅All required conditions are satisfied:
🎉 Great job! This PR is ready for review. |
Contributor
SDK Size Comparison 📏
|
andremion
changed the title
andremion
added
pr:breaking-change
andremion
changed the title
andremion
force-pushed
the
redesign/AND-1101-attachment-picker-tweaks
branch
from
c61a36f to
57e9f9f
Compare
WalkthroughRefactors attachment handling: renames attachment APIs (addSelectedAttachments/removeSelectedAttachment → addAttachments/removeAttachment), adds SavedStateHandle-backed persistence for composer and picker state, moves picker selection from Uri to String keys, introduces staging in MessageComposerController, updates factories, UI wiring, samples, and tests. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AttachmentPickerUI as AttachmentPicker
participant PickerVM as AttachmentsPickerViewModel
participant ComposerCtrl as MessageComposerController
participant ComposerVM as MessageComposerViewModel
User->>AttachmentPickerUI: select item(s)
AttachmentPickerUI->>PickerVM: selectItem(uriString)
PickerVM-->>AttachmentPickerUI: items updated (isSelected flags)
User->>AttachmentPickerUI: confirm selection
AttachmentPickerUI->>PickerVM: onSubmit -> emits submittedAttachments
PickerVM->>ComposerVM: (via actions) addAttachments(metadata -> attachments)
ComposerVM->>ComposerCtrl: addAttachments(list)
ComposerCtrl->>ComposerCtrl: stage attachments (keyed by EXTRA_SOURCE_URI)
ComposerCtrl->>ComposerVM: update selectedAttachments flow
ComposerVM->>User: UI reflects staged attachments
Note right of ComposerCtrl: SavedStateHandle persists session as needed
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
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)
📝 Coding Plan
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 Tip CodeRabbit can use TruffleHog to scan for secrets in your code with verification capabilities.Add a TruffleHog config file (e.g. trufflehog-config.yml, trufflehog.yml) to your project to customize detectors and scanning behavior. The tool runs only when a config file is present. |
![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 (7)
stream-chat-android-ui-guides/src/main/java/io/getstream/chat/android/guides/catalog/compose/customattachments/MessagesActivity.kt (1)
131-137:⚠️ Potential issue | 🟠 MajorAdd
EXTRA_SOURCE_URIbefore staging this custom attachment.
addAttachmentsnow only keeps attachments that resolve a non-null source URI inMessageComposerController.addAttachments(stream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/feature/messages/composer/MessageComposerController.kt:619-628). This sample creates a"date"attachment with only"payload"inextraData, so Line 137 will silently drop it and the selected date never reaches the composer. Please stamp a syntheticEXTRA_SOURCE_URIintoextraDatafor this custom attachment before callingaddAttachments.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-chat-android-ui-guides/src/main/java/io/getstream/chat/android/guides/catalog/compose/customattachments/MessagesActivity.kt` around lines 131 - 137, The custom "date" Attachment being added via composerViewModel.addAttachments is dropped because MessageComposerController.addAttachments filters attachments without a non-null source URI; before calling composerViewModel.addAttachments(listOf(attachment)) add a synthetic EXTRA_SOURCE_URI entry into the attachment's extraData (Attachment.extraData) so it resolves to a non-null source URI; update the Attachment constructed in MessagesActivity (where you create Attachment(type="date", extraData=...)) to put the EXTRA_SOURCE_URI key with a unique/stub URI string into extraData so the attachment is retained by MessageComposerController.addAttachments.stream-chat-android-docs/src/main/java/io/getstream/chat/docs/java/ui/messages/MessageComposer.java (1)
413-433:⚠️ Potential issue | 🟡 MinorUpdate the inline sample comment to the renamed API.
The comment on Line 416 still says
addSelectedAttachments(attachments), which now disagrees with the code and the publishedaddAttachments(...)rename.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-chat-android-docs/src/main/java/io/getstream/chat/docs/java/ui/messages/MessageComposer.java` around lines 413 - 433, Update the inline comment in contentCustomization2(): change the comment that mentions addSelectedAttachments(attachments) to use the new API name addAttachments(...). Locate DefaultMessageComposerLeadingContent and the attachments button click listener (the lambda passed to setAttachmentsButtonClickListener) and update the comment to reference messageComposerViewModel.addAttachments(...) with appropriate placeholder args.stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/theme/ChatComponentFactory.kt (1)
3873-3877:⚠️ Potential issue | 🟡 MinorFinish the attachment-picker KDoc rename in this file.
This fixes one reference, but the earlier KDocs at Line 3636 and Line 3662 still point to
ChatTheme.attachmentPickerConfig..., so IDE docs remain inconsistent after the config move.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/theme/ChatComponentFactory.kt` around lines 3873 - 3877, KDoc references to the moved attachment picker config still point to ChatTheme.attachmentPickerConfig; update those docs to reference the new property path ChatTheme.config.attachmentPicker (and specific flags like ChatTheme.config.attachmentPicker.useSystemPicker where applicable) in ChatComponentFactory.kt so all KDoc mentions reflect the config move and remain consistent across the file.stream-chat-android-ui-components/api/stream-chat-android-ui-components.api (1)
4514-4539:⚠️ Potential issue | 🟠 MajorAdd MessageComposerViewModel API changes to CHANGELOG.md.
The
stream-chat-android-ui-componentssection in the UNRELEASED CHANGELOG is empty. Document the attachment API changes under⚠️ Changed for v7:
addSelectedAttachments()→addAttachments()removeSelectedAttachment()→removeAttachment()updateSelectedAttachments()removed,clearAttachments()addedSince v7 allows breaking changes without deprecated overloads, explicit changelog documentation is the migration path for SDK consumers.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-chat-android-ui-components/api/stream-chat-android-ui-components.api` around lines 4514 - 4539, The UNRELEASED CHANGELOG entry for stream-chat-android-ui-components is empty; add a v7 "⚠️ Changed" entry documenting the MessageComposerViewModel attachment API changes: list addSelectedAttachments() → addAttachments(), removeSelectedAttachment() → removeAttachment(), and note that updateSelectedAttachments() was removed and clearAttachments() was added as the replacement; place this under the stream-chat-android-ui-components section in CHANGELOG.md so consumers see the migration path.stream-chat-android-compose-sample/src/androidTestE2eDebug/kotlin/io/getstream/chat/android/compose/tests/AttachmentsTests.kt (2)
140-141:⚠️ Potential issue | 🟡 MinorSame issue: test step says "sends" but only attaches.
Similar to
test_deleteImage, the step description says "WHEN user sends a file" but onlyuploadAttachmentis called withouttapOnSendButton().🐛 Proposed fix
step("WHEN user sends a file") { userRobot.uploadAttachment(type = AttachmentType.IMAGE) + .tapOnSendButton() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-chat-android-compose-sample/src/androidTestE2eDebug/kotlin/io/getstream/chat/android/compose/tests/AttachmentsTests.kt` around lines 140 - 141, The step description says "WHEN user sends a file" but the test only calls userRobot.uploadAttachment(type = AttachmentType.IMAGE) and never actually sends it; update the test to match the step by invoking the send action after attaching (e.g., call the existing tapOnSendButton() helper or equivalent on userRobot) so the flow mirrors test_deleteImage where uploadAttachment is followed by tapOnSendButton().
77-92:⚠️ Potential issue | 🟡 MinorTest step description doesn't match the action performed.
Line 81 says "WHEN user sends an image" but
uploadAttachmentonly attaches without sending. There's notapOnSendButton()call, so the message is never actually sent before attempting to delete it. This may cause the test to fail or test unintended behavior.🐛 Proposed fix
step("WHEN user sends an image") { userRobot.uploadAttachment(type = AttachmentType.IMAGE) + .tapOnSendButton() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-chat-android-compose-sample/src/androidTestE2eDebug/kotlin/io/getstream/chat/android/compose/tests/AttachmentsTests.kt` around lines 77 - 92, The step description is misleading because userRobot.uploadAttachment(type = AttachmentType.IMAGE) only attaches but does not send; update test_deleteImage so after calling userRobot.uploadAttachment(...) you call userRobot.tapOnSendButton() (or change the step text to "attaches an image" if you want to keep it unsent); ensure the sequence uses userRobot.tapOnSendButton() before userRobot.deleteMessage() so the image message is actually sent and then deleted, and keep assertions userRobot.assertImage(...) and userRobot.assertDeletedMessage() as-is.stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/messages/MessagesScreen.kt (1)
364-374:⚠️ Potential issue | 🟠 MajorOnly clear staged attachments after a successful send.
clearAttachments()now runs unconditionally right aftersendMessage(). The send flow already owns attachment cleanup, so doing it here widens the reset to failure/validation paths as well and can drop staged attachments before the user can correct and retry. Please move this to the send success/result handler instead.🤖 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/messages/MessagesScreen.kt` around lines 364 - 374, The current onSendMessage handler clears staged attachments unconditionally (composerViewModel.clearAttachments) immediately after calling composerViewModel.sendMessage, which can drop attachments on failure; remove the unconditional clearAttachments call from this lambda and instead invoke composerViewModel.clearAttachments only when the send completes successfully (move the call into the send success/result handler or observe the send result from composerViewModel and clear attachments on success). Keep the existing attachmentsPickerViewModel.setPickerVisible and clearSelection behavior, but ensure attachment cleanup is triggered from the send completion callback/flow associated with composerViewModel.sendMessage so retries/validation failures keep staged attachments intact.
🧹 Nitpick comments (1)
stream-chat-android-docs/src/main/kotlin/io/getstream/chat/docs/kotlin/compose/messages/AttachmentsPicker.kt (1)
106-111: Consider addingcomposerViewModelas a remember key.The
rememberblock referencescomposerViewModelbut only keys onattachmentsPickerViewModel. IfcomposerViewModelchanges (e.g., due to configuration change with different factory), the memoized actions would reference a stale ViewModel.♻️ Suggested improvement
- actions = remember(attachmentsPickerViewModel) { + actions = remember(attachmentsPickerViewModel, composerViewModel) { AttachmentPickerActions.defaultActions( attachmentsPickerViewModel, composerViewModel, ) },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-chat-android-docs/src/main/kotlin/io/getstream/chat/docs/kotlin/compose/messages/AttachmentsPicker.kt` around lines 106 - 111, The remember call that builds actions currently only keys on attachmentsPickerViewModel but also captures composerViewModel, risking stale references; update the remember invocation to include composerViewModel as a key so the lambda recreates AttachmentPickerActions.defaultActions(attachmentsPickerViewModel, composerViewModel) whenever either attachmentsPickerViewModel or composerViewModel changes.
🤖 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/api/stream-chat-android-compose.api`:
- Around line 4421-4423: Add a changelog entry under the
stream-chat-android-compose UNRELEASED CHANGELOG ⚠️ Changed section that
documents the MessageComposerViewModel attachment API migration: list the method
renames (addSelectedAttachments → addAttachments, removeSelectedAttachment →
removeAttachment, updateSelectedAttachments → clearAttachments) and describe the
new constructor signature change that MessageComposerViewModel now accepts a
androidx.lifecycle.SavedStateHandle parameter; include a short migration note
showing the old method names and their new equivalents and mention that
consumers must update calls and the ViewModel instantiation to pass
SavedStateHandle.
In
`@stream-chat-android-compose/src/test/kotlin/io/getstream/chat/android/compose/viewmodel/messages/AttachmentsPickerViewModelTest.kt`:
- Around line 380-435: Wrap each affected test in runTest and drain the test
scheduler before assertions: replace the plain `@Test` bodies in
AttachmentsPickerViewModelTest for the selection-state cases (the tests invoking
viewModel.select, viewModel.selectItem and asserting
viewModel.attachments.first().isSelected / last().isSelected) with a
kotlinx.coroutines.test.runTest { ... } block and call advanceUntilIdle() (or
equivalent test scheduler drain) after performing selection operations and
before the assertTrue/assertFalse/assertEquals checks to make the flow-backed
state deterministic; ensure the appropriate kotlinx.coroutines.test imports are
added.
---
Outside diff comments:
In
`@stream-chat-android-compose-sample/src/androidTestE2eDebug/kotlin/io/getstream/chat/android/compose/tests/AttachmentsTests.kt`:
- Around line 140-141: The step description says "WHEN user sends a file" but
the test only calls userRobot.uploadAttachment(type = AttachmentType.IMAGE) and
never actually sends it; update the test to match the step by invoking the send
action after attaching (e.g., call the existing tapOnSendButton() helper or
equivalent on userRobot) so the flow mirrors test_deleteImage where
uploadAttachment is followed by tapOnSendButton().
- Around line 77-92: The step description is misleading because
userRobot.uploadAttachment(type = AttachmentType.IMAGE) only attaches but does
not send; update test_deleteImage so after calling
userRobot.uploadAttachment(...) you call userRobot.tapOnSendButton() (or change
the step text to "attaches an image" if you want to keep it unsent); ensure the
sequence uses userRobot.tapOnSendButton() before userRobot.deleteMessage() so
the image message is actually sent and then deleted, and keep assertions
userRobot.assertImage(...) and userRobot.assertDeletedMessage() as-is.
In
`@stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/messages/MessagesScreen.kt`:
- Around line 364-374: The current onSendMessage handler clears staged
attachments unconditionally (composerViewModel.clearAttachments) immediately
after calling composerViewModel.sendMessage, which can drop attachments on
failure; remove the unconditional clearAttachments call from this lambda and
instead invoke composerViewModel.clearAttachments only when the send completes
successfully (move the call into the send success/result handler or observe the
send result from composerViewModel and clear attachments on success). Keep the
existing attachmentsPickerViewModel.setPickerVisible and clearSelection
behavior, but ensure attachment cleanup is triggered from the send completion
callback/flow associated with composerViewModel.sendMessage so
retries/validation failures keep staged attachments intact.
In
`@stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/theme/ChatComponentFactory.kt`:
- Around line 3873-3877: KDoc references to the moved attachment picker config
still point to ChatTheme.attachmentPickerConfig; update those docs to reference
the new property path ChatTheme.config.attachmentPicker (and specific flags like
ChatTheme.config.attachmentPicker.useSystemPicker where applicable) in
ChatComponentFactory.kt so all KDoc mentions reflect the config move and remain
consistent across the file.
In
`@stream-chat-android-docs/src/main/java/io/getstream/chat/docs/java/ui/messages/MessageComposer.java`:
- Around line 413-433: Update the inline comment in contentCustomization2():
change the comment that mentions addSelectedAttachments(attachments) to use the
new API name addAttachments(...). Locate DefaultMessageComposerLeadingContent
and the attachments button click listener (the lambda passed to
setAttachmentsButtonClickListener) and update the comment to reference
messageComposerViewModel.addAttachments(...) with appropriate placeholder args.
In `@stream-chat-android-ui-components/api/stream-chat-android-ui-components.api`:
- Around line 4514-4539: The UNRELEASED CHANGELOG entry for
stream-chat-android-ui-components is empty; add a v7 "⚠️ Changed" entry
documenting the MessageComposerViewModel attachment API changes: list
addSelectedAttachments() → addAttachments(), removeSelectedAttachment() →
removeAttachment(), and note that updateSelectedAttachments() was removed and
clearAttachments() was added as the replacement; place this under the
stream-chat-android-ui-components section in CHANGELOG.md so consumers see the
migration path.
In
`@stream-chat-android-ui-guides/src/main/java/io/getstream/chat/android/guides/catalog/compose/customattachments/MessagesActivity.kt`:
- Around line 131-137: The custom "date" Attachment being added via
composerViewModel.addAttachments is dropped because
MessageComposerController.addAttachments filters attachments without a non-null
source URI; before calling composerViewModel.addAttachments(listOf(attachment))
add a synthetic EXTRA_SOURCE_URI entry into the attachment's extraData
(Attachment.extraData) so it resolves to a non-null source URI; update the
Attachment constructed in MessagesActivity (where you create
Attachment(type="date", extraData=...)) to put the EXTRA_SOURCE_URI key with a
unique/stub URI string into extraData so the attachment is retained by
MessageComposerController.addAttachments.
---
Nitpick comments:
In
`@stream-chat-android-docs/src/main/kotlin/io/getstream/chat/docs/kotlin/compose/messages/AttachmentsPicker.kt`:
- Around line 106-111: The remember call that builds actions currently only keys
on attachmentsPickerViewModel but also captures composerViewModel, risking stale
references; update the remember invocation to include composerViewModel as a key
so the lambda recreates
AttachmentPickerActions.defaultActions(attachmentsPickerViewModel,
composerViewModel) whenever either attachmentsPickerViewModel or
composerViewModel changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 2e30dff2-25be-4b13-a57f-ba97751a8a13
📒 Files selected for processing (34)
stream-chat-android-compose-sample/src/androidTestE2eDebug/kotlin/io/getstream/chat/android/compose/robots/UserRobot.ktstream-chat-android-compose-sample/src/androidTestE2eDebug/kotlin/io/getstream/chat/android/compose/tests/AttachmentsTests.ktstream-chat-android-compose/api/stream-chat-android-compose.apistream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/messages/MessagesScreen.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/messages/attachments/AttachmentPicker.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/messages/attachments/AttachmentPickerActions.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/messages/attachments/AttachmentTypePicker.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/messages/composer/MessageComposer.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/theme/ChatComponentFactory.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/viewmodel/messages/AttachmentsPickerViewModel.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/viewmodel/messages/MessageComposerViewModel.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/viewmodel/messages/MessagesViewModelFactory.ktstream-chat-android-compose/src/test/kotlin/io/getstream/chat/android/compose/viewmodel/messages/AttachmentsPickerViewModelTest.ktstream-chat-android-compose/src/test/kotlin/io/getstream/chat/android/compose/viewmodel/messages/MessageComposerViewModelTest.ktstream-chat-android-compose/src/test/kotlin/io/getstream/chat/android/compose/viewmodel/messages/MessagesViewModelFactoryTest.ktstream-chat-android-docs/src/main/java/io/getstream/chat/docs/java/ui/guides/AddingCustomAttachments.javastream-chat-android-docs/src/main/java/io/getstream/chat/docs/java/ui/messages/MessageComposer.javastream-chat-android-docs/src/main/kotlin/io/getstream/chat/docs/kotlin/compose/guides/AddingCustomAttachments.ktstream-chat-android-docs/src/main/kotlin/io/getstream/chat/docs/kotlin/compose/messages/AttachmentsPicker.ktstream-chat-android-docs/src/main/kotlin/io/getstream/chat/docs/kotlin/compose/messages/MessageComposer.ktstream-chat-android-docs/src/main/kotlin/io/getstream/chat/docs/kotlin/cookbook/ui/CustomComposerAndAttachmentsPicker.ktstream-chat-android-docs/src/main/kotlin/io/getstream/chat/docs/kotlin/ui/guides/AddingCustomAttachments.ktstream-chat-android-docs/src/main/kotlin/io/getstream/chat/docs/kotlin/ui/messages/MessageComposer.ktstream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/feature/messages/composer/MessageComposerController.ktstream-chat-android-ui-common/src/test/kotlin/io/getstream/chat/android/ui/common/feature/messages/composer/MessageComposerControllerTest.ktstream-chat-android-ui-components-sample/src/main/res/layout/fragment_chat.xmlstream-chat-android-ui-components-sample/src/main/res/layout/fragment_chat_preview.xmlstream-chat-android-ui-components/api/stream-chat-android-ui-components.apistream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/feature/messages/composer/internal/AttachmentMetaDataMapper.ktstream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/viewmodel/messages/MessageComposerViewModel.ktstream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/viewmodel/messages/MessageComposerViewModelBinding.ktstream-chat-android-ui-components/src/test/kotlin/io/getstream/chat/android/ui/viewmodels/messages/MessageComposerViewModelTest.ktstream-chat-android-ui-guides/src/main/java/io/getstream/chat/android/guides/catalog/compose/customattachments/MessagesActivity.ktstream-chat-android-ui-guides/src/main/java/io/getstream/chat/android/guides/catalog/uicomponents/customattachments/MessagesActivity.kt
andremion
force-pushed
the
redesign/AND-1101-attachment-picker-tweaks
branch
from
9cd398e to
eb0778c
Compare
gpunto
reviewed
Mar 12, 2026
andremion
force-pushed
the
redesign/AND-1101-attachment-picker-tweaks
branch
2 times, most recently
from
0a56359 to
88348a3
Compare
gpunto
approved these changes
Mar 16, 2026
Contributor
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
stream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/feature/messages/composer/ComposerSessionRepository.kt (2)
113-118: Consider using indexed accessor for consistency.Per static analysis,
obj.get(key)can be replaced withobj[key].Suggested change
val extraData = buildMap { json.optJSONObject(KEY_EXTRA_DATA)?.let { obj -> - for (key in obj.keys()) put(key, obj.get(key)) + for (key in obj.keys()) put(key, obj[key]) } if (uri != null) put(EXTRA_SOURCE_URI, uri) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/feature/messages/composer/ComposerSessionRepository.kt` around lines 113 - 118, In the buildMap block that constructs extraData inside ComposerSessionRepository (the json.optJSONObject(KEY_EXTRA_DATA)?.let { obj -> for (key in obj.keys()) put(key, obj.get(key)) } section), replace the call to obj.get(key) with the indexed accessor obj[key] for consistency with Kotlin style and static analysis; keep the surrounding logic (including the put(EXTRA_SOURCE_URI, uri) branch) unchanged.
84-87: Consider using indexed accessor for consistency.Per static analysis,
savedStateHandle.get<String>(KEY_SESSION)can be simplified tosavedStateHandle[KEY_SESSION].Suggested change
private fun sessionJson(): JSONObject? { - val str = savedStateHandle.get<String>(KEY_SESSION) ?: return null + val str = savedStateHandle[KEY_SESSION] ?: return null return try { JSONObject(str) } catch (_: Exception) { null } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/feature/messages/composer/ComposerSessionRepository.kt` around lines 84 - 87, In sessionJson(), replace the generic get call with the indexed accessor on savedStateHandle: use savedStateHandle[KEY_SESSION] instead of savedStateHandle.get<String>(KEY_SESSION); keep the null check and try/catch behavior the same (i.e., return null if the value is absent or JSONObject parsing fails) and update only the retrieval expression in the sessionJson() function.
🤖 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-sample/src/androidTestE2eDebug/kotlin/io/getstream/chat/android/compose/tests/AttachmentsTests.kt`:
- Around line 143-148: The test uses userRobot.attachFile(type =
AttachmentType.IMAGE) but the step and intent are for sending a generic file;
change the attachment type to AttachmentType.FILE and update any image-specific
assertions (e.g., assertImage) to the corresponding file assertions (e.g.,
assertFile) so the step "WHEN user sends a file" matches the actual attachment
and validations; locate calls to userRobot.attachFile, AttachmentType.IMAGE, and
the assertions around lines after tapOnSendButton to make the edits.
In `@stream-chat-android-ui-components/api/stream-chat-android-ui-components.api`:
- Line 4514: Document the API renames for MessageComposerViewModel in the v7
migration docs: add explicit mappings from addSelectedAttachments() →
addAttachments(), removeSelectedAttachment() → removeAttachment(), and announce
the new clearAttachments() method; update MIGRATION_TO_V7.md (or the changelog)
with those mappings, mention any signature/behavior changes for
addAttachments/removeAttachment if applicable, and include a short example or
one-line replacement guidance so callers can update their code quickly.
---
Nitpick comments:
In
`@stream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/feature/messages/composer/ComposerSessionRepository.kt`:
- Around line 113-118: In the buildMap block that constructs extraData inside
ComposerSessionRepository (the json.optJSONObject(KEY_EXTRA_DATA)?.let { obj ->
for (key in obj.keys()) put(key, obj.get(key)) } section), replace the call to
obj.get(key) with the indexed accessor obj[key] for consistency with Kotlin
style and static analysis; keep the surrounding logic (including the
put(EXTRA_SOURCE_URI, uri) branch) unchanged.
- Around line 84-87: In sessionJson(), replace the generic get call with the
indexed accessor on savedStateHandle: use savedStateHandle[KEY_SESSION] instead
of savedStateHandle.get<String>(KEY_SESSION); keep the null check and try/catch
behavior the same (i.e., return null if the value is absent or JSONObject
parsing fails) and update only the retrieval expression in the sessionJson()
function.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 91b54bac-7622-4c9b-9f41-f9f637fb6f1c
📒 Files selected for processing (42)
stream-chat-android-compose-sample/src/androidTestE2eDebug/kotlin/io/getstream/chat/android/compose/robots/UserRobot.ktstream-chat-android-compose-sample/src/androidTestE2eDebug/kotlin/io/getstream/chat/android/compose/tests/AttachmentsTests.ktstream-chat-android-compose-sample/src/main/java/io/getstream/chat/android/compose/sample/data/CustomSettings.ktstream-chat-android-compose-sample/src/main/java/io/getstream/chat/android/compose/sample/ui/MessagesActivity.ktstream-chat-android-compose-sample/src/main/java/io/getstream/chat/android/compose/sample/ui/login/CustomLoginActivity.ktstream-chat-android-compose-sample/src/main/res/values/strings.xmlstream-chat-android-compose/api/stream-chat-android-compose.apistream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/messages/MessagesScreen.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/messages/attachments/AttachmentPicker.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/messages/attachments/AttachmentPickerActions.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/messages/attachments/AttachmentTypePicker.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/messages/composer/MessageComposer.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/theme/ChatComponentFactory.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/viewmodel/messages/AttachmentsPickerViewModel.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/viewmodel/messages/MessageComposerViewModel.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/viewmodel/messages/MessagesViewModelFactory.ktstream-chat-android-compose/src/test/kotlin/io/getstream/chat/android/compose/viewmodel/messages/AttachmentsPickerViewModelTest.ktstream-chat-android-compose/src/test/kotlin/io/getstream/chat/android/compose/viewmodel/messages/MessageComposerViewModelTest.ktstream-chat-android-compose/src/test/kotlin/io/getstream/chat/android/compose/viewmodel/messages/MessagesViewModelFactoryTest.ktstream-chat-android-docs/src/main/java/io/getstream/chat/docs/java/ui/guides/AddingCustomAttachments.javastream-chat-android-docs/src/main/java/io/getstream/chat/docs/java/ui/messages/MessageComposer.javastream-chat-android-docs/src/main/kotlin/io/getstream/chat/docs/kotlin/compose/guides/AddingCustomAttachments.ktstream-chat-android-docs/src/main/kotlin/io/getstream/chat/docs/kotlin/compose/messages/AttachmentsPicker.ktstream-chat-android-docs/src/main/kotlin/io/getstream/chat/docs/kotlin/compose/messages/MessageComposer.ktstream-chat-android-docs/src/main/kotlin/io/getstream/chat/docs/kotlin/cookbook/ui/CustomComposerAndAttachmentsPicker.ktstream-chat-android-docs/src/main/kotlin/io/getstream/chat/docs/kotlin/ui/guides/AddingCustomAttachments.ktstream-chat-android-docs/src/main/kotlin/io/getstream/chat/docs/kotlin/ui/messages/MessageComposer.ktstream-chat-android-ui-common/build.gradle.ktsstream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/feature/messages/composer/ComposerSessionRepository.ktstream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/feature/messages/composer/MessageComposerController.ktstream-chat-android-ui-common/src/test/kotlin/io/getstream/chat/android/ui/common/feature/messages/composer/ComposerSessionRepositoryTest.ktstream-chat-android-ui-common/src/test/kotlin/io/getstream/chat/android/ui/common/feature/messages/composer/MessageComposerControllerTest.ktstream-chat-android-ui-components-sample/src/main/res/layout/fragment_chat.xmlstream-chat-android-ui-components-sample/src/main/res/layout/fragment_chat_preview.xmlstream-chat-android-ui-components/api/stream-chat-android-ui-components.apistream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/feature/messages/composer/internal/AttachmentMetaDataMapper.ktstream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/viewmodel/messages/MessageComposerViewModel.ktstream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/viewmodel/messages/MessageComposerViewModelBinding.ktstream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/viewmodel/messages/MessageListViewModelFactory.ktstream-chat-android-ui-components/src/test/kotlin/io/getstream/chat/android/ui/viewmodels/messages/MessageComposerViewModelTest.ktstream-chat-android-ui-guides/src/main/java/io/getstream/chat/android/guides/catalog/compose/customattachments/MessagesActivity.ktstream-chat-android-ui-guides/src/main/java/io/getstream/chat/android/guides/catalog/uicomponents/customattachments/MessagesActivity.kt
…e across process death and synchronize external attachments. - Implement `SavedStateHandle` persistence for selected URIs, grid attachments, and external attachments (camera/system picker). - Add `addExternalAttachments` and `removeExternalAttachment` to manage one-shot attachment sources. - Update `getSelectedAttachments` to serve as the single source of truth for the message composer. - Ensure `clearSelection` resets all persisted picker state.
…ckerViewModel` and `MessageComposerViewModel`. - Shift responsibility for the master list of staged attachments from `AttachmentsPickerViewModel` to `MessageComposerViewModel`, making the latter the single source of truth for message attachments. - Implement state persistence for selected attachments in `MessageComposerViewModel` using `SavedStateHandle` to survive Activity recreation. - Update `AttachmentsPickerViewModel` to only manage grid selection state (URI-based checkmarks) and storage browsing. - Refactor `AttachmentPickerActions` to coordinate selection between the picker (grid UI) and the composer (staged list). - Rename attachment manipulation methods in `MessageComposerViewModel` to `addAttachments`, `removeAttachment`, and `clearAttachments`. - Update `MessagesViewModelFactory` to provide `SavedStateHandle` to the composer and picker ViewModels.
…tions across process death
…AttachmentsPickerViewModel`, `MessageComposerController`, and `MessageComposerViewModel`.
…in attachment metadata. - Map the `uri` from `AttachmentMetaData` to `extraData` using the `EXTRA_SOURCE_URI` key. - Ensure the source URI is preserved when converting metadata to an `Attachment` object.
…th the composer sample app's
…er state across process death
…ViewModel` to `MessageComposerController` via `SavedStateHandle`.
- Introduce a dedicated state for audio recording attachments to ensure they are properly tracked and synced. - Implement a fallback attachment key using name, MIME type, and file size when a source URI is missing. - Update `removeAttachment` and `clearAttachments` to correctly handle picker selections, edit-mode attachments, and recording attachments. - Ensure all attachment types are preserved and correctly merged during synchronization.
…o the text field is populated after process-death restore in edit mode.
…ose sample app. - Add string resources for the system attachment picker setting. - Extend `CustomSettings` to persist the `isSystemAttachmentPickerEnabled` preference. - Update `CustomLoginActivity` to include a UI toggle for the system attachment picker feature flag. - Configure `AttachmentPickerConfig` in `MessagesActivity` to use the preference value.
andremion
force-pushed
the
redesign/AND-1101-attachment-picker-tweaks
branch
from
46ae8f1 to
7660fe9
Compare
|
Contributor
|
🚀 Available in v7.0.0-beta |
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
3 participants
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
Camera-captured attachments were silently dropped when the user subsequently selected items
from the gallery or file tabs. It was also impossible to combine camera captures with gallery
or file selections in a single message. This change fixes the picker so all attachment types
co-exist in the composer, insertion order is preserved, and
allowMultipleSelectionis respectedper tab.
Implementation
Root causes fixed:
AttachmentTypePicker'sLaunchedEffect(modes)re-ran on every recomposition, resetting theactive tab to the first one and discarding the user's current context.
updateSelectedAttachments(getSelectedAttachments())rebuilt the composer list from only thecurrent tab's selection, silently dropping camera attachments that arrived via a separate path.
AttachmentPickerActionslacked@Stable, so it was re-created on every recomposition.Attachment staging redesign (
MessageComposerController):List<Attachment>with aLinkedHashMap<String, Attachment>keyed byEXTRA_SOURCE_URI. All sources (camera, gallery, files) write to the same map, preservinginsertion order and deduplicating by URI rather than by filename.
addSelectedAttachments→addAttachments,removeSelectedAttachment→removeAttachment. AddedremoveAttachmentsByUris,clearAttachments. RemovedupdateSelectedAttachments.AttachmentsPickerViewModel/AttachmentPickerActions(Compose):Set<Uri>toSet<String>(URI strings), persisted inSavedStateHandleso checkmarks survive process death (e.g. camera launch and return).and the composer list atomically. In single-select mode (
allowMultipleSelection = false),the displaced URI is removed from the composer before the new item is added.
@Stableadded toAttachmentPickerActions; default instance wrapped withremember.MessagesViewModelFactory/MessageComposerViewModel(Compose):SavedStateHandlethreaded through to bothMessageComposerViewModelandAttachmentsPickerViewModelfor full state survival across Activity recreation.MessagesViewModelFactorysimplified from aMap<Class<*>, () -> ViewModel>registry toa
whenexpression.XML SDK fix (
AttachmentMetaDataMapper):toAttachment()now stampsEXTRA_SOURCE_URIintoextraData, fixing a regression wherethe XML in-app picker silently dropped all selected attachments.
XML sample: switched to in-app picker (
streamUiMessageComposerAttachmentsPickerSystemPickerEnabled="false").Composer sample: Add a feature flag to enable the system attachment picker.
🎨 UI Changes
Screen_recording_20260310_093908.webm
Screen_recording_20260310_094007.webm
Testing
select an image. Verify both attachments appear in the composer.
both appear in the composer.
the composer.
it appears in the composer and sends correctly.
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Improvements