Sitelet https://github.com/nextcloud/talk-android/pull/6820
Skip to content

feat(chat): take photos with an in-app camera and edit attachments before sending - #6820

Open
ToteMeiSter wants to merge 25 commits into
nextcloud:masterfrom
ToteMeiSter:feat/in-app-camera
Open

ToteMeiSter wants to merge 25 commits into
nextcloud:masterfrom
ToteMeiSter:feat/in-app-camera

Conversation

@ToteMeiSter

Copy link
Copy Markdown
Contributor

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

🏚️ Before 🏡 After
Photos are taken by the system camera app; the preview before sending has an inline layout A full-screen CameraX capture screen; a full-screen attachment preview with crop, rotate and drawing

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:

  • full-screen capture screen: shutter, front/back lens, flash off/auto/on, close; it asks CameraX for the full 4:3 resolution;
  • the controls stay on the device body when the device is rotated;
  • the photo goes to the attachment preview; the camera tile and the camera entry of the sheet, and the photo button of the preview use it. The system ACTION_IMAGE_CAPTURE path is removed, video capture is unchanged;
  • rotationForDeviceOrientation() is shared with the video message recorder (the copy there is removed).

Commit 2, attachment preview:

  • the photo fills the screen with a floating caption, a compact panel with the SD/HD switch and View-only choice, a round send button;
  • a tick mark per file excludes it from sending; "add more" menu (gallery, photo, video);
  • crop and rotate through the bundled uCrop (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;
  • the preview result is delivered through the fragment manager, so a send after the activity was recreated (rotation) still uploads the files;
  • the original image size is shown in display orientation.

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 in AppPreferences.

Structure

Three commits on top of the attachment sheet PR:

  1. feat(chat): take photos with an in-app camera instead of the system one
  2. feat(chat): full-screen attachment preview with photo editing
  3. feat(camera): switch between fast and maximum quality shutter

How to check on a device

  1. Tap the camera tile or the camera entry in the attachment sheet: the capture screen opens. Take a photo, switch the lens and the flash, rotate the device.
  2. The photo opens in the attachment preview: crop, rotate, draw, exclude a file, add more files, edit the caption, send.
  3. Rotate the device in the preview and send: the files are uploaded.
  4. Switch Fast/Quality and compare the shutter delay and the file size.

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

  • ⛑️ Tests (unit and/or integration) are included or not needed
  • 🔖 Capability is checked or not needed
  • 🔙 Backport requests are created or not needed: /backport to stable-xx.x
  • 📅 Milestone is set
  • 🌸 PR title is meaningful (if it should be in the changelog: is it meaningful to users?)

🤖 AI (if applicable)

  • The content of this PR was partly or fully generated using AI

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.

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
@coderabbitai

coderabbitai Bot commented Oct 4, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 9e8fa1f3-84af-45ed-9db0-6f20126dc345
📥 Commits

Reviewing files that changed from the base of the PR and between 3d16383 and 17430de.

📒 Files selected for processing (1)
  • app/src/main/java/com/nextcloud/talk/attachmentsheet/AttachmentSheet.kt

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 1 remain after this review.


📝 Walkthrough

Walkthrough

The 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 17430

The attachment sheet’s permission flow does not block first-time access. No merge-blocking issue was established in the selected change.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 17430

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
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The inspected selection path operates within the app's Android-authorized local media scope: granted image or video collections, or the selected-media subset. Its internal URI callback does not establish a new remote caller, service privilege, or cross-account authority.

Trust Boundaries and Controls

  • observed — For uncached content URIs, the preview's file helper reads through ContentResolver.openInputStream and handles SecurityException. Existing cache entries are reused by display-derived filename, so not every request performs a fresh provider read. This cache policy is unchanged from the full PR base; no PR-introduced increase in its exposure was established.

Resilience and Maintainability Implications

  • observed — Sheet handoff and upload dispatch are separate transitions. ChatActivity first opens the preview; upload dispatch follows the preview's fragment result, emitted through its onSend callback. A stale sheet URI callback alone therefore does not demonstrate immediate disclosure.

Hardening Proposals

  • proposed — Consider associating media and selection with the current permission-refresh generation and allowing handoff only from a completed, current snapshot. This would make revocation and partial-access refresh semantics explicit rather than relying on eventual reconciliation.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 32.32% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 393 functions across 55 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main changes: an in-app camera and attachment editing before sending.
Description check ✅ Passed The description covers the template sections and provides feature details, testing information, and a completed checklist. The milestone remains unchecked, but this is a non-critical omission.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@mahibi

mahibi commented Oct 5, 2026

Copy link
Copy Markdown
Collaborator

Hi @ToteMeiSter thank you for your contributions.
We will review them soon

…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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 value

Make the documented tie order explicit.

The KDoc says equal dates keep images before videos. sortedByDescending is stable, so the result depends on the order of the concatenation images + videos. The behavior is correct now. A thenBy { 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 value

Video start failure leaves the record button state inconsistent.

startVideoRecording() can return true after it calls onVideoRecordingStarted(). That call sets _getVoiceRecordingInProgress only if the audio-focus callback runs synchronously. If focus is denied, startVideoRecording returns false. In that case beginRecording returns early, which is correct.

There is a second case. startVideoRecording() returns false when recorder.isActive is true, because file is 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
📥 Commits

Reviewing files that changed from the base of the PR and between 1e9c2f1 and 8922f97.

📒 Files selected for processing (91)
  • app/build.gradle.kts
  • app/src/main/AndroidManifest.xml
  • app/src/main/java/com/nextcloud/talk/attachmentpreview/AttachmentEditLogic.kt
  • app/src/main/java/com/nextcloud/talk/attachmentpreview/AttachmentEditing.kt
  • app/src/main/java/com/nextcloud/talk/attachmentpreview/AttachmentToolBar.kt
  • app/src/main/java/com/nextcloud/talk/attachmentpreview/AttachmentToolBarPreviews.kt
  • app/src/main/java/com/nextcloud/talk/attachmentpreview/CameraCaptureActions.kt
  • app/src/main/java/com/nextcloud/talk/attachmentpreview/CaptionInputBar.kt
  • app/src/main/java/com/nextcloud/talk/attachmentpreview/DetailVariants.kt
  • app/src/main/java/com/nextcloud/talk/attachmentpreview/DrawingEditor.kt
  • app/src/main/java/com/nextcloud/talk/attachmentpreview/DrawingSession.kt
  • app/src/main/java/com/nextcloud/talk/attachmentpreview/FileAttachmentPreviewFragment.kt
  • app/src/main/java/com/nextcloud/talk/attachmentpreview/FileAttachmentPreviewScreen.kt
  • app/src/main/java/com/nextcloud/talk/attachmentpreview/FileAttachmentPreviewViewModel.kt
  • app/src/main/java/com/nextcloud/talk/attachmentpreview/FileDescriber.kt
  • app/src/main/java/com/nextcloud/talk/attachmentpreview/FileThumbnailImage.kt
  • app/src/main/java/com/nextcloud/talk/attachmentpreview/ImageDetailLogic.kt
  • app/src/main/java/com/nextcloud/talk/attachmentpreview/LargePreview.kt
  • app/src/main/java/com/nextcloud/talk/attachmentpreview/ThumbnailStrip.kt
  • app/src/main/java/com/nextcloud/talk/attachmentpreview/ThumbnailStripPreviews.kt
  • app/src/main/java/com/nextcloud/talk/attachmentsheet/AttachmentAction.kt
  • app/src/main/java/com/nextcloud/talk/attachmentsheet/AttachmentActionResources.kt
  • app/src/main/java/com/nextcloud/talk/attachmentsheet/AttachmentSheet.kt
  • app/src/main/java/com/nextcloud/talk/attachmentsheet/CameraTile.kt
  • app/src/main/java/com/nextcloud/talk/attachmentsheet/MediaAccess.kt
  • app/src/main/java/com/nextcloud/talk/attachmentsheet/MediaSelection.kt
  • app/src/main/java/com/nextcloud/talk/attachmentsheet/RecentMedia.kt
  • app/src/main/java/com/nextcloud/talk/attachmentsheet/RecentMediaLoader.kt
  • app/src/main/java/com/nextcloud/talk/camera/CaptureActivity.kt
  • app/src/main/java/com/nextcloud/talk/camera/CaptureOrientation.kt
  • app/src/main/java/com/nextcloud/talk/camera/PhotoCamera.kt
  • app/src/main/java/com/nextcloud/talk/camera/PhotoCaptureLogic.kt
  • app/src/main/java/com/nextcloud/talk/camera/PhotoCaptureScreen.kt
  • app/src/main/java/com/nextcloud/talk/camera/TakePhotoInApp.kt
  • app/src/main/java/com/nextcloud/talk/chat/CameraLens.kt
  • app/src/main/java/com/nextcloud/talk/chat/ChatActivity.kt
  • app/src/main/java/com/nextcloud/talk/chat/MessageInputFragment.kt
  • app/src/main/java/com/nextcloud/talk/chat/MessageInputVoiceRecordingFragment.kt
  • app/src/main/java/com/nextcloud/talk/chat/RecordButtonGesture.kt
  • app/src/main/java/com/nextcloud/talk/chat/RecordHintPopup.kt
  • app/src/main/java/com/nextcloud/talk/chat/RecordInputMode.kt
  • app/src/main/java/com/nextcloud/talk/chat/RecordingStopCoordinator.kt
  • app/src/main/java/com/nextcloud/talk/chat/VideoMessageRecorder.kt
  • app/src/main/java/com/nextcloud/talk/chat/VideoPreviewPlacement.kt
  • app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt
  • app/src/main/java/com/nextcloud/talk/ui/dialog/AttachmentDialog.kt
  • app/src/main/java/com/nextcloud/talk/utils/permissions/PlatformPermissionUtilImpl.kt
  • app/src/main/java/com/nextcloud/talk/utils/preferences/AppPreferences.java
  • app/src/main/java/com/nextcloud/talk/utils/preferences/AppPreferencesImpl.kt
  • app/src/main/res/drawable/bg_record_hint.xml
  • app/src/main/res/drawable/ic_baseline_flash_auto_24.xml
  • app/src/main/res/drawable/ic_baseline_flash_off_24.xml
  • app/src/main/res/drawable/ic_baseline_flash_on_24.xml
  • app/src/main/res/drawable/ic_baseline_hd_24.xml
  • app/src/main/res/drawable/ic_baseline_speed_24.xml
  • app/src/main/res/drawable/ic_record_hint_arrow.xml
  • app/src/main/res/drawable/video_recording_dot.xml
  • app/src/main/res/layout/activity_chat.xml
  • app/src/main/res/layout/dialog_attachment.xml
  • app/src/main/res/layout/fragment_message_input_voice_recording.xml
  • app/src/main/res/layout/view_message_input.xml
  • app/src/main/res/layout/view_record_hint.xml
  • app/src/main/res/values/colors.xml
  • app/src/main/res/values/dimens.xml
  • app/src/main/res/values/strings.xml
  • app/src/test/java/com/nextcloud/talk/attachmentpreview/AttachmentSelectionAndNamingTest.kt
  • app/src/test/java/com/nextcloud/talk/attachmentpreview/AttachmentToolBarLogicTest.kt
  • app/src/test/java/com/nextcloud/talk/attachmentpreview/DrawingGeometryTest.kt
  • app/src/test/java/com/nextcloud/talk/attachmentpreview/EditedFileOwnershipTest.kt
  • app/src/test/java/com/nextcloud/talk/attachmentpreview/FileAttachmentPreviewResultTest.kt
  • app/src/test/java/com/nextcloud/talk/attachmentpreview/FileAttachmentPreviewViewModelTest.kt
  • app/src/test/java/com/nextcloud/talk/attachmentpreview/ImageDetailLogicTest.kt
  • app/src/test/java/com/nextcloud/talk/attachmentpreview/UprightMatrixTest.kt
  • app/src/test/java/com/nextcloud/talk/attachmentsheet/AttachmentActionTest.kt
  • app/src/test/java/com/nextcloud/talk/attachmentsheet/MediaAccessTest.kt
  • app/src/test/java/com/nextcloud/talk/attachmentsheet/MediaSelectionTest.kt
  • app/src/test/java/com/nextcloud/talk/attachmentsheet/RecentMediaTest.kt
  • app/src/test/java/com/nextcloud/talk/attachmentsheet/ShareFilePermissionTest.kt
  • app/src/test/java/com/nextcloud/talk/camera/CaptureModeSettingTest.kt
  • app/src/test/java/com/nextcloud/talk/camera/CaptureOrientationTest.kt
  • app/src/test/java/com/nextcloud/talk/camera/PhotoCaptureLogicTest.kt
  • app/src/test/java/com/nextcloud/talk/chat/RecordButtonGestureTest.kt
  • app/src/test/java/com/nextcloud/talk/chat/RecordHintPopupPlacementTest.kt
  • app/src/test/java/com/nextcloud/talk/chat/RecordingStopCoordinatorTest.kt
  • app/src/test/java/com/nextcloud/talk/chat/ShouldVibrateOnRecordingChangeTest.kt
  • app/src/test/java/com/nextcloud/talk/chat/VideoFrameAspectTest.kt
  • app/src/test/java/com/nextcloud/talk/chat/VideoMessageRecordingTest.kt
  • app/src/test/java/com/nextcloud/talk/chat/VideoPreviewPlacementCompactPanelTest.kt
  • app/src/test/java/com/nextcloud/talk/chat/VideoPreviewPlacementTest.kt
  • app/src/test/java/com/nextcloud/talk/chat/VideoRecorderLifecycleTest.kt
  • app/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.

Comment thread app/src/main/java/com/nextcloud/talk/chat/ChatActivity.kt
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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to GitHub limitations.

⚠️ Outside diff range comments (1)

🟡 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 win

Reserve a collision-free output path for each edit.

createEditOutputFile uses millisecond precision without checking the path. Two edit entrypoints can therefore receive the same path. The second replaceFile call becomes a no-op, but finishEdit still 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
📥 Commits

Reviewing files that changed from the base of the PR and between fb46aa9 and 8e475a1.

📒 Files selected for processing (4)
  • app/src/main/java/com/nextcloud/talk/attachmentsheet/AttachmentAction.kt
  • app/src/main/java/com/nextcloud/talk/camera/PhotoCamera.kt
  • app/src/main/java/com/nextcloud/talk/chat/ChatActivity.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; 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
@ToteMeiSter

Copy link
Copy Markdown
Contributor Author

@coderabbitai Re the outside-diff finding on AttachmentEditing.kt 48-52 of this review (collision-free output path for each edit):

Fixed in 3d16383: createEditOutputFile now reserves the file with createNewFile() and adds a numeric suffix to the millisecond stamp when the name is taken (bounded attempts; IOException / SecurityException return null). The suffix stays inside the stamp, so ownEditedName still recognizes the file. Every failure path already deletes the reserved file (cancelled crop, failed drawing, unshareable crop), and uCrop opens the destination with truncation. The pure part is claimFirstFree, covered by EditNameClaimTest.

This reply and the fix were prepared with the help of an AI assistant (Claude Code) and reviewed before posting.

@ToteMeiSter

Copy link
Copy Markdown
Contributor Author

@coderabbitai Replies to the two nitpicks of this review:

RecentMedia.kt 30-31, tie order. The order is already enforced by a test: RecentMediaTest merges an image and a video with the same date and asserts that the image comes first. A refactor that breaks the order of images + videos would fail that test, so I keep the current code.

MessageInputFragment.kt 710-723, no feedback while the previous video finalizes. Right, a hold during the finalization of the previous video starts nothing. The window is short (CameraX finalizes the file), and a second hold works. A hint would only flash for that moment, so I leave it out of this PR. The state stays consistent: beginRecording returns before any UI or timer change.

This reply was prepared with the help of an AI assistant (Claude Code) and reviewed before posting.

@coderabbitai

coderabbitai Bot commented Oct 5, 2026

Copy link
Copy Markdown

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.kt

Length of output: 9228


@ToteMeiSter Thanks for the clarification.

  • Tie order: RecentMediaTest.equalDatesKeepImagesBeforeVideos() already asserts image-first ordering for equal dates. I missed that coverage and withdraw the tie-order nitpick.
  • Finalization feedback: beginRecording returns at Line 713 before changing the recording timestamp, timer, or recording UI. I accept your decision to omit a hint from this PR. No change is needed for this nitpick.

✏️ Learnings added
Learnt from: ToteMeiSter
URL: https://github.com/nextcloud/talk-android/pull/6820

Timestamp: 2026-10-05T13:49:58.741Z
Learning: In nextcloud/talk-android, app/src/main/java/com/nextcloud/talk/attachmentsheet/RecentMedia.kt uses images + videos followed by sortedByDescending to preserve image-first ordering for equal dates. The Kotlin test RecentMediaTest.equalDatesKeepImagesBeforeVideos in app/src/test/java/com/nextcloud/talk/attachmentsheet/RecentMediaTest.kt explicitly covers this contract. Do not request duplicate tie-order coverage.

Learnt from: ToteMeiSter
URL: https://github.com/nextcloud/talk-android/pull/6820

Timestamp: 2026-10-05T13:49:58.741Z
Learning: For the in-app camera PR #6820 in nextcloud/talk-android, the author intentionally omits feedback when a video-recording hold occurs during the previous video's brief finalization window. In app/src/main/java/com/nextcloud/talk/chat/MessageInputFragment.kt, beginRecording returns when startVideoRecording returns false, before changing the recording timestamp, timer, or recording UI. The author reports that a subsequent hold works and considers a transient hint unnecessary for this PR.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

You are interacting with an AI system.

@coderabbitai

coderabbitai Bot commented Oct 5, 2026

Copy link
Copy Markdown

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.kt

Length of output: 10777


@ToteMeiSter Thanks for the fix. I checked createEditOutputFile and EditNameClaimTest.

  • createNewFile() reserves each output file atomically.
  • Numeric suffixes handle collisions, with a limit of 100 attempts.
  • IOException and SecurityException return null.
  • The tests cover name selection, collision retries, attempt exhaustion, and numbered edit-name recognition.

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
@ToteMeiSter

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Oct 5, 2026 •

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews AI assisted

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants