Repository navigation
feat(mediaviewer): show the position, add forward and draw actions to the media viewer - #6823
Open
ToteMeiSter wants to merge 28 commits into
Open
ToteMeiSter wants to merge 28 commits into
ToteMeiSter wants to merge 28 commits into
Conversation
The observer of the recording state is registered again when the chat activity is recreated, for example on a screen rotation. LiveData then delivers the current value to it and the device vibrated as if a recording had just started or ended. Vibrate only when the state differs from the one the activity already knew. Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru> Assisted-by: Claude-Code:claude-opus-5-5
A rotation stops the chat activity. ChatViewModel.onStop then stopped the MediaRecorder, while the locked and in-progress state of the recording stayed. The screen still showed a running recording and a truncated file was sent. The view model now keeps the recorder running when the stopping activity is changing its configuration. It reads this from the lifecycle owner: ON_STOP reaches the observer before the body of Activity.onStop runs (API 29+), so a flag set by the activity in onStop would come too late. The recorder is still stopped when the user leaves the chat. Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru> Assisted-by: Claude-Code:claude-opus-5-5
A short tap on the record button switches between voice and video mode. The mode is stored in AppPreferences and shown by the button icon and a snackbar hint. The result is a regular video attachment. In video mode holding the button records with CameraX (front camera, 720p with fallback, about 2.5 Mbit/s, at most 120 s, then it stops and is sent). Sliding left cancels, sliding up locks. The gestures, the lock state, the timer and the locked recording view are shared with the voice recording. The preview is shown above the input and the camera can be switched during the recording. An unfinished recording is cancelled and the camera released when the chat is paused. Too short recordings show a hint, camera errors a message. The finished mp4 goes through the regular file upload. Refs nextcloud#6812 Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru> Assisted-by: Claude-Code:claude-opus-5-5
… popup The record button used to start a recording on the first touch, so a short tap that should switch the mode started a recording first. Recording now starts only when the button is held. A tap shows the record hint as a popup above the button instead of a snackbar, placed in the window of the button and kept above it after layout changes. Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru> Assisted-by: Claude-Code:claude-opus-5-5
The activity is recreated on rotation while the video recorder belonged to it: onPause cancelled the recording and deleted its file, and binding the camera to the next activity made the CameraX recorder configure itself in a state where that is not allowed. - The video recorder lives in ChatViewModel like the voice recorder. The activity attaches its preview and callback, and the recorder lets go of them when the activity is destroyed. A result that arrives without an activity is delivered to the next one. - The camera is bound to a lifecycle of the recorder itself, not to the one of the activity, so it stays bound while the activity is recreated. - A new activity picks up the recording and locks it when it was held, so the timer, stop, send and cancel are shown. - The recording is cancelled when the user leaves the chat, not on a configuration change. - A recording cut off by the camera is not lost but shown in the attachment preview. - The video is recorded in the rotation of the sensor. - A recording is never stopped while a switched camera settles. Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru> Assisted-by: Claude-Code:claude-opus-5-5
The container of the video recording preview did not consume touches, so taps on it reached the chat below. Make it consume them and hide it from TalkBack, where it is only decoration. Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru> Assisted-by: Claude-Code:claude-opus-5-5
Show recent photos and videos from MediaStore in a 3-column grid with a live camera tile and multi-select, and keep every former menu entry in a bar below with unchanged visibility rules. Selected media open the existing attachment preview with caption. Supports Android 14 partial media access. Removes AttachmentDialog and dialog_attachment.xml. Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru> Assisted-by: Claude-Code:claude-opus-5-5
READ_MEDIA_VISUAL_USER_SELECTED turns off the Android 14 compatibility mode, so isFilesPermissionGranted() must accept the partial grant, otherwise voice and video messages, camera photos, local files and share-to-Talk ask for permissions again. Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru> Assisted-by: Claude-Code:claude-opus-5-5
Add a full screen CameraX capture screen (shutter, front/back lens, flash off/auto/on, close) behind the TakePhotoInApp result contract. The photo is returned right after the shutter, without a confirmation step of its own, and goes to the attachment preview. The camera tile, the camera action of the attachment sheet and the photo button of the preview use it. The system ACTION_IMAGE_CAPTURE path is removed; video capture is unchanged. The capture screen asks CameraX for the full 4:3 resolution explicitly, ignores a second shutter tap while a photo is taken and keeps the controls on the device body when the device is rotated. rotationForDeviceOrientation() is shared with the video message recorder. Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru> Assisted-by: Claude-Code:claude-opus-5-5
Rework the pre-send attachment preview: the photo fills the screen with the caption floating over it, a compact bottom panel holds the SD/HD switch and View-only choice, and a round send button replaces the inline one. - a tick mark per file excludes it from sending (at least one stays) - "add more" menu next to the caption: gallery, photo, video - crop and rotate through the bundled uCrop (setMaxBitmapSize(4096), so uCrop does not downscale the picture to the screen size); the result replaces the file at the same position and is what gets sent - brush drawing on a Compose canvas, burned into a full-resolution copy - the preview result is delivered through the fragment manager, so a send after the activity was recreated still uploads the files - the original image size is shown in display orientation - selection, uri replacement, drawing geometry and file ownership are pure functions covered by unit tests Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru> Assisted-by: Claude-Code:claude-opus-5-5
A button at the top edge of the capture screen toggles ImageCapture between CAPTURE_MODE_MINIMIZE_LATENCY (default, fast) and CAPTURE_MODE_MAXIMIZE_QUALITY. The choice is kept in AppPreferences (camera_capture_mode). Each saved photo logs "capture mode=... took=... ms" under the PhotoCamera tag to compare modes. Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru> Assisted-by: Claude-Code:claude-opus-5-5
…g start The start time was taken on ACTION_DOWN, but the recording starts only after the 400-600 ms hold threshold, so a voice message was judged too short (or long enough) by a duration which included the hold. Take the time in beginRecording() after the recording has started and use the monotonic clock. Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru> Assisted-by: Claude-Code:claude-opus-5-5
…alized it Send or delete of a locked video recording cleared the lock and the in-progress state at once, while CameraX was still finalizing the file. A new recording started in that window was silently dropped in beginRecording(). For video the state is now released in onVideoRecordingFinished(), which CameraX reaches for every outcome (send, cancel, error, too short). Audio, and a video recorder which is already idle, are still cleared at once. Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru> Assisted-by: Claude-Code:claude-opus-5-5
…rded frame The locked video recording preview was a fixed 180x240dp box floating over the message bubbles, squashed to about 2:1 in landscape. Now a scrim dims the chat pane (it blocks touches and is hidden from TalkBack), and the preview is centered in it in the aspect of the recorded frame, fitted with margins, at most 75% of the width and 480dp on the longest side. The placement is a pure function (videoPreviewPlacement) with unit tests; it is recomputed when the pane changes size and when the recorder learns the frame aspect from CameraX. Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru> Assisted-by: Claude-Code:claude-opus-5-5
…entation, hide chat from TalkBack - Center by Gravity.CENTER and change only width and height: the margin comparison never settled in RTL and re-laid out the chat every frame. - The frame aspect is turned into screen coordinates by the difference of the recording and the display rotation (screenFrameAspect), so the frame matches the picture with auto-rotate off and after a turn during the recording. - While recording, the siblings of the scrim are hidden from TalkBack and get their previous values back afterwards. - The preview size is recalculated after a camera switch. Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru> Assisted-by: Claude-Code:claude-opus-5-5
In video mode the locked recording panel is one row: delete, red dot in a progress ring of the 120 s limit, timer, send. The voice panel is unchanged. The preview keeps a smaller gap on a short area. Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru> Assisted-by: Claude-Code:claude-opus-5-5
- Hide the whole recording indicator from TalkBack, so the progress ring does not announce its percentage every second. - Relative padding of the timer, so the gap stays between dot and timer in RTL. - No inset around the progress ring, so it fills its 24dp. - The ring scale is set in code only (PROGRESS_MAX). Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru> Assisted-by: Claude-Code:claude-opus-5-5
…t by the effective state requestReadFilesPermissions() did not request READ_MEDIA_VISUAL_USER_SELECTED on Android 14+, although the manifest declares it, and the result handler looked only at grantResults[0]. With "selected photos" the first permission is denied, so a partial access showed a false refusal. The request is now the set of the attachment sheet (mediaPermissionsToRequest) plus READ_MEDIA_AUDIO from Android 13 on, as before. The result allows the file picker if any requested permission was granted or the effective access (permissionUtil.isFilesPermissionGranted()) is given: before Android 10 the granted READ permission differs from the WRITE one that check looks at. Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru> Assisted-by: Claude-Code:claude-opus-5-5
A video recording result that waited for a new activity was delivered from onCreate, before currentConversation and spreedCapabilities were set: the send path and the preview path could throw. The pending result is now delivered once the initial capabilities state has initialized both; a running recording still reattaches at once. The delete and send buttons of the compact video row announced the voice recording labels to TalkBack. Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru> Assisted-by: Claude-Code:claude-opus-5-5
… the media viewer
- top bar: centered "N of M" counter counted from the newest item, so it
stays put when older items are paged in ("M+" while more may load)
- menu: save to gallery, show all media, show in chat, reply, share,
delete (with confirmation); buttons: forward (file share to another
conversation) and draw (photos only)
- visibility comes from the chat's buildMessageActionsState (new fields
showForwardFile, canSendToConversation); hidden when the message is not
in the local database
- reply/delete/forward/draw run in ChatActivity through its own handlers,
handed over by intent extras and re-checked there
- the file of a forward is not passed in an intent extra (the conversation
list is exported): ChatActivity keeps it in PendingFileForward and hands
over a random key; the entry is taken only after the confirmation
- draw opens the upload preview on the cached file with the drawing editor
started; edits write copies
- SaveToStorageDialogFragment now finds files in the per-file-id cache
- ShareOperationWorker tells the user when a share is refused
Depends on nextcloud#6820 (drawing editor of the attachment preview).
Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru>
Assisted-by: Claude-Code:claude-opus-5-5
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe pull request adds an attachment sheet for recent media, in-app CameraX photo capture, and crop and drawing tools in the attachment preview. It adds voice and video recording controls and CameraX video recording to chat, including recording restoration across activity recreation. The media viewer gains item counters, chat actions, navigation, and file forwarding. Android media-permission handling, image orientation details, and save-file resolution also change. Fixed issue severity: Medium Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to The "Show in chat" navigation gap is fixed. No outstanding merge-blocking risk was found in the reviewed change. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The reviewed flows retain account and conversation binding, permission checks, and forwarding confirmation. No introduced security defect was established. Recovery behavior and coverage of the wider stacked changes remain incomplete. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
Full details: Linked Issues checkExplanation [ Full details: Out of Scope Changes checkExplanation The whole PR also adds in-app photo capture, video-message recording, and a replacement attachment sheet with a recent-media picker. These features do not implement Resolution Move in-app capture, video-message recording, the replacement attachment sheet, the recent-media picker, and their supporting changes to separate PRs. Retain the media-viewer work, drawing support required by
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 |
takePhotoWithInAppCamera() requested the permission and the grant handler only showed a snackbar, so the user had to open the sheet and tap the camera again. The request is now remembered and the camera opens from the grant branch. The flag is cleared by the other path which asks for the same permission (system video capture). Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru> Assisted-by: Claude-Code:claude-opus-5-5
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
app/src/main/java/com/nextcloud/talk/attachmentpreview/FileAttachmentPreviewViewModel.kt (1)
102-112: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueHandle the case where the user removes or replaces the file while a drawing save is in progress.
saveDrawingcapturesdescription.uri. It then callsfinishEditafter the IO work completes. IfoldUriis no longer infiles,replaceFilereturns early.discardIfOwnEdit(oldUri)still runs, and the new edited output is never referenced. The new output file stays in the cache as an orphan. In this case, deletenewUri's file instead.Proposed fix
private fun finishEdit(oldUri: String, newUri: Uri?) { if (newUri == null) { editFailed = true - } else { + } else if (oldUri in files) { replaceFile(oldUri, newUri.toString()) discardIfOwnEdit(oldUri) + } else { + discardIfOwnEdit(newUri.toString()) } isEditing = false }
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Organization UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
e176f2d9-c3fa-4ab6-a85a-d9a593922193
📒 Files selected for processing (108)
app/build.gradle.ktsapp/src/main/AndroidManifest.xmlapp/src/main/java/com/nextcloud/talk/attachmentpreview/AttachmentEditLogic.ktapp/src/main/java/com/nextcloud/talk/attachmentpreview/AttachmentEditing.ktapp/src/main/java/com/nextcloud/talk/attachmentpreview/AttachmentToolBar.ktapp/src/main/java/com/nextcloud/talk/attachmentpreview/AttachmentToolBarPreviews.ktapp/src/main/java/com/nextcloud/talk/attachmentpreview/CameraCaptureActions.ktapp/src/main/java/com/nextcloud/talk/attachmentpreview/CaptionInputBar.ktapp/src/main/java/com/nextcloud/talk/attachmentpreview/DetailVariants.ktapp/src/main/java/com/nextcloud/talk/attachmentpreview/DrawingEditor.ktapp/src/main/java/com/nextcloud/talk/attachmentpreview/DrawingSession.ktapp/src/main/java/com/nextcloud/talk/attachmentpreview/FileAttachmentPreviewFragment.ktapp/src/main/java/com/nextcloud/talk/attachmentpreview/FileAttachmentPreviewScreen.ktapp/src/main/java/com/nextcloud/talk/attachmentpreview/FileAttachmentPreviewViewModel.ktapp/src/main/java/com/nextcloud/talk/attachmentpreview/FileDescriber.ktapp/src/main/java/com/nextcloud/talk/attachmentpreview/FileThumbnailImage.ktapp/src/main/java/com/nextcloud/talk/attachmentpreview/ImageDetailLogic.ktapp/src/main/java/com/nextcloud/talk/attachmentpreview/LargePreview.ktapp/src/main/java/com/nextcloud/talk/attachmentpreview/ThumbnailStrip.ktapp/src/main/java/com/nextcloud/talk/attachmentpreview/ThumbnailStripPreviews.ktapp/src/main/java/com/nextcloud/talk/attachmentsheet/AttachmentAction.ktapp/src/main/java/com/nextcloud/talk/attachmentsheet/AttachmentActionResources.ktapp/src/main/java/com/nextcloud/talk/attachmentsheet/AttachmentSheet.ktapp/src/main/java/com/nextcloud/talk/attachmentsheet/CameraTile.ktapp/src/main/java/com/nextcloud/talk/attachmentsheet/MediaAccess.ktapp/src/main/java/com/nextcloud/talk/attachmentsheet/MediaSelection.ktapp/src/main/java/com/nextcloud/talk/attachmentsheet/RecentMedia.ktapp/src/main/java/com/nextcloud/talk/attachmentsheet/RecentMediaLoader.ktapp/src/main/java/com/nextcloud/talk/camera/CaptureActivity.ktapp/src/main/java/com/nextcloud/talk/camera/CaptureOrientation.ktapp/src/main/java/com/nextcloud/talk/camera/PhotoCamera.ktapp/src/main/java/com/nextcloud/talk/camera/PhotoCaptureLogic.ktapp/src/main/java/com/nextcloud/talk/camera/PhotoCaptureScreen.ktapp/src/main/java/com/nextcloud/talk/camera/TakePhotoInApp.ktapp/src/main/java/com/nextcloud/talk/chat/CameraLens.ktapp/src/main/java/com/nextcloud/talk/chat/ChatActivity.ktapp/src/main/java/com/nextcloud/talk/chat/MessageInputFragment.ktapp/src/main/java/com/nextcloud/talk/chat/MessageInputVoiceRecordingFragment.ktapp/src/main/java/com/nextcloud/talk/chat/RecordButtonGesture.ktapp/src/main/java/com/nextcloud/talk/chat/RecordHintPopup.ktapp/src/main/java/com/nextcloud/talk/chat/RecordInputMode.ktapp/src/main/java/com/nextcloud/talk/chat/RecordingStopCoordinator.ktapp/src/main/java/com/nextcloud/talk/chat/VideoMessageRecorder.ktapp/src/main/java/com/nextcloud/talk/chat/VideoPreviewPlacement.ktapp/src/main/java/com/nextcloud/talk/chat/ui/MessageActionsBottomSheet.ktapp/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.ktapp/src/main/java/com/nextcloud/talk/components/StandardAppBar.ktapp/src/main/java/com/nextcloud/talk/conversationinfo/ConversationInfoActivity.ktapp/src/main/java/com/nextcloud/talk/conversationlist/ConversationsListActivity.ktapp/src/main/java/com/nextcloud/talk/jobs/ShareOperationWorker.ktapp/src/main/java/com/nextcloud/talk/mediaviewer/activities/MediaViewerActivity.ktapp/src/main/java/com/nextcloud/talk/mediaviewer/activities/MediaViewerScreen.ktapp/src/main/java/com/nextcloud/talk/mediaviewer/model/MediaViewerActions.ktapp/src/main/java/com/nextcloud/talk/mediaviewer/model/PendingFileForward.ktapp/src/main/java/com/nextcloud/talk/mediaviewer/viewmodels/MediaViewerViewModel.ktapp/src/main/java/com/nextcloud/talk/shareditems/activities/SharedItemsActivity.ktapp/src/main/java/com/nextcloud/talk/shareditems/adapters/SharedItemsAdapter.ktapp/src/main/java/com/nextcloud/talk/ui/dialog/AttachmentDialog.ktapp/src/main/java/com/nextcloud/talk/ui/dialog/SaveToStorageDialogFragment.ktapp/src/main/java/com/nextcloud/talk/utils/bundle/BundleKeys.ktapp/src/main/java/com/nextcloud/talk/utils/permissions/PlatformPermissionUtilImpl.ktapp/src/main/java/com/nextcloud/talk/utils/preferences/AppPreferences.javaapp/src/main/java/com/nextcloud/talk/utils/preferences/AppPreferencesImpl.ktapp/src/main/res/drawable/bg_record_hint.xmlapp/src/main/res/drawable/ic_baseline_flash_auto_24.xmlapp/src/main/res/drawable/ic_baseline_flash_off_24.xmlapp/src/main/res/drawable/ic_baseline_flash_on_24.xmlapp/src/main/res/drawable/ic_baseline_hd_24.xmlapp/src/main/res/drawable/ic_baseline_speed_24.xmlapp/src/main/res/drawable/ic_record_hint_arrow.xmlapp/src/main/res/drawable/video_recording_dot.xmlapp/src/main/res/layout/activity_chat.xmlapp/src/main/res/layout/dialog_attachment.xmlapp/src/main/res/layout/fragment_message_input_voice_recording.xmlapp/src/main/res/layout/view_message_input.xmlapp/src/main/res/layout/view_record_hint.xmlapp/src/main/res/values/colors.xmlapp/src/main/res/values/dimens.xmlapp/src/main/res/values/strings.xmlapp/src/test/java/com/nextcloud/talk/attachmentpreview/AttachmentSelectionAndNamingTest.ktapp/src/test/java/com/nextcloud/talk/attachmentpreview/AttachmentToolBarLogicTest.ktapp/src/test/java/com/nextcloud/talk/attachmentpreview/DrawingGeometryTest.ktapp/src/test/java/com/nextcloud/talk/attachmentpreview/EditedFileOwnershipTest.ktapp/src/test/java/com/nextcloud/talk/attachmentpreview/FileAttachmentPreviewResultTest.ktapp/src/test/java/com/nextcloud/talk/attachmentpreview/FileAttachmentPreviewViewModelTest.ktapp/src/test/java/com/nextcloud/talk/attachmentpreview/ImageDetailLogicTest.ktapp/src/test/java/com/nextcloud/talk/attachmentpreview/UprightMatrixTest.ktapp/src/test/java/com/nextcloud/talk/attachmentsheet/AttachmentActionTest.ktapp/src/test/java/com/nextcloud/talk/attachmentsheet/MediaAccessTest.ktapp/src/test/java/com/nextcloud/talk/attachmentsheet/MediaSelectionTest.ktapp/src/test/java/com/nextcloud/talk/attachmentsheet/RecentMediaTest.ktapp/src/test/java/com/nextcloud/talk/attachmentsheet/ShareFilePermissionTest.ktapp/src/test/java/com/nextcloud/talk/camera/CaptureModeSettingTest.ktapp/src/test/java/com/nextcloud/talk/camera/CaptureOrientationTest.ktapp/src/test/java/com/nextcloud/talk/camera/PhotoCaptureLogicTest.ktapp/src/test/java/com/nextcloud/talk/chat/RecordButtonGestureTest.ktapp/src/test/java/com/nextcloud/talk/chat/RecordHintPopupPlacementTest.ktapp/src/test/java/com/nextcloud/talk/chat/RecordingStopCoordinatorTest.ktapp/src/test/java/com/nextcloud/talk/chat/ShouldVibrateOnRecordingChangeTest.ktapp/src/test/java/com/nextcloud/talk/chat/VideoFrameAspectTest.ktapp/src/test/java/com/nextcloud/talk/chat/VideoMessageRecordingTest.ktapp/src/test/java/com/nextcloud/talk/chat/VideoPreviewPlacementCompactPanelTest.ktapp/src/test/java/com/nextcloud/talk/chat/VideoPreviewPlacementTest.ktapp/src/test/java/com/nextcloud/talk/chat/VideoRecorderLifecycleTest.ktapp/src/test/java/com/nextcloud/talk/chat/VideoRecordingRecreationTest.ktapp/src/test/java/com/nextcloud/talk/chat/ui/MessageActionsStateTest.ktapp/src/test/java/com/nextcloud/talk/mediaviewer/model/MediaViewerActionsTest.ktapp/src/test/java/com/nextcloud/talk/mediaviewer/model/MediaViewerChatRequestTest.kt
💤 Files with no reviewable changes (2)
- app/src/main/res/layout/dialog_attachment.xml
- app/src/main/java/com/nextcloud/talk/ui/dialog/AttachmentDialog.kt
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 0 remain after this review.
…e loaded The conversation can be set before the capabilities, and the request reads spreedCapabilities through actionsStateFor(), which threw UninitializedPropertyAccessException. The request now stays pending and runs from the initial capabilities state. Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru> Assisted-by: Claude-Code:claude-opus-5-5
The photo entry opens the in-app capture screen, which stays black on a device without camera hardware. It now needs hasCamera like the video entry. Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru> Assisted-by: Claude-Code:claude-opus-5-5
setTargetRotation() dropped a rotation which arrived before start() had built the ImageCapture, so a later photo used the display rotation. The latest rotation is now stored and applied to every ImageCapture that is built. Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru> Assisted-by: Claude-Code:claude-opus-5-5
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
🟡 Minor · Open the pending message after the conversation loads. · ChatActivity.kt:1440-1445
app/src/main/java/com/nextcloud/talk/chat/ChatActivity.kt:1440-1445
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winOpen the pending message after the conversation loads.
If a same-room “Show in chat” intent arrives before
currentConversationloads, this method leaves its target pending. The initial-load observer runsrunPendingMediaRequest()but never callsopenPendingTargetMessage(). The chat therefore does not navigate to the requested message. CallopenPendingTargetMessage()from the initial-load branch as well.
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Organization UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
33a490a5-c437-4ef3-b985-c929299efa01
📒 Files selected for processing (4)
app/src/main/java/com/nextcloud/talk/attachmentsheet/AttachmentAction.ktapp/src/main/java/com/nextcloud/talk/camera/PhotoCamera.ktapp/src/main/java/com/nextcloud/talk/chat/ChatActivity.ktapp/src/test/java/com/nextcloud/talk/attachmentsheet/AttachmentActionTest.kt
🚧 Files skipped from review as they are similar to previous changes (2)
- app/src/main/java/com/nextcloud/talk/attachmentsheet/AttachmentAction.kt
- app/src/test/java/com/nextcloud/talk/attachmentsheet/AttachmentActionTest.kt
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.
…Android 14 With READ_MEDIA_VISUAL_USER_SELECTED and only one of READ_MEDIA_IMAGES/VIDEO granted, the access resolves to FULL and the sheet hid the action. canSelectMoreMedia also covers that mix; media loading is unchanged. Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru> Assisted-by: Claude-Code:claude-opus-5-5
Two edits in the same millisecond got the same path, so the second result replaced nothing and the old file was deleted. The output is now reserved with createNewFile() and gets a numeric suffix when the name is taken. Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru> Assisted-by: Claude-Code:claude-opus-5-5
… later For an already open chat whose conversation was not loaded yet, the target message stayed pending and was never opened. The joined-room branch now calls openPendingTargetMessage() next to runPendingMediaRequest(). Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru> Assisted-by: Claude-Code:claude-opus-5-5
Contributor
Author
|
@coderabbitai Re the outside-diff finding on Confirmed and fixed in ce420ca: the initial-load branch now calls This reply and the fix were prepared with the help of an AI assistant (Claude Code) and reviewed before posting. |
Contributor
Author
|
@coderabbitai Reply to the nitpick on Confirmed, the output file stays in the cache in that case. The race needs the user to remove the page within the short save of a drawing. The file lives in This reply was prepared with the help of an AI assistant (Claude Code) and reviewed before posting. |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Organization UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
ac66d67d-ad9d-4afb-870b-283de6a1830c
📒 Files selected for processing (7)
app/src/main/java/com/nextcloud/talk/attachmentpreview/AttachmentEditing.ktapp/src/main/java/com/nextcloud/talk/attachmentpreview/EditNameClaim.ktapp/src/main/java/com/nextcloud/talk/attachmentsheet/AttachmentSheet.ktapp/src/main/java/com/nextcloud/talk/attachmentsheet/MediaAccess.ktapp/src/main/java/com/nextcloud/talk/chat/ChatActivity.ktapp/src/test/java/com/nextcloud/talk/attachmentpreview/EditNameClaimTest.ktapp/src/test/java/com/nextcloud/talk/attachmentsheet/CanSelectMoreMediaTest.kt
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
fd '^ChatActivity\.kt$' .
file="app/src/main/java/com/nextcloud/talk/chat/ChatActivity.kt"
ast-grep outline "$file" --match 'onCreate|openPendingTargetMessage|applyIntentTargetsToJoinedRoom|runPendingMediaRequest' --view expanded
rg -n -C 12 'openPendingTargetMessage\(|applyIntentTargetsToJoinedRoom\(|runPendingMediaRequest\(' "$file"Length of output: 7238
✏️ Learnings added
If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! You are interacting with an AI system. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
fd '^FileAttachmentPreviewViewModel\.kt$' .
file="app/src/main/java/com/nextcloud/talk/attachmentpreview/FileAttachmentPreviewViewModel.kt"
ast-grep outline "$file" --match 'saveDrawing|replaceFile|applyCrop' --view expanded
sed -n '1,240p' "$file"Length of output: 8113
I accept leaving this low-impact cache cleanup out of Would you like me to open a follow-up issue for cleanup if the source disappears during a drawing save? ✏️ Learnings added
You are interacting with an AI system. |
…st returns When the system grants without a dialog there is no pause/resume, so the sheet kept the old grants and the placeholder. The request result now bumps a key that the grants and the media list depend on. Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru> Assisted-by: Claude-Code:claude-opus-5-5
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.
Closes #6822
Fixes #6806
Depends on #6820: "Draw" opens its attachment preview with the brush. The branch is stacked on #6820, so the diff against
masteralso shows #6818–#6820; the change of this PR is the last commit.🖼️ Screenshots
Screenshots from the Huawei DEL-LX9 will follow in a comment.
Feature
Media viewer:
Visibility rules are not copied. The viewer uses the same
buildMessageActionsStateas the message menu, with the message and the conversation from the local database. When the item is not in the database (loaded from the server only), "Reply", "Delete", "Forward" and "Draw" are hidden. The rules are re-evaluated when the online state changes.The chat runs the actions with its own handlers. The viewer starts
ChatActivitywithCLEAR_TOP | SINGLE_TOPand the action plus the message id. The chat checks the permissions again before it runs the action. The request survives a configuration change, but it does not run again after process death.ShareOperationWorker.shareFile— the share step that ends an upload. The file is shared into the target conversation again, without a new upload. The caption is sent asmetaData. A server error is now shown to the user; before, the error was silent (this also affects "File from cloud").shared_attachments.Also fixed (#6806): "Save" from the viewer looked for the file in the cache root, but the viewer stores files in folders per file id, so an empty file was created. The dialog now resolves the source by file id.
Safe passing of the file path when forwarding
This is part of the design of the new "Forward" action, not a fix of an existing vulnerability: there is no file forwarding in
master.ConversationsListActivityis exported, so any app can start it with any extras. The file path is therefore not put into the intent. The chat reads the path from the message in the database, stores it in process memory (PendingFileForward) and passes only a random one-time key. A forged or unknown key sends nothing. This way another app cannot make Talk share a private file.How to check on a device
Tested by the author on a Huawei DEL-LX9 with a fork build (
develop) that has this feature together with other fork changes. Unit tests:MediaViewerActionsTest,MediaViewerChatRequestTest,MessageActionsStateTest.🚧 TODO
approvedlabel🏁 Checklist
/backport to stable-xx.x🤖 AI (if applicable)
The change was written with the help of Claude Code (Anthropic). The author reviewed the code, built it and ran the unit tests, and checked the behavior on a device with the fork build.