Repository navigation
feat(chat): replace the attachment menu with a media bottom sheet - #6819
ToteMeiSter wants to merge 23 commits into
Conversation
|
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:
📝 Walkthrough
Priority: ⬇️ Low Merge Risk: 🟡 Moderate · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 20.70% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 343 functions across 43 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly and concisely describes the primary change: replacing the attachment menu with a media bottom sheet. Description check The description follows the repository template and provides screenshots, feature details, testing information, TODO items, checklist status, AI disclosure, and implementation context. The milestone c… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Description check
-
Explanation
The description follows the repository template and provides screenshots, feature details, testing information, TODO items, checklist status, AI disclosure, and implementation context. The milestone checkbox is not marked, but this is a non-critical omission.
-
✨ Finishing Touches: 💡 1
-
⚔️ Resolve merge conflicts: 💡
-
- Resolve merge conflict in branch
feat/attachment-sheet
- Resolve merge conflict in branch
-
- 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.
Comment @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Organization UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
cad5ab06-8c7d-4c28-b0e2-80f15e84101f
📒 Files selected for processing (43)
app/build.gradle.ktsapp/src/main/AndroidManifest.xmlapp/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/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/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_record_hint_arrow.xmlapp/src/main/res/layout/activity_chat.xmlapp/src/main/res/layout/dialog_attachment.xmlapp/src/main/res/layout/view_message_input.xmlapp/src/main/res/layout/view_record_hint.xmlapp/src/main/res/values/dimens.xmlapp/src/main/res/values/strings.xmlapp/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/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/VideoMessageRecordingTest.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/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.
|
Hi @ToteMeiSter thank you for your contributions. |
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 · Keep reselection available for mixed Android 14 grants. · MediaAccess.kt:18-39
app/src/main/java/com/nextcloud/talk/attachmentsheet/MediaAccess.kt:18-39
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep reselection available for mixed Android 14 grants.
When image access is full, video access is selected-only, and
READ_MEDIA_VISUAL_USER_SELECTEDis granted,resolveMediaAccessreturnsFULL.AttachmentSheetBodythen omits the “Select more” callback, so users cannot expand the selected videos from the sheet. The existinggrantedMediaPermissions(context)helper can supply the per-type grants for this check; keepFULLunchanged for media loading.Suggested fix
- val access = remember(refreshKey) { currentMediaAccess(context) } + val grantedPermissions = remember(refreshKey) { grantedMediaPermissions(context) } + val access = remember(refreshKey) { + resolveMediaAccess(Build.VERSION.SDK_INT, grantedPermissions) + } + val canSelectMore = access == MediaAccess.PARTIAL || + (Build.VERSION.SDK_INT >= Build.VERSION_CODES.UPSIDE_DOWN_CAKE && + READ_MEDIA_VISUAL_USER_SELECTED in grantedPermissions && + (android.Manifest.permission.READ_MEDIA_IMAGES !in grantedPermissions || + android.Manifest.permission.READ_MEDIA_VIDEO !in grantedPermissions)) @@ - onSelectMore = permissionRequest.takeIf { access == MediaAccess.PARTIAL } + onSelectMore = permissionRequest.takeIf { canSelectMore }
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Organization UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
f2b0ae11-95a7-42a9-8de4-3692cf68f739
📒 Files selected for processing (19)
app/src/main/java/com/nextcloud/talk/attachmentsheet/MediaAccess.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/VideoMessageRecorder.ktapp/src/main/java/com/nextcloud/talk/chat/VideoPreviewPlacement.ktapp/src/main/res/drawable/video_recording_dot.xmlapp/src/main/res/layout/activity_chat.xmlapp/src/main/res/layout/fragment_message_input_voice_recording.xmlapp/src/main/res/values/colors.xmlapp/src/main/res/values/dimens.xmlapp/src/test/java/com/nextcloud/talk/attachmentsheet/MediaAccessTest.ktapp/src/test/java/com/nextcloud/talk/attachmentsheet/ShareFilePermissionTest.ktapp/src/test/java/com/nextcloud/talk/chat/RecordButtonGestureTest.ktapp/src/test/java/com/nextcloud/talk/chat/VideoFrameAspectTest.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/VideoRecordingRecreationTest.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/test/java/com/nextcloud/talk/attachmentsheet/MediaAccessTest.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.
|
@coderabbitai Re the outside-diff finding on Confirmed and fixed in b8eaef7: the new This reply and the fix were 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 '^(MediaAccess|CanSelectMoreMediaTest|AttachmentSheet)\.kt$'
ast-grep outline app/src/main/java/com/nextcloud/talk/attachmentsheet/MediaAccess.kt
ast-grep outline app/src/test/java/com/nextcloud/talk/attachmentsheet/CanSelectMoreMediaTest.kt
ast-grep outline app/src/main/java/com/nextcloud/talk/attachmentsheet/AttachmentSheet.kt
cat -n app/src/main/java/com/nextcloud/talk/attachmentsheet/MediaAccess.kt
cat -n app/src/test/java/com/nextcloud/talk/attachmentsheet/CanSelectMoreMediaTest.kt
rg -n -C5 'canSelectMoreMedia\(|resolveMediaAccess\(|RecentMediaLoader\.load\(|MediaAccess\.(FULL|PARTIAL|NONE)' app/src/main/java/com/nextcloud/talk/attachmentsheet/AttachmentSheet.ktLength of output: 13523 🏁 Script executed: #!/bin/bash
set -euo pipefail
sed -n '155,190p' app/src/main/java/com/nextcloud/talk/attachmentsheet/AttachmentSheet.ktLength of output: 1871
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. |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
🟡 Changes recommended
Recording lifecycle, overlapping starts, permission handling, and accessible mode switching have unresolved correctness issues.
6 open findings
What changed in this PR
Replaces Talk’s attachment dialog with a Compose media sheet, building on the stacked video-message recording work. Feature acceptance and Google Play policy remain under discussion.
Changes:
- Adds recent-media selection, a camera preview tile, and existing attachment actions.
- Supports Android 14 partial media access.
- Includes video recording, lifecycle handling, UI resources, and tests.
| File | Description |
|---|---|
| app/src/test/java/com/nextcloud/talk/chat/VideoRecordingRecreationTest.kt | Tests recording restoration and outcomes. |
| app/src/test/java/com/nextcloud/talk/chat/VideoRecorderLifecycleTest.kt | Tests recorder lifecycle transitions. |
| app/src/test/java/com/nextcloud/talk/chat/VideoPreviewPlacementTest.kt | Tests preview sizing. |
| app/src/test/java/com/nextcloud/talk/chat/VideoPreviewPlacementCompactPanelTest.kt | Tests compact-panel preview sizing. |
| app/src/test/java/com/nextcloud/talk/chat/VideoMessageRecordingTest.kt | Tests modes and recording outcomes. |
| app/src/test/java/com/nextcloud/talk/chat/VideoFrameAspectTest.kt | Tests frame orientation calculations. |
| app/src/test/java/com/nextcloud/talk/chat/ShouldVibrateOnRecordingChangeTest.kt | Tests recording vibration rules. |
| app/src/test/java/com/nextcloud/talk/chat/RecordingStopCoordinatorTest.kt | Tests coordinated recording stops. |
| app/src/test/java/com/nextcloud/talk/chat/RecordHintPopupPlacementTest.kt | Tests hint positioning. |
| app/src/test/java/com/nextcloud/talk/chat/RecordButtonGestureTest.kt | Tests tap and hold gestures. |
| app/src/test/java/com/nextcloud/talk/attachmentsheet/ShareFilePermissionTest.kt | Tests file-sharing permission decisions. |
| app/src/test/java/com/nextcloud/talk/attachmentsheet/RecentMediaTest.kt | Tests media ordering and durations. |
| app/src/test/java/com/nextcloud/talk/attachmentsheet/MediaSelectionTest.kt | Tests ordered media selection. |
| app/src/test/java/com/nextcloud/talk/attachmentsheet/MediaAccessTest.kt | Tests media access rules. |
| app/src/test/java/com/nextcloud/talk/attachmentsheet/CanSelectMoreMediaTest.kt | Tests reselection availability. |
| app/src/test/java/com/nextcloud/talk/attachmentsheet/AttachmentActionTest.kt | Tests attachment action visibility. |
| app/src/main/res/values/strings.xml | Adds media and recording labels. |
| app/src/main/res/values/dimens.xml | Adds video preview dimensions. |
| app/src/main/res/values/colors.xml | Adds recording overlay color. |
| app/src/main/res/layout/view_record_hint.xml | Adds recording hint layout. |
| app/src/main/res/layout/view_message_input.xml | Tints the recording button. |
| app/src/main/res/layout/fragment_message_input_voice_recording.xml | Adds video recording indicators. |
| app/src/main/res/layout/dialog_attachment.xml | Removes the old attachment layout. |
| app/src/main/res/layout/activity_chat.xml | Adds recording preview and overlay. |
| app/src/main/res/drawable/video_recording_dot.xml | Adds recording indicator artwork. |
| app/src/main/res/drawable/ic_record_hint_arrow.xml | Adds hint arrow artwork. |
| app/src/main/res/drawable/bg_record_hint.xml | Adds hint bubble background. |
| app/src/main/java/com/nextcloud/talk/utils/preferences/AppPreferencesImpl.kt | Persists recording mode. |
| app/src/main/java/com/nextcloud/talk/utils/preferences/AppPreferences.java | Exposes recording mode preferences. |
| app/src/main/java/com/nextcloud/talk/utils/permissions/PlatformPermissionUtilImpl.kt | Recognizes partial media access. |
| app/src/main/java/com/nextcloud/talk/ui/dialog/AttachmentDialog.kt | Removes the old attachment dialog. |
| app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt | Retains recorder and recording state. |
| app/src/main/java/com/nextcloud/talk/chat/VideoPreviewPlacement.kt | Calculates preview size and aspect. |
| app/src/main/java/com/nextcloud/talk/chat/VideoMessageRecorder.kt | Implements CameraX video recording. |
| app/src/main/java/com/nextcloud/talk/chat/RecordInputMode.kt | Defines voice and video modes. |
| app/src/main/java/com/nextcloud/talk/chat/RecordingStopCoordinator.kt | Coordinates stops during camera switches. |
| app/src/main/java/com/nextcloud/talk/chat/RecordHintPopup.kt | Displays recording hints. |
| app/src/main/java/com/nextcloud/talk/chat/RecordButtonGesture.kt | Classifies recording gestures. |
| app/src/main/java/com/nextcloud/talk/chat/MessageInputVoiceRecordingFragment.kt | Supports locked video recording controls. |
| app/src/main/java/com/nextcloud/talk/chat/MessageInputFragment.kt | Connects recording gestures and attachment sheet. |
| app/src/main/java/com/nextcloud/talk/chat/ChatActivity.kt | Integrates media sheet and video recording. |
| app/src/main/java/com/nextcloud/talk/chat/CameraLens.kt | Shares camera binding helpers. |
| app/src/main/java/com/nextcloud/talk/attachmentsheet/RecentMediaLoader.kt | Loads media and video thumbnails. |
| app/src/main/java/com/nextcloud/talk/attachmentsheet/RecentMedia.kt | Models and formats recent media. |
| app/src/main/java/com/nextcloud/talk/attachmentsheet/MediaSelection.kt | Tracks bounded media selection. |
| app/src/main/java/com/nextcloud/talk/attachmentsheet/MediaAccess.kt | Defines media permission rules. |
| app/src/main/java/com/nextcloud/talk/attachmentsheet/CameraTile.kt | Displays the live camera tile. |
| app/src/main/java/com/nextcloud/talk/attachmentsheet/AttachmentSheet.kt | Implements the Compose attachment sheet. |
| app/src/main/java/com/nextcloud/talk/attachmentsheet/AttachmentActionResources.kt | Maps actions to labels and icons. |
| app/src/main/java/com/nextcloud/talk/attachmentsheet/AttachmentAction.kt | Defines actions and visibility rules. |
| app/src/main/AndroidManifest.xml | Declares selected-media permission. |
| app/build.gradle.kts | Adds CameraX dependencies. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
🟡 Minor · Handle each MediaStore type independently. · MediaAccess.kt:18-39
app/src/main/java/com/nextcloud/talk/attachmentsheet/MediaAccess.kt:18-39
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winHandle each MediaStore type independently.
resolveMediaAccessreturnsFULLwhen only one Android 13 media permission is granted. The sheet therefore callsRecentMediaLoader.load, which always queries images and videos. Android may throwSecurityExceptionfor the ungranted type.loaddoes not catch it, so the recent-media list is not returned and the grid cannot load.Suggested fix
suspend fun load(resolver: ContentResolver, limit: Int = LIMIT): List<RecentMedia> { - val images = query(resolver, MediaStore.Images.Media.EXTERNAL_CONTENT_URI, isVideo = false, limit) - val videos = query(resolver, MediaStore.Video.Media.EXTERNAL_CONTENT_URI, isVideo = true, limit) + val images = querySafely(resolver, MediaStore.Images.Media.EXTERNAL_CONTENT_URI, isVideo = false, limit) + val videos = querySafely(resolver, MediaStore.Video.Media.EXTERNAL_CONTENT_URI, isVideo = true, limit) return mergeRecentMedia(images, videos, limit) } + private suspend fun querySafely( + resolver: ContentResolver, + uri: Uri, + isVideo: Boolean, + limit: Int + ): List<RecentMedia> = + try { + query(resolver, uri, isVideo, limit) + } catch (e: SecurityException) { + Log.w(TAG, "media type not readable: $uri", e) + emptyList() + } +
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Organization UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
d47df9d5-0621-4e6d-8bdb-efcb27578b57
📒 Files selected for processing: 11
app/src/main/java/com/nextcloud/talk/attachmentsheet/MediaAccess.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/RecordInputMode.ktapp/src/main/java/com/nextcloud/talk/chat/VideoMessageRecorder.ktapp/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.ktapp/src/main/java/com/nextcloud/talk/conversationlist/ConversationsListActivity.ktapp/src/main/java/com/nextcloud/talk/jobs/UploadAndShareFilesWorker.ktapp/src/test/java/com/nextcloud/talk/attachmentsheet/ShareFilePermissionTest.ktapp/src/test/java/com/nextcloud/talk/chat/RecordStartTest.ktapp/src/test/java/com/nextcloud/talk/chat/VideoRecordingCopilotFixesTest.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.
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
…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
…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
…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
- A recording result that arrives before the chat of a recreated activity is loaded is held by the recorder and delivered once it is loaded, as the results that were already pending. - No recording of either kind starts while a video is still being finalized. Its end would clear the state and the audio focus of the new recording. - A video that was sent is finalized and uploaded even if the chat is left before CameraX has finished; clearing the view model gives the audio focus of a video recording back. - The record button switches the mode on a click as well (TalkBack, Switch Access, keyboard); a touch does not switch it twice. - Recording a video no longer asks for access to the media files, it writes to the app cache. Assisted-by: Claude-Code:claude-sonnet-5-5 Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru> (cherry picked from commit fdef851)
…ding After a recreation of the activity the lock observer shows the locked recording panel at once, but the capabilities arrive later and their handler replaced the panel with the ordinary message input. The lock stayed set, so nothing brought the panel back. The handler now keeps the panel of a locked recording. Assisted-by: Claude-Code:claude-sonnet-5-5 Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru> (cherry picked from commit 62d3140)
…est, close the sheet on stop - The storage permission request of a file that is already at hand (camera photo, share to Talk, voice recording) asked for the media permissions without the user-selected grant, and its callbacks looked only at the first result. It now asks like the file picker does and accepts any granted result or an effective access. - The attachment sheet is closed when the chat stops, so a call answered meanwhile cannot leave a live camera preview open next to the call camera. Assisted-by: Claude-Code:claude-sonnet-5-5 Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru>
The photo of the system camera is written to the app cache (createAttachmentFile) and handed over through the own file provider, so it needs no access to the media. Drop the gate in onPickCameraResult and the permission callback branch that only resumed that preview; the callback is still used by the voice recording. Assisted-by: Claude-Code:claude-sonnet-5-5 Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru> (cherry picked from commit 8c4e966)
View.onTouchEvent posts the click of a released touch, so it ran after the touch router had reset its flag and switched the recording mode once more, e.g. right after a recording was sent. The release is now consumed by the touch listener and never reaches the view; the pressed state is cleared by hand. The click listener stays for TalkBack, Switch Access and the keyboard. Replaces RecordButtonClickRouter. Assisted-by: Claude-Code:claude-sonnet-5-5 Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru> (cherry picked from commit 70c586b)
bef9b53 to
9bb514b
Compare
…essage records sound The recording asked for the audio focus and opened the microphone in the same call. The player of another app pauses some time after it lost the focus, so the first moments of a video message contained the end of the music from the speaker. The camera and the preview are bound at once, but the recording starts only when AudioManager.isMusicActive is false, for at most one second; without music there is no delay. Stopping in the meantime ends the start as before. MusicSilenceGate is the same file as in the voice message fix. Assisted-by: Claude-Code:claude-sonnet-5-5 Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru> (cherry picked from commit 86493e1)
|
Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! (If you believe you should not receive this message, you can add yourself to the blocklist.) |
…e type RecentMediaLoader.load asks MediaStore for photos and videos and has no catch at its call site. On AOSP a query without the matching READ_MEDIA permission returns only the files of the app, but the provider of a device may throw SecurityException instead, and the refusal of one type then left the whole grid empty. The query of a type now returns an empty list on SecurityException and logs it without file names; the other type is still loaded. Both queries stay, because the own photos and videos of the app are useful. The KDoc of canSelectMoreMedia said the second type is limited to the picked items. With one per-type permission granted, the provider shows only the files of the app for the other type and the picked items do not apply; the action opens the request again, where "allow all" is offered. Tests: a fake provider for authority media refuses one type or both; the permission matrix of resolveMediaAccess and canSelectMoreMedia is completed for Android 12L, 13 and 14. Assisted-by: Claude-Code:claude-opus-5-5 Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru>
8caea1b to
1b9830a
Compare
|
@coderabbitai Re the outside-diff note on Thanks for the note. We checked the MediaProvider sources (android13-release / android14-release): a query of Since |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
fd '^(RecentMediaLoader|RecentMediaLoaderTest|MediaAccess)\.kt$' --exec bash -c '
for file do
printf "\n--- Outline: %s ---\n" "$file"
ast-grep outline "$file"
printf "\n--- Content: %s ---\n" "$file"
cat -n "$file"
done
' bashLength of output: 18593
I verified the defensive handling in The 🔥 Failed to add learningsLearnings cannot be added due to server error. Please try again. You are interacting with an AI system. |
Refs #6814
This is a feature PR; the issue is not approved yet, the PR is opened for discussion.
Depends on #6818
(the stack also contains the two voice recording fixes it depends on; the lens helpers in
CameraLens.ktcome from it)🖼️ Screenshots
Feature
The attachment dialog is replaced by a Compose
ModalBottomSheet:AttachmentDialoganddialog_attachment.xmlare removed.Media access:
READ_MEDIA_IMAGES,READ_MEDIA_VIDEOandREAD_MEDIA_VISUAL_USER_SELECTED. With partial access the grid shows the selected items and a "Select more photos and videos" entry.READ_MEDIA_VISUAL_USER_SELECTEDturns off the Android 14 compatibility mode for the whole app, soPlatformPermissionUtilImpl.isFilesPermissionGranted()accepts partial access (second commit). Without it voice and video messages, camera photos and "file from device" would ask for permissions again.Google Play policy
The grid reads MediaStore and needs
READ_MEDIA_IMAGES/READ_MEDIA_VIDEO; the system Photo Picker (#4829) was chosen before partly because of the Play policy (#4313). Whether the grid is acceptable at all, and the option of agplayvariant with only the Photo Picker (and noREAD_MEDIA_*request for the grid), are discussed in #6814.Current behavior of this branch:
genericandgplayare the same. The permissions are declared in the main manifest, so both flavors request them when the user taps "Allow access"; the merged manifests of both flavors containREAD_MEDIA_IMAGES,READ_MEDIA_VIDEOandREAD_MEDIA_VISUAL_USER_SELECTED. There is nogplay-only reduced variant yet; I will add one if the maintainers prefer it.Structure
Two commits on top of the video message PR:
feat(chat): replace attachment menu with a media bottom sheetfix(chat): count partial media access as files access on Android 14How 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.