Repository navigation
feat(chat): take photos with an in-app camera and edit attachments before sending - #6820
ToteMeiSter wants to merge 25 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
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (1)
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 1 remain after this review. 📝 WalkthroughWalkthroughThe pull request replaces the attachment dialog with a Compose media sheet and adds Android media-permission handling and in-app CameraX photo capture. The attachment preview gains selection, cropping, drawing, EXIF-aware image details, and Fragment Result delivery. Chat recording gains voice/video mode switching, hold-based controls, retained CameraX video recording, activity-recreation recovery, and an in-chat preview. The changes also add preferences, layouts, resources, and tests. Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to The attachment sheet’s permission flow does not block first-time access. No merge-blocking issue was established in the selected change. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The inspected media-selection flow remains inside the app and retains Android permission controls and a separate attachment preview. No new unauthorized disclosure was established. Permission-change timing and the broader capture and editing lifecycles remain incompletely verified. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
|
Hi @ToteMeiSter thank you for your contributions. |
…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
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
app/src/main/java/com/nextcloud/talk/attachmentsheet/RecentMedia.kt (1)
30-31: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMake the documented tie order explicit.
The KDoc says equal dates keep images before videos.
sortedByDescendingis stable, so the result depends on the order of the concatenationimages + videos. The behavior is correct now. AthenBy { it.isVideo }comparator would state the rule directly in the code.app/src/main/java/com/nextcloud/talk/chat/MessageInputFragment.kt (1)
710-723: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueVideo start failure leaves the record button state inconsistent.
startVideoRecording()can return true after it callsonVideoRecordingStarted(). That call sets_getVoiceRecordingInProgressonly if the audio-focus callback runs synchronously. If focus is denied,startVideoRecordingreturns false. In that casebeginRecordingreturns early, which is correct.There is a second case.
startVideoRecording()returns false whenrecorder.isActiveis true, becausefileis then null. This happens when a previous video is still finalizing. The user holds the button and nothing happens, with no feedback. A short hint would let the user know why nothing started.
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Organization UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
603e42a8-1f8d-4216-ae59-501d7ec87fa5
📒 Files selected for processing (91)
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/viewmodels/ChatViewModel.ktapp/src/main/java/com/nextcloud/talk/ui/dialog/AttachmentDialog.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.kt
💤 Files with no reviewable changes (2)
- app/src/main/java/com/nextcloud/talk/ui/dialog/AttachmentDialog.kt
- app/src/main/res/layout/dialog_attachment.xml
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.
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
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 · Reserve a collision-free output path for each edit. · AttachmentEditing.kt:48-52
app/src/main/java/com/nextcloud/talk/attachmentpreview/AttachmentEditing.kt:48-52
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winReserve a collision-free output path for each edit.
createEditOutputFileuses millisecond precision without checking the path. Two edit entrypoints can therefore receive the same path. The secondreplaceFilecall becomes a no-op, butfinishEditstill deletes the old URI. The preview then retains a URI whose edited file was deleted.Suggested fix
- * Creates the (not yet existing) output file of an edit in the cache dir shared through the app's - * FileProvider; null when that directory is unavailable (anything else would be unshareable). + * Reserves a collision-free output file of an edit in the cache dir shared through the app's + * FileProvider; null when the directory or a unique path is unavailable. internal fun createEditOutputFile(context: Context, sourceName: String, sourceMimeType: String?): File? { val directory = FileUtils.getSharedAttachmentsDirectory(context.cacheDir) ?: return null val stamp = SimpleDateFormat(EDIT_FILE_STAMP_PATTERN, Locale.ROOT).format(Date()) val extension = if (editOutputIsPng(sourceMimeType)) "png" else "jpg" - return File(directory, editedFileName(sourceName, stamp, extension)) + var attempt = 0 + while (attempt < Int.MAX_VALUE) { + val suffix = if (attempt == 0) stamp else "$stamp-$attempt" + val file = File(directory, editedFileName(sourceName, suffix, extension)) + try { + if (file.createNewFile()) return file + } catch (_: IOException) { + return null + } catch (_: SecurityException) { + return null + } + attempt++ + } + return null }
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Organization UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
8903c8cc-c96d-4444-af28-46984458831e
📒 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
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 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
|
@coderabbitai Re the outside-diff finding on Fixed in 3d16383: This reply and the fix were prepared with the help of an AI assistant (Claude Code) and reviewed before posting. |
|
@coderabbitai Replies to the two nitpicks of this review:
This reply was prepared with the help of an AI assistant (Claude Code) and reviewed before posting. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
fd '^(RecentMedia|RecentMediaTest|MessageInputFragment)\.kt$'
ast-grep outline app/src/main/java/com/nextcloud/talk/attachmentsheet/RecentMedia.kt
ast-grep outline app/src/test/java/com/nextcloud/talk/attachmentsheet/RecentMediaTest.kt
ast-grep outline app/src/main/java/com/nextcloud/talk/chat/MessageInputFragment.kt --match 'beginRecording'
sed -n '1,100p' app/src/main/java/com/nextcloud/talk/attachmentsheet/RecentMedia.kt
sed -n '1,100p' app/src/test/java/com/nextcloud/talk/attachmentsheet/RecentMediaTest.kt
rg -n -A65 -B8 'fun beginRecording\(' app/src/main/java/com/nextcloud/talk/chat/MessageInputFragment.ktLength of output: 9228
✏️ Learnings added
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 '^(AttachmentEditing|EditNameClaim|EditNameClaimTest)\.kt$'
ast-grep outline app/src/main/java/com/nextcloud/talk/attachmentpreview/AttachmentEditing.kt
ast-grep outline app/src/main/java/com/nextcloud/talk/attachmentpreview/EditNameClaim.kt
ast-grep outline app/src/test/java/com/nextcloud/talk/attachmentpreview/EditNameClaimTest.kt
sed -n '1,120p' app/src/main/java/com/nextcloud/talk/attachmentpreview/AttachmentEditing.kt
cat -n app/src/main/java/com/nextcloud/talk/attachmentpreview/EditNameClaim.kt
cat -n app/src/test/java/com/nextcloud/talk/attachmentpreview/EditNameClaimTest.ktLength of output: 10777
This addresses the collision-free output-path finding. I inspected the tests but did not run them. 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. |
…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
|
@coderabbitai review |
✅ Action performedReview finished.
|
Refs #6815
This is a feature PR; the issue is not approved yet, the PR is opened for discussion.
Depends on #6819
(and through it on the video message PR and the two voice recording fixes)
🖼️ Screenshots
Why
Upstream removed the built-in camera in #6429 / #2461 and uses the system camera app. Here CameraX is used again for photos: the result goes straight to the attachment preview, without the confirmation step of the camera app, and works the same on all devices (some vendor camera apps do not return a result reliably). This is the maintainers' decision; if the system camera is preferred, the PR can be closed or reduced to the preview commit.
Feature
Commit 1, in-app camera:
ACTION_IMAGE_CAPTUREpath is removed, video capture is unchanged;rotationForDeviceOrientation()is shared with the video message recorder (the copy there is removed).Commit 2, attachment preview:
setMaxBitmapSize(4096), otherwise uCrop 2.2.11 downscales the picture to the screen size) and brush drawing on a Compose canvas, burned into a full-resolution copy;Commit 3, shutter mode: a Fast / Quality switch on the capture screen. Fast (
CAPTURE_MODE_MINIMIZE_LATENCY) is the default; Quality is the maximum quality capture mode (JPEG without loss, about 3 times the file size). The choice is kept inAppPreferences.Structure
Three commits on top of the attachment sheet PR:
feat(chat): take photos with an in-app camera instead of the system onefeat(chat): full-screen attachment preview with photo editingfeat(camera): switch between fast and maximum quality shutterHow to check on a device
Tested by the author on a Huawei DEL-LX9 with a fork build that has this feature together with other fork changes; this branch itself was built and unit-tested.
🚧 TODO
🏁 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.