Improve attachment picker selection and staging#6233
Open
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
WalkthroughThis pull request refactors the attachment handling system across the Stream Chat Android SDK by renaming attachment-related APIs, integrating SavedStateHandle for state persistence across process death, transitioning from URI-based to string-based selection storage in the picker, and introducing new internal helper methods for attachment synchronization. The changes span ViewModels, the controller layer, UI components, and associated tests. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 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)
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: 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
stream-chat-android-compose/api/stream-chat-android-compose.api
Outdated
Show resolved
Hide resolved
...otlin/io/getstream/chat/android/compose/viewmodel/messages/AttachmentsPickerViewModelTest.kt
Show resolved
Hide resolved
andremion
force-pushed
the
redesign/AND-1101-attachment-picker-tweaks
branch
from
9cd398e to
eb0778c
Compare
gpunto
reviewed
Mar 12, 2026
| private const val KeyBundleMimeType = "mimeType" | ||
| private const val AttachmentBundleSize = 5 | ||
|
|
||
| private fun Attachment.toBundle(): Bundle = Bundle(AttachmentBundleSize).apply { |
Contributor
There was a problem hiding this comment.
Here we ignore many attachment properties and in particular the custom data, wouldn't that be a problem if an integrator sets it in a custom attachment picker like in AddingCustomAttachmentsSnippet?
TBH I'm also not sure what's the alternative for the Map because we can't really serialize opaque data. I guess a partial mitigation would be to iterate through the entries and add to the bundle the ones we know are fine, like strings, ints, etc. But this doesn't address other data, which would still be lost.
Or maybe it's better to just rely just on memory for configuration changes (so we skip the bundle) and restart from an empty state on recreation after process death. Is the app consistently killed when capturing a picture? I'm surprised if that's the case.
Contributor
Author
There was a problem hiding this comment.
The reason for supporting activity restoration is that we had customers reporting issues because their users have low-end devices, which often reproduce scenarios of DKA (Don't Keep Activities).
I will reiterate on that code again to see if something different can be done.
Contributor
Author
There was a problem hiding this comment.
I pushed some changes. I'm not 100% happy with the solution. Let me know WYT
Comment on lines
+251
to
+253
| // Insertion-ordered map keyed by EXTRA_SOURCE_URI. Tracks picker selections | ||
| // independently of edit-mode attachments, so selections survive entering and exiting edit mode. | ||
| private val _selectedAttachments = MutableStateFlow(linkedMapOf<String, Attachment>()) |
Contributor
There was a problem hiding this comment.
I'm facing this in edit mode:
- I can't remove attachments
- Whenever I add a new attachment, the existing ones are cleared
Contributor
Author
There was a problem hiding this comment.
Good catch. I will check that. Thanks
Contributor
Author
There was a problem hiding this comment.
I pushed the fixes. Let me know if it's still reproducible
…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.
andremion
force-pushed
the
redesign/AND-1101-attachment-picker-tweaks
branch
from
931a76b to
0a56359
Compare
|
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
2 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").🎨 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