Fix CaptureMediaContract not remembered across recompositions#6169
Conversation
Co-Authored-By: Claude <noreply@anthropic.com>
PR checklist ✅All required conditions are satisfied:
🎉 Great job! This PR is ready for review. |
VelikovPetar
changed the title
CaptureMediaContract not remembered across recompositions
SDK Size Comparison 📏
|
|
WalkthroughRefactors CaptureMediaContract initialization by moving its construction from a direct value passed to rememberLauncherForActivityResult into a separate remember block keyed by photo and video parameters. Maintains identical observable behavior while optimizing contract object lifecycle management. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 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 Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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.
🧹 Nitpick comments (1)
stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/messages/attachments/media/CaptureMediaLauncher.kt (1)
66-72: Consider restructuring to avoid a conditional composable call.The
?: return nullon line 68 meansrememberLauncherForActivityResultis only called whencontractis non-null. This is a conditional composable invocation: ifphoto/videotransitions between(false, false)and a valid combination while this composable is in the tree, the slot forrememberLauncherForActivityResultis reset. While this was pre-existing andphoto/videoare unlikely to change at runtime, it's generally safer to always callrememberLauncherForActivityResultand returnnullafter the fact, or to guard the call site sorememberCaptureMediaLauncheris only called when at least one mode istrue.An alternative that always invokes the composable:
♻️ Suggested restructure
`@Composable` public fun rememberCaptureMediaLauncher( photo: Boolean, video: Boolean, onResult: (File) -> Unit, ): ManagedActivityResultLauncher<Unit, File?>? { - val contract = remember(photo, video) { - resolveMediaPickerMode(photo, video)?.let { CaptureMediaContract(it) } - } ?: return null - return rememberLauncherForActivityResult(contract) { file -> + val mode = remember(photo, video) { resolveMediaPickerMode(photo, video) } + val contract = remember(mode) { mode?.let { CaptureMediaContract(it) } } + val launcher = contract?.let { + rememberLauncherForActivityResult(it) { file -> file?.let(onResult) } + } + return launcher }🤖 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/attachments/media/CaptureMediaLauncher.kt` around lines 66 - 72, Compute val mode = resolveMediaPickerMode(photo, video) via remember(photo, video) and always call rememberLauncherForActivityResult with a non-null ActivityResultContract instance (e.g., construct a CaptureMediaContract using mode when non-null, or pass a harmless dummy CaptureMediaContract when mode is null) so rememberLauncherForActivityResult is invoked unconditionally; then, after creating the launcher, return null if mode is null, otherwise return the launcher that forwards results to onResult. Make sure to reference resolveMediaPickerMode, CaptureMediaContract, rememberLauncherForActivityResult, and the local contract/mode variable so the composable call is not conditionally skipped.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/messages/attachments/media/CaptureMediaLauncher.kt`:
- Around line 66-72: Compute val mode = resolveMediaPickerMode(photo, video) via
remember(photo, video) and always call rememberLauncherForActivityResult with a
non-null ActivityResultContract instance (e.g., construct a CaptureMediaContract
using mode when non-null, or pass a harmless dummy CaptureMediaContract when
mode is null) so rememberLauncherForActivityResult is invoked unconditionally;
then, after creating the launcher, return null if mode is null, otherwise return
the launcher that forwards results to onResult. Make sure to reference
resolveMediaPickerMode, CaptureMediaContract, rememberLauncherForActivityResult,
and the local contract/mode variable so the composable call is not conditionally
skipped.
|
🚀 Available in v6.32.4 |
3 participants
Goal
Fix a bug where
CaptureMediaContractwas being created on every recomposition inrememberCaptureMediaLauncher, causing unnecessary re-registration of the activity result launcher.Implementation
CaptureMediaContractcreation in arememberblock keyed onphotoandvideoparameters🎨 UI Changes
No UI changes.
Testing
Summary by CodeRabbit