From 26ca3eb1d846f4fb72fe388413b3a06b11c09a02 Mon Sep 17 00:00:00 2001 From: Jens Zalzala Date: Thu, 30 Apr 2026 12:24:02 -0500 Subject: [PATCH 01/26] Show file upload progress and placeholder media message in chat. Signed-off-by: Jens Zalzala # Conflicts: # gradle/verification-keyring.keys --- .../java/com/nextcloud/talk/api/NcApi.java | 3 +- .../com/nextcloud/talk/chat/ChatActivity.kt | 9 +- .../talk/chat/data/ChatMessageRepository.kt | 11 + .../network/OfflineFirstChatRepository.kt | 74 ++++++ .../talk/chat/ui/model/ChatMessageUi.kt | 26 +- .../talk/chat/viewmodels/ChatViewModel.kt | 83 +++++- .../talk/jobs/ShareOperationWorker.kt | 3 +- .../talk/jobs/UploadAndShareFilesWorker.kt | 70 +++++- .../nextcloud/talk/ui/chat/ChatMessageView.kt | 18 +- .../com/nextcloud/talk/ui/chat/ChatView.kt | 3 +- .../nextcloud/talk/ui/chat/MediaMessage.kt | 238 ++++++++++++++++-- .../upload/chunked/ChunkedFileUploader.kt | 2 + .../talk/upload/normal/FileUploader.kt | 7 - 13 files changed, 506 insertions(+), 41 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/api/NcApi.java b/app/src/main/java/com/nextcloud/talk/api/NcApi.java index 82b44342bec..3efbce1444f 100644 --- a/app/src/main/java/com/nextcloud/talk/api/NcApi.java +++ b/app/src/main/java/com/nextcloud/talk/api/NcApi.java @@ -410,7 +410,8 @@ Observable createRemoteShare(@Nullable @Header("Authorization") @Field("path") String remotePath, @Field("shareWith") String roomToken, @Field("shareType") String shareType, - @Field("talkMetaData") String talkMetaData); + @Field("talkMetaData") String talkMetaData, + @Field("referenceId") String referenceId); @FormUrlEncoded @PUT diff --git a/app/src/main/java/com/nextcloud/talk/chat/ChatActivity.kt b/app/src/main/java/com/nextcloud/talk/chat/ChatActivity.kt index fc66fa78713..2c02977d29f 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/ChatActivity.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/ChatActivity.kt @@ -169,6 +169,7 @@ import com.nextcloud.talk.ui.chat.ChatMessageCallbacks import com.nextcloud.talk.ui.chat.ChatView import com.nextcloud.talk.ui.chat.ChatViewCallbacks import com.nextcloud.talk.ui.chat.ChatViewState +import com.nextcloud.talk.ui.chat.LocalUploadProgressProvider import com.nextcloud.talk.ui.dialog.DateTimeCompose import com.nextcloud.talk.ui.dialog.GetPinnedOptionsDialog import com.nextcloud.talk.ui.dialog.SaveToStorageDialogFragment @@ -876,10 +877,13 @@ class ChatActivity : SideEffect { chatListState = listState } + val uploadProgressMap by chatViewModel.uploadProgressMap.collectAsStateWithLifecycle() + CompositionLocalProvider( LocalViewThemeUtils provides viewThemeUtils, LocalMessageUtils provides messageUtils, - LocalOpenGraphFetcher provides { url -> chatViewModel.fetchOpenGraph(url) } + LocalOpenGraphFetcher provides { url -> chatViewModel.fetchOpenGraph(url) }, + LocalUploadProgressProvider provides { refId -> uploadProgressMap[refId] } ) { val isOneToOneConversation by remember { mutableStateOf(uiState.isOneToOneConversation) } Log.d(TAG, "isOneToOneConversation=" + isOneToOneConversation) @@ -938,7 +942,8 @@ class ChatActivity : onSystemMessageExpandClick = { messageId -> chatViewModel.toggleSystemMessageCollapse(messageId) }, - onAvatarClick = { messageId -> chatViewModel.showProfileSheet(messageId.toLong()) } + onAvatarClick = { messageId -> chatViewModel.showProfileSheet(messageId.toLong()) }, + onCancelUpload = { referenceId -> chatViewModel.cancelUpload(referenceId) } ) ), listState = listState diff --git a/app/src/main/java/com/nextcloud/talk/chat/data/ChatMessageRepository.kt b/app/src/main/java/com/nextcloud/talk/chat/data/ChatMessageRepository.kt index aa111de3f0a..ce1d99a40fc 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/data/ChatMessageRepository.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/data/ChatMessageRepository.kt @@ -150,6 +150,17 @@ interface ChatMessageRepository : LifecycleAwareManager { referenceId: String ): Flow> + @Suppress("LongParameterList") + suspend fun addUploadPlaceholderMessage( + localFileUri: String, + caption: String, + mimeType: String?, + fileSize: Long, + referenceId: String + ): Flow> + + suspend fun deleteTempMessageByReferenceId(referenceId: String) + suspend fun editChatMessage(credentials: String, url: String, text: String): Flow> suspend fun editTempChatMessage(message: ChatMessage, editedMessageText: String): Flow diff --git a/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt b/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt index 79cbe4171fa..75234876703 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt @@ -684,6 +684,80 @@ class OfflineFirstChatRepository @Inject constructor( } } + @Suppress("Detekt.TooGenericExceptionCaught", "LongMethod") + override suspend fun addUploadPlaceholderMessage( + localFileUri: String, + caption: String, + mimeType: String?, + fileSize: Long, + referenceId: String + ): Flow> = + flow { + try { + val currentTimeMillis = System.currentTimeMillis() + + // Use the first 15 hex chars so the value always fits in a signed Long. + // Use referenceId.hashCode() as the placeholder id so that: + // 1. It is unique per file even when multiple files are selected simultaneously + // 2. It fits in an Int, so it survives the Long→Int cast in ChatMessageUi.id without + // truncation, keeping DB lookups consistent when the message is tapped. + // 3. It is always positive, because getMessagesEqualOrNewerThan expects it to be larger + // than oldestMessageId + @Suppress("MagicNumber") + val placeholderId = (referenceId.hashCode().toLong() and 0x7FFF_FFFFL) + + Log.d( + TAG, + "addUploadPlaceholderMessage: referenceId=$referenceId " + + "placeholderId=$placeholderId caption=$caption" + ) + + val fileParams = hashMapOf( + "type" to "file", + "name" to caption, + "mimetype" to (mimeType ?: ""), + "size" to fileSize.toString(), + "path" to localFileUri + ) + val messageParameters = hashMapOf>( + "file" to fileParams + ) + + val entity = ChatMessageEntity( + internalId = "$internalConversationId@_temp_$referenceId", + internalConversationId = internalConversationId, + id = placeholderId, + threadId = threadId, + message = "{file}", + deleted = false, + token = conversationModel.token, + actorId = currentUser.userId!!, + actorType = EnumActorTypeConverter().convertToString(Participant.ActorType.USERS), + accountId = currentUser.id!!, + messageParameters = messageParameters, + messageType = "comment", + parentMessageId = null, + systemMessageType = ChatMessage.SystemMessageType.DUMMY, + replyable = false, + timestamp = currentTimeMillis / MILLIES, + expirationTimestamp = 0, + actorDisplayName = currentUser.displayName!!, + referenceId = referenceId, + isTemporary = true, + sendStatus = SendStatus.PENDING, + silent = false + ) + chatDao.upsertChatMessage(entity) + } catch (e: Exception) { + Log.e(TAG, "addUploadPlaceholderMessage failed for referenceId=$referenceId", e) + emit(Result.failure(e)) + } + } + + override suspend fun deleteTempMessageByReferenceId(referenceId: String) { + chatDao.deleteTempChatMessages(internalConversationId, listOf(referenceId)) + } + @Suppress("Detekt.TooGenericExceptionCaught") override suspend fun editChatMessage( credentials: String, diff --git a/app/src/main/java/com/nextcloud/talk/chat/ui/model/ChatMessageUi.kt b/app/src/main/java/com/nextcloud/talk/chat/ui/model/ChatMessageUi.kt index 6b9826bb3db..bc045155a01 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/ui/model/ChatMessageUi.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/ui/model/ChatMessageUi.kt @@ -56,7 +56,8 @@ data class ChatMessageUi( val isExpandableParent: Boolean = false, val expandableChildrenAmount: Int = 0, val isHiddenByCollapse: Boolean = false, - val isExpanded: Boolean = false + val isExpanded: Boolean = false, + val referenceId: String? = null ) data class MessageReactionUi(val emoji: String, val amount: Int, val isSelfReaction: Boolean) @@ -78,6 +79,13 @@ sealed interface MessageTypeContent { val isClassified: Boolean = false ) : MessageTypeContent + data class UploadingMedia( + val localFileUri: String, + val caption: String, + val mimeType: String?, + val drawableResourceId: Int + ) : MessageTypeContent + data class Geolocation(val id: String, val name: String, val lat: Double, val lon: Double) : MessageTypeContent data class Poll(val pollId: String, val pollName: String) : MessageTypeContent @@ -155,7 +163,8 @@ fun ChatMessage.toUiModel( isSilent = silent, isExpandableParent = expandableParent, expandableChildrenAmount = expandableChildrenAmount, - isHiddenByCollapse = hiddenByCollapse + isHiddenByCollapse = hiddenByCollapse, + referenceId = referenceId ) fun ChatMessage.toScheduledMessageUiModel( @@ -251,6 +260,8 @@ fun getMessageTypeContent(user: User, message: ChatMessage, isClassified: Boolea MessageTypeContent.SystemMessage } else if (message.isVoiceMessage) { getVoiceContent(message) + } else if (message.hasFileAttachment && message.isTemporary) { + getUploadingMediaContent(message) } else if (message.hasFileAttachment) { getMediaContent(user, message, isClassified) } else if (message.hasGeoLocation) { @@ -265,6 +276,17 @@ fun getMessageTypeContent(user: User, message: ChatMessage, isClassified: Boolea ?: MessageTypeContent.RegularText } +fun getUploadingMediaContent(message: ChatMessage): MessageTypeContent.UploadingMedia { + val mimetype = message.fileParameters.mimetype + val drawableResourceId = DrawableUtils.getDrawableResourceIdForMimeType(mimetype) + return MessageTypeContent.UploadingMedia( + localFileUri = message.fileParameters.path.orEmpty(), + caption = message.fileParameters.name.orEmpty(), + mimeType = mimetype.takeIf { !it.isNullOrEmpty() }, + drawableResourceId = drawableResourceId + ) +} + fun getMediaContent(user: User, message: ChatMessage, isClassified: Boolean = false): MessageTypeContent.Media { val mimetype = message.fileParameters.mimetype val drawableResourceId = DrawableUtils.getDrawableResourceIdForMimeType(mimetype) diff --git a/app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt b/app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt index 20b99605f77..e24ee9f1b71 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt @@ -10,6 +10,7 @@ package com.nextcloud.talk.chat.viewmodels import android.content.Context import android.net.Uri import android.os.Bundle +import android.provider.OpenableColumns import android.util.Log import androidx.lifecycle.DefaultLifecycleObserver import androidx.lifecycle.LifecycleOwner @@ -42,6 +43,8 @@ import com.nextcloud.talk.data.user.model.User import com.nextcloud.talk.extensions.toIntOrZero import com.nextcloud.talk.jobs.ReadMarkerSyncWorker import com.nextcloud.talk.jobs.ShareOperationWorker +import androidx.lifecycle.asFlow +import androidx.work.WorkManager import com.nextcloud.talk.jobs.UploadAndShareFilesWorker import com.nextcloud.talk.logger.Logger import com.nextcloud.talk.messagesearch.MessageSearchHelper @@ -123,7 +126,9 @@ import java.io.IOException import java.time.Instant import java.time.LocalDate import java.time.ZoneId +import java.util.UUID import javax.inject.Inject +import androidx.core.net.toUri @Suppress("TooManyFunctions", "LongParameterList") class ChatViewModel @AssistedInject constructor( @@ -216,6 +221,21 @@ class ChatViewModel @AssistedInject constructor( private var lobbyPollingJob: Job? = null + private val _uploadProgressMap = MutableStateFlow>(emptyMap()) + val uploadProgressMap: StateFlow> = _uploadProgressMap + + // Maps referenceId -> fileUri for cancellation support + private val uploadReferenceToUri = mutableMapOf() + + fun cancelUpload(referenceId: String) { + val fileUri = uploadReferenceToUri.remove(referenceId) ?: return + WorkManager.getInstance(NextcloudTalkApplication.sharedApplication!!).cancelUniqueWork(fileUri) + viewModelScope.launch { + chatRepository.deleteTempMessageByReferenceId(referenceId) + } + _uploadProgressMap.update { it - referenceId } + } + fun getChatRepository(): ChatMessageRepository = chatRepository override fun onResume(owner: LifecycleOwner) { @@ -2023,24 +2043,83 @@ class ChatViewModel @AssistedInject constructor( metaDataMap["caption"] = caption } + val referenceId = UUID.randomUUID().toString().replace("-", "") + metaDataMap["referenceId"] = referenceId + val metaData = Gson().toJson(metaDataMap) room = if (roomToken == "") chatRoomToken else roomToken try { require(fileUri.isNotEmpty()) - UploadAndShareFilesWorker.upload( + + if (!isVoiceMessage) { + val (fileName, mimeType, fileSize) = resolveFileInfo(fileUri) + viewModelScope.launch { + chatRepository.addUploadPlaceholderMessage( + localFileUri = fileUri, + caption = caption.ifEmpty { fileName }, + mimeType = mimeType, + fileSize = fileSize, + referenceId = referenceId + ).collect {} + } + } + + val internalConversationId = "${currentUser.id}@$chatRoomToken" + val workerId = UploadAndShareFilesWorker.upload( fileUri, room, displayName, metaData, - compressImages + compressImages, + referenceId, + internalConversationId ) + + if (!isVoiceMessage) { + uploadReferenceToUri[referenceId] = fileUri + observeUploadProgress(workerId, referenceId) + } } catch (e: IllegalArgumentException) { Log.e(javaClass.simpleName, "Something went wrong when trying to upload file", e) } } + private fun resolveFileInfo(fileUri: String): Triple { + val uri = fileUri.toUri() + val mimeType = NextcloudTalkApplication.sharedApplication!!.contentResolver.getType(uri) + val cursor = NextcloudTalkApplication.sharedApplication!!.contentResolver.query(uri, null, null, null, null) + cursor?.use { + val nameIndex = it.getColumnIndex(OpenableColumns.DISPLAY_NAME) + val sizeIndex = it.getColumnIndex(OpenableColumns.SIZE) + if (it.moveToFirst()) { + val name = if (nameIndex >= 0) it.getString(nameIndex).orEmpty() else uri.lastPathSegment.orEmpty() + val size = if (sizeIndex >= 0) it.getLong(sizeIndex) else 0L + return Triple(name, mimeType, size) + } + } + return Triple(uri.lastPathSegment.orEmpty(), mimeType, 0L) + } + + private fun observeUploadProgress(workerId: UUID, referenceId: String) { + WorkManager.getInstance(NextcloudTalkApplication.sharedApplication!!) + .getWorkInfoByIdLiveData(workerId) + .asFlow() + .onEach { workInfo -> + if (workInfo == null) return@onEach + val progress = workInfo.progress.getInt(UploadAndShareFilesWorker.PROGRESS_KEY, -1) + if (progress >= 0) { + _uploadProgressMap.update { it + (referenceId to progress) } + } + if (workInfo.state.isFinished) { + _uploadProgressMap.update { it - referenceId } + uploadReferenceToUri.remove(referenceId) + } + } + .launchIn(viewModelScope) + } + fun postToRecordTouchObserver(float: Float) { _recordTouchObserver.postValue(float) } diff --git a/app/src/main/java/com/nextcloud/talk/jobs/ShareOperationWorker.kt b/app/src/main/java/com/nextcloud/talk/jobs/ShareOperationWorker.kt index 4f3a026ed13..0c18028f672 100644 --- a/app/src/main/java/com/nextcloud/talk/jobs/ShareOperationWorker.kt +++ b/app/src/main/java/com/nextcloud/talk/jobs/ShareOperationWorker.kt @@ -67,7 +67,8 @@ class ShareOperationWorker(context: Context, workerParams: WorkerParameters) : W filePath, roomToken, "10", - metaData + metaData, + "" // no reference id ) .subscribeOn(Schedulers.io()) .blockingSubscribe( diff --git a/app/src/main/java/com/nextcloud/talk/jobs/UploadAndShareFilesWorker.kt b/app/src/main/java/com/nextcloud/talk/jobs/UploadAndShareFilesWorker.kt index 9dffb5cca7d..1a4feb20bcb 100644 --- a/app/src/main/java/com/nextcloud/talk/jobs/UploadAndShareFilesWorker.kt +++ b/app/src/main/java/com/nextcloud/talk/jobs/UploadAndShareFilesWorker.kt @@ -8,6 +8,7 @@ package com.nextcloud.talk.jobs import android.Manifest +import android.annotation.SuppressLint import android.app.Activity import android.app.NotificationManager import android.app.PendingIntent @@ -32,6 +33,8 @@ import com.nextcloud.talk.activities.MainActivity import com.nextcloud.talk.api.NcApi import com.nextcloud.talk.api.NcApiCoroutines import com.nextcloud.talk.application.NextcloudTalkApplication +import com.nextcloud.talk.data.database.dao.ChatMessagesDao +import com.nextcloud.talk.data.database.model.SendStatus import com.nextcloud.talk.data.user.model.User import com.nextcloud.talk.models.json.chatpostattachment.PostConversationAttachmentRequest import com.nextcloud.talk.models.json.chatprobeattachmentfolder.ChatProbeAttachmentData @@ -56,6 +59,8 @@ import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.MutableSharedFlow import kotlinx.coroutines.flow.SharedFlow import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.flow.firstOrNull +import kotlinx.coroutines.runBlocking import okhttp3.MediaType.Companion.toMediaTypeOrNull import okhttp3.OkHttpClient import java.io.File @@ -88,6 +93,9 @@ class UploadAndShareFilesWorker(val context: Context, workerParameters: WorkerPa @Inject lateinit var platformPermissionUtil: PlatformPermissionUtil + @Inject + lateinit var chatDao: ChatMessagesDao + lateinit var fileName: String private var mNotifyManager: NotificationManager? = null @@ -100,6 +108,8 @@ class UploadAndShareFilesWorker(val context: Context, workerParameters: WorkerPa private var isChunkedUploading = false private var file: File? = null private var chunkedFileUploader: ChunkedFileUploader? = null + private var referenceId: String? = null + private var internalConversationId: String? = null @Suppress("Detekt.TooGenericExceptionCaught") override fun doWork(): Result { @@ -111,6 +121,8 @@ class UploadAndShareFilesWorker(val context: Context, workerParameters: WorkerPa roomToken = inputData.getString(ROOM_TOKEN)!! conversationName = inputData.getString(CONVERSATION_NAME)!! val metaData = inputData.getString(META_DATA) + referenceId = inputData.getString(KEY_REFERENCE_ID) + internalConversationId = inputData.getString(KEY_INTERNAL_CONVERSATION_ID) checkNotNull(currentUser) checkNotNull(sourceFile) @@ -133,12 +145,20 @@ class UploadAndShareFilesWorker(val context: Context, workerParameters: WorkerPa currentUser.capabilities!!.spreedCapability!! ) file?.let { isChunkedUploading = it.length() > CHUNK_UPLOAD_THRESHOLD_SIZE } - val uploadSuccess: Boolean = uploadFile(sourceFileUri, metaData, remotePath, useConversationSubfolders) + val uploadSuccess: Boolean = uploadFile(sourceFileUri, remotePath, useConversationSubfolders) if (uploadSuccess) { + val shareSuccess = shareFile(remotePath, metaData) cancelNotification() - _uploadCompletedFlow.tryEmit(roomToken) - return Result.success() + if (shareSuccess) { + updatePlaceholderStatus(SendStatus.SENT_PENDING_ACK) + // _uploadCompletedFlow.tryEmit(roomToken) <- Check if this still makes sense! + return Result.success() + } + Log.e(TAG, "Share operation failed after upload") + showFailedToUploadNotification() + updatePlaceholderStatus(SendStatus.FAILED) + return Result.failure() } else if (isStopped) { // since work is cancelled the result would be ignored anyways return Result.failure() @@ -146,10 +166,12 @@ class UploadAndShareFilesWorker(val context: Context, workerParameters: WorkerPa Log.e(TAG, "Something went wrong when trying to upload file") showFailedToUploadNotification() + updatePlaceholderStatus(SendStatus.FAILED) return Result.failure() } catch (e: Exception) { Log.e(TAG, "Something went wrong when trying to upload file", e) showFailedToUploadNotification() + updatePlaceholderStatus(SendStatus.FAILED) return Result.failure() } } @@ -201,7 +223,7 @@ class UploadAndShareFilesWorker(val context: Context, workerParameters: WorkerPa okHttpClient, currentUser, roomToken, - metaData, + null, this, ncApiCoroutines, useConversationSubfolders @@ -218,7 +240,7 @@ class UploadAndShareFilesWorker(val context: Context, workerParameters: WorkerPa file!!, ncApiCoroutines ) - .upload(sourceFileUri, fileName, remotePath, metaData) + .upload(sourceFileUri, fileName, remotePath, null) .blockingFirst() } @@ -287,6 +309,24 @@ class UploadAndShareFilesWorker(val context: Context, workerParameters: WorkerPa .isSuccess } + @SuppressLint("CheckResult") + private fun shareFile(remotePath: String, metaData: String?): Boolean = + try { + ncApi.createRemoteShare( + ApiUtils.getCredentials(currentUser.username, currentUser.token), + ApiUtils.getSharingUrl(currentUser.baseUrl!!), + remotePath, + roomToken, + "10", + metaData, + referenceId.orEmpty() + ).blockingFirst() + true + } catch (e: NoSuchElementException) { + Log.e(TAG, "Failed to share file to room", e) + false + } + private fun resolveFinalFileName(originalName: String, probeData: ChatProbeAttachmentData): String = probeData.renames?.get(originalName) ?: originalName @@ -298,6 +338,8 @@ class UploadAndShareFilesWorker(val context: Context, workerParameters: WorkerPa } override fun onTransferProgress(percentage: Int) { + setProgressAsync(Data.Builder().putInt(PROGRESS_KEY, percentage).build()) + val progressUpdateNotification = mBuilder!! .setProgress(HUNDRED_PERCENT, percentage, false) .setContentText(getNotificationContentText(percentage)) @@ -306,6 +348,13 @@ class UploadAndShareFilesWorker(val context: Context, workerParameters: WorkerPa mNotifyManager!!.notify(notificationId, progressUpdateNotification) } + private fun updatePlaceholderStatus(status: SendStatus) { + val refId = referenceId ?: return + val convId = internalConversationId ?: return + val entity = runBlocking { chatDao.getTempMessageForConversation(convId, refId, null).firstOrNull() } + entity?.let { chatDao.updateChatMessage(it.copy(sendStatus = status)) } + } + override fun onStopped() { if (file != null && isChunkedUploading) { chunkedFileUploader?.abortUpload { @@ -488,6 +537,9 @@ class UploadAndShareFilesWorker(val context: Context, workerParameters: WorkerPa private const val ROOM_TOKEN = "ROOM_TOKEN" private const val CONVERSATION_NAME = "CONVERSATION_NAME" private const val META_DATA = "META_DATA" + const val KEY_REFERENCE_ID = "REFERENCE_ID" + const val KEY_INTERNAL_CONVERSATION_ID = "INTERNAL_CONVERSATION_ID" + const val PROGRESS_KEY = "UPLOAD_PROGRESS" private const val COMPRESS_IMAGES = "COMPRESS_IMAGES" private const val CHUNK_UPLOAD_THRESHOLD_SIZE: Long = 1024 * 1024 private const val NOTIFICATION_FILE_NAME_MAX_LENGTH = 20 @@ -541,24 +593,30 @@ class UploadAndShareFilesWorker(val context: Context, workerParameters: WorkerPa } } + @Suppress("LongParameterList") fun upload( fileUri: String, roomToken: String, conversationName: String, metaData: String?, + referenceId: String = "", + internalConversationId: String = "", compressImages: Boolean = false - ) { + ): UUID { val data: Data = Data.Builder() .putString(DEVICE_SOURCE_FILE, fileUri) .putString(ROOM_TOKEN, roomToken) .putString(CONVERSATION_NAME, conversationName) .putString(META_DATA, metaData) + .putString(KEY_REFERENCE_ID, referenceId) + .putString(KEY_INTERNAL_CONVERSATION_ID, internalConversationId) .putBoolean(COMPRESS_IMAGES, compressImages) .build() val uploadWorker: OneTimeWorkRequest = OneTimeWorkRequest.Builder(UploadAndShareFilesWorker::class.java) .setInputData(data) .build() WorkManager.getInstance().enqueueUniqueWork(fileUri, ExistingWorkPolicy.KEEP, uploadWorker) + return uploadWorker.id } } } diff --git a/app/src/main/java/com/nextcloud/talk/ui/chat/ChatMessageView.kt b/app/src/main/java/com/nextcloud/talk/ui/chat/ChatMessageView.kt index 3672ca8a8b2..979e1c76851 100644 --- a/app/src/main/java/com/nextcloud/talk/ui/chat/ChatMessageView.kt +++ b/app/src/main/java/com/nextcloud/talk/ui/chat/ChatMessageView.kt @@ -72,7 +72,8 @@ data class ChatMessageCallbacks( val onOpenThreadClick: (Int) -> Unit = {}, val onQuotedMessageClick: (Int) -> Unit = {}, val onSystemMessageExpandClick: (Int) -> Unit = {}, - val onAvatarClick: (Int) -> Unit = {} + val onAvatarClick: (Int) -> Unit = {}, + val onCancelUpload: (String) -> Unit = {} ) @Suppress("Detekt.LongParameterList", "Detekt.LongMethod", "Detekt.CyclomaticComplexMethod") @@ -206,9 +207,18 @@ fun ChatMessageView( ) } - else -> { - Log.d("ChatView", "Unknown message type: ${'$'}content") - } + is MessageTypeContent.UploadingMedia -> { + UploadingMediaMessage( + typeContent = content, + message = message, + isOneToOneConversation = context.isOneToOneConversation, + conversationThreadId = context.conversationThreadId, + onCancelUpload = callbacks.onCancelUpload + ) + } + + else -> { + Log.d("ChatView", "Unknown message type: ${'$'}content")} } } val useContainerHighlight = highlightSearchTerm.isNullOrBlank() || isSelected diff --git a/app/src/main/java/com/nextcloud/talk/ui/chat/ChatView.kt b/app/src/main/java/com/nextcloud/talk/ui/chat/ChatView.kt index c4b8bc2bfdc..399a4075dc1 100644 --- a/app/src/main/java/com/nextcloud/talk/ui/chat/ChatView.kt +++ b/app/src/main/java/com/nextcloud/talk/ui/chat/ChatView.kt @@ -426,7 +426,8 @@ fun ChatView( onOpenThreadClick = callbacks.messageCallbacks.onOpenThreadClick, onQuotedMessageClick = handleQuotedMessageClick, onSystemMessageExpandClick = callbacks.messageCallbacks.onSystemMessageExpandClick, - onAvatarClick = callbacks.messageCallbacks.onAvatarClick + onAvatarClick = callbacks.messageCallbacks.onAvatarClick, + onCancelUpload = callbacks.onCancelUpload ) ) } diff --git a/app/src/main/java/com/nextcloud/talk/ui/chat/MediaMessage.kt b/app/src/main/java/com/nextcloud/talk/ui/chat/MediaMessage.kt index 45df391c857..7af06c0c187 100644 --- a/app/src/main/java/com/nextcloud/talk/ui/chat/MediaMessage.kt +++ b/app/src/main/java/com/nextcloud/talk/ui/chat/MediaMessage.kt @@ -17,11 +17,18 @@ import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.size import androidx.compose.foundation.shape.RoundedCornerShape import androidx.compose.material3.CircularProgressIndicator +import androidx.compose.material.icons.Icons +import androidx.compose.material.icons.filled.Close import androidx.compose.material3.Icon +import androidx.compose.material3.IconButton +import androidx.compose.material3.CircularProgressIndicator +import androidx.compose.material3.LinearProgressIndicator +import androidx.compose.material3.Text import androidx.compose.runtime.Composable import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableIntStateOf import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.compositionLocalOf import androidx.compose.runtime.remember import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.runtime.setValue @@ -35,6 +42,8 @@ import androidx.compose.ui.layout.ContentScale import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.res.painterResource import androidx.compose.ui.res.stringResource +import androidx.compose.runtime.CompositionLocalProvider +import androidx.compose.ui.draw.blur import androidx.compose.ui.unit.dp import coil.compose.AsyncImage import coil.network.HttpException @@ -42,12 +51,16 @@ import com.nextcloud.talk.R import com.nextcloud.talk.chat.data.model.FileParameters import com.nextcloud.talk.chat.data.model.decodeBlurhashPlaceholder import com.nextcloud.talk.chat.ui.model.ChatMessageUi +import com.nextcloud.talk.chat.ui.model.MessageStatusIcon import com.nextcloud.talk.chat.ui.model.MessageTypeContent import com.nextcloud.talk.contacts.load import com.nextcloud.talk.utils.Mimetype import com.nextcloud.talk.utils.MimetypeUtils import kotlinx.coroutines.delay import kotlinx.coroutines.launch +import androidx.core.net.toUri + +val LocalUploadProgressProvider = compositionLocalOf<(referenceId: String) -> Int?> { { null } } private const val FILE_PLACEHOLDER_MESSAGE = "{file}" private const val PREVIEW_MAX_RETRIES = 3 @@ -86,21 +99,7 @@ fun MediaMessage( val hasCaption = captionText != null val mediaInset = 4.dp val mediaShape = remember(message.incoming) { - if (message.incoming) { - RoundedCornerShape( - topStart = mediaRadiusSmall, - topEnd = mediaRadiusBig, - bottomEnd = mediaRadiusBig, - bottomStart = mediaRadiusBig - ) - } else { - RoundedCornerShape( - topStart = mediaRadiusBig, - topEnd = mediaRadiusSmall, - bottomEnd = mediaRadiusBig, - bottomStart = mediaRadiusBig - ) - } + shape(message.incoming) } MessageScaffold( @@ -226,3 +225,212 @@ fun MediaMessage( } ) } + +@Suppress("Detekt.LongMethod") +@Composable +fun UploadingMediaMessage( + typeContent: MessageTypeContent.UploadingMedia, + message: ChatMessageUi, + isOneToOneConversation: Boolean = false, + conversationThreadId: Long? = null, + onCancelUpload: (referenceId: String) -> Unit = {} +) { + val getProgress = LocalUploadProgressProvider.current + val progress = getProgress(message.referenceId.orEmpty()) + val isFailed = message.statusIcon == MessageStatusIcon.FAILED + val isSent = message.statusIcon == MessageStatusIcon.SENT + + val mediaInset = 4.dp + val mediaShape = remember(message.incoming) { + shape(message.incoming) + } + + MessageScaffold( + uiMessage = message, + isOneToOneConversation = isOneToOneConversation, + conversationThreadId = conversationThreadId, + includePadding = false, + captionText = typeContent.caption, + content = { + Column(modifier = Modifier.fillMaxWidth()) { + Box(modifier = Modifier.fillMaxWidth()) { + val isImage = typeContent.mimeType?.startsWith("image") == true + if (isImage && typeContent.localFileUri.isNotEmpty()) { + AsyncImage( + model = typeContent.localFileUri.toUri(), + contentDescription = typeContent.caption, + modifier = Modifier + .fillMaxWidth() + .blur(4.dp) + .padding(mediaInset) + .clip(mediaShape), + contentScale = ContentScale.FillWidth + ) + } else { + Icon( + painter = painterResource(typeContent.drawableResourceId), + contentDescription = typeContent.caption, + modifier = Modifier + .size(64.dp) + .padding(mediaInset) + .align(Alignment.Center), + tint = Color.Unspecified + ) + } + + if (isSent) { + CircularProgressIndicator( + modifier = Modifier + .align(Alignment.TopEnd) + .padding(8.dp) + .size(24.dp) + ) + } else if (!isFailed) { + IconButton( + onClick = { onCancelUpload(message.referenceId.orEmpty()) }, + modifier = Modifier.align(Alignment.TopEnd) + ) { + Icon( + imageVector = Icons.Default.Close, + contentDescription = stringResource(R.string.nc_cancel), + tint = Color.White + ) + } + } + } + + if (isFailed) { + Text( + text = stringResource(R.string.nc_upload_failed_notification_title), + modifier = Modifier.padding(horizontal = 8.dp, vertical = 2.dp), + color = androidx.compose.ui.graphics.Color.Red + ) + } else if (!isSent) { + if (progress != null) { + LinearProgressIndicator( + progress = { progress / 100f }, + modifier = Modifier + .fillMaxWidth() + .padding(horizontal = 8.dp, vertical = 4.dp) + ) + } else { + LinearProgressIndicator( + modifier = Modifier + .fillMaxWidth() + .padding(horizontal = 8.dp, vertical = 4.dp) + ) + } + } + } + } + ) +} + +fun shape(incoming: Boolean): RoundedCornerShape = + if (incoming) { + RoundedCornerShape( + topStart = mediaRadiusSmall, + topEnd = mediaRadiusBig, + bottomEnd = mediaRadiusBig, + bottomStart = mediaRadiusBig + ) + } else { + RoundedCornerShape( + topStart = mediaRadiusBig, + topEnd = mediaRadiusSmall, + bottomEnd = mediaRadiusBig, + bottomStart = mediaRadiusBig + ) + } + +private fun previewUploadingContent(mimeType: String? = "image/jpeg") = + MessageTypeContent.UploadingMedia( + localFileUri = "", + caption = "photo.jpg", + mimeType = mimeType, + drawableResourceId = R.drawable.ic_mimetype_image + ) + +private fun previewUploadingMessage(statusIcon: MessageStatusIcon = MessageStatusIcon.SENDING) = + ChatMessageUi( + id = 0, + message = "{file}", + plainMessage = "photo.jpg", + renderMarkdown = false, + actorDisplayName = "Jane Doe", + isThread = false, + threadTitle = "", + threadReplies = 0, + incoming = false, + isDeleted = false, + avatarUrl = null, + statusIcon = statusIcon, + timestamp = System.currentTimeMillis() / 1000, + date = java.time.LocalDate.now(), + content = previewUploadingContent(), + reactions = emptyList(), + referenceId = "preview-ref-id" + ) + +@Suppress("MagicNumber") +@ChatMessagePreviews +@Composable +private fun UploadingMediaMessageProgressPreview() { + PreviewContainer { + CompositionLocalProvider(LocalUploadProgressProvider provides { 42 }) { + UploadingMediaMessage( + typeContent = previewUploadingContent(), + message = previewUploadingMessage() + ) + } + } +} + +@ChatMessagePreviews +@Composable +private fun UploadingMediaMessageIndeterminatePreview() { + PreviewContainer { + UploadingMediaMessage( + typeContent = previewUploadingContent(), + message = previewUploadingMessage() + ) + } +} + +@ChatMessagePreviews +@Composable +private fun UploadingMediaMessageFailedPreview() { + PreviewContainer { + UploadingMediaMessage( + typeContent = previewUploadingContent(), + message = previewUploadingMessage(statusIcon = MessageStatusIcon.FAILED) + ) + } +} + +@ChatMessagePreviews +@Composable +private fun UploadingMediaMessageSentPreview() { + PreviewContainer { + UploadingMediaMessage( + typeContent = previewUploadingContent(), + message = previewUploadingMessage(statusIcon = MessageStatusIcon.SENT) + ) + } +} + +@ChatMessagePreviews +@Composable +private fun UploadingMediaMessageNonImagePreview() { + PreviewContainer { + UploadingMediaMessage( + typeContent = MessageTypeContent.UploadingMedia( + localFileUri = "", + caption = "document.pdf", + mimeType = "application/pdf", + drawableResourceId = R.drawable.ic_mimetype_application_pdf + ), + message = previewUploadingMessage() + ) + } +} diff --git a/app/src/main/java/com/nextcloud/talk/upload/chunked/ChunkedFileUploader.kt b/app/src/main/java/com/nextcloud/talk/upload/chunked/ChunkedFileUploader.kt index 7e8b2b1e347..3423363577f 100644 --- a/app/src/main/java/com/nextcloud/talk/upload/chunked/ChunkedFileUploader.kt +++ b/app/src/main/java/com/nextcloud/talk/upload/chunked/ChunkedFileUploader.kt @@ -32,6 +32,8 @@ import com.nextcloud.talk.filebrowser.models.properties.OCFavorite import com.nextcloud.talk.filebrowser.models.properties.OCId import com.nextcloud.talk.filebrowser.models.properties.OCSize import com.nextcloud.talk.jobs.ShareOperationWorker +import com.nextcloud.talk.dagger.modules.RestModule +import com.nextcloud.talk.data.user.model.User import com.nextcloud.talk.remotefilebrowser.model.RemoteFileBrowserItem import com.nextcloud.talk.utils.ApiUtils import com.nextcloud.talk.utils.FileUtils diff --git a/app/src/main/java/com/nextcloud/talk/upload/normal/FileUploader.kt b/app/src/main/java/com/nextcloud/talk/upload/normal/FileUploader.kt index 55e1d334b8c..000cf522ec2 100644 --- a/app/src/main/java/com/nextcloud/talk/upload/normal/FileUploader.kt +++ b/app/src/main/java/com/nextcloud/talk/upload/normal/FileUploader.kt @@ -16,7 +16,6 @@ import com.nextcloud.talk.api.NcApi import com.nextcloud.talk.api.NcApiCoroutines import com.nextcloud.talk.dagger.modules.RestModule import com.nextcloud.talk.data.user.model.User -import com.nextcloud.talk.jobs.ShareOperationWorker import com.nextcloud.talk.utils.ApiUtils import com.nextcloud.talk.utils.FileUtils import io.reactivex.Observable @@ -78,12 +77,6 @@ class FileUploader( .observeOn(AndroidSchedulers.mainThread()) .flatMap { response -> if (response.isSuccessful) { - ShareOperationWorker.shareFile( - roomToken, - currentUser, - remotePath, - metaData - ) FileUtils.copyFileToCache(context, sourceFileUri, fileName) Observable.just(true) } else { From 7db055fccdb142bfe791e0ec70f2e34f0c2cf4d8 Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Tue, 9 Jun 2026 11:41:29 +0200 Subject: [PATCH 02/26] fixes after resolving merge conflicts App compiles but the upload progress is buggy Signed-off-by: Marcel Hibbe --- .../com/nextcloud/talk/jobs/UploadAndShareFilesWorker.kt | 7 ++++++- app/src/main/java/com/nextcloud/talk/ui/chat/ChatView.kt | 2 +- .../nextcloud/talk/upload/chunked/ChunkedFileUploader.kt | 2 -- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/jobs/UploadAndShareFilesWorker.kt b/app/src/main/java/com/nextcloud/talk/jobs/UploadAndShareFilesWorker.kt index 1a4feb20bcb..2661c06e889 100644 --- a/app/src/main/java/com/nextcloud/talk/jobs/UploadAndShareFilesWorker.kt +++ b/app/src/main/java/com/nextcloud/talk/jobs/UploadAndShareFilesWorker.kt @@ -145,7 +145,12 @@ class UploadAndShareFilesWorker(val context: Context, workerParameters: WorkerPa currentUser.capabilities!!.spreedCapability!! ) file?.let { isChunkedUploading = it.length() > CHUNK_UPLOAD_THRESHOLD_SIZE } - val uploadSuccess: Boolean = uploadFile(sourceFileUri, remotePath, useConversationSubfolders) + val uploadSuccess: Boolean = uploadFile( + sourceFileUri = sourceFileUri, + metaData = metaData, + remotePath = remotePath, + useConversationSubfolders = useConversationSubfolders + ) if (uploadSuccess) { val shareSuccess = shareFile(remotePath, metaData) diff --git a/app/src/main/java/com/nextcloud/talk/ui/chat/ChatView.kt b/app/src/main/java/com/nextcloud/talk/ui/chat/ChatView.kt index 399a4075dc1..78e2055334f 100644 --- a/app/src/main/java/com/nextcloud/talk/ui/chat/ChatView.kt +++ b/app/src/main/java/com/nextcloud/talk/ui/chat/ChatView.kt @@ -427,7 +427,7 @@ fun ChatView( onQuotedMessageClick = handleQuotedMessageClick, onSystemMessageExpandClick = callbacks.messageCallbacks.onSystemMessageExpandClick, onAvatarClick = callbacks.messageCallbacks.onAvatarClick, - onCancelUpload = callbacks.onCancelUpload + onCancelUpload = callbacks.messageCallbacks.onCancelUpload ) ) } diff --git a/app/src/main/java/com/nextcloud/talk/upload/chunked/ChunkedFileUploader.kt b/app/src/main/java/com/nextcloud/talk/upload/chunked/ChunkedFileUploader.kt index 3423363577f..7e8b2b1e347 100644 --- a/app/src/main/java/com/nextcloud/talk/upload/chunked/ChunkedFileUploader.kt +++ b/app/src/main/java/com/nextcloud/talk/upload/chunked/ChunkedFileUploader.kt @@ -32,8 +32,6 @@ import com.nextcloud.talk.filebrowser.models.properties.OCFavorite import com.nextcloud.talk.filebrowser.models.properties.OCId import com.nextcloud.talk.filebrowser.models.properties.OCSize import com.nextcloud.talk.jobs.ShareOperationWorker -import com.nextcloud.talk.dagger.modules.RestModule -import com.nextcloud.talk.data.user.model.User import com.nextcloud.talk.remotefilebrowser.model.RemoteFileBrowserItem import com.nextcloud.talk.utils.ApiUtils import com.nextcloud.talk.utils.FileUtils From b45db848e4954838ce0326df8d6601d64bb1ab6c Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Sat, 8 Aug 2026 21:03:43 +0200 Subject: [PATCH 03/26] fix and improve upload progress in chat Bundles the fixes and follow-up polish for the upload-progress/placeholder feature from the recent merge-conflict cleanup: Correctness fixes: - UploadAndShareFilesWorker called shareFile() unconditionally after every successful upload, even though two other paths already share the file themselves: ChunkedFileUploader still had a leftover ShareOperationWorker.shareFile() call, so any chunked upload (files >1MB) without conversation subfolders posted the attachment twice; and conversation-subfolder uploads already share via postConversationAttachment, so the extra call tried to share a path the file was never uploaded to, failed, and incorrectly marked successful uploads as FAILED. - uploadUsingConversationSubfolders() sent a freshly generated UUID as the message's referenceId instead of the placeholder's actual referenceId, so the server echoed back the wrong id and the temp placeholder could never be matched against the real incoming message, leaving it stuck forever. - sendUnsentChatMessages() (resend-on-reconnect) picked up FAILED upload placeholders and reposted their "{file}" sentinel text as a bogus new message. Placeholders with a file attachment are now excluded from that resend path. - The "upload completed" signal that triggers an immediate message refetch was commented out, so a successfully uploaded video's placeholder could spin forever until the chat was closed and reopened. - Coil's AsyncImage never showed a composable-supplied fallback painter when passed a pre-built ImageRequest with null data, so previews without a server URL (e.g. video with no server preview) silently fell back to Coil's own null-data handling instead of our local first-frame image. Reliability: - UploadAndShareFilesWorker now retries transient network failures (socket resets, timeouts) with backoff and a network-connected constraint instead of failing immediately, up to a bounded number of attempts. UI/UX: - Replaced the linear upload progress bar with a WhatsApp-style circular spinner overlay (with cancel button) centered on the thumbnail, and fixed a metadata-layout bug that left a padding gap next to the placeholder. - Removed the persistent Android notifications duplicating in-chat upload/ compression progress; kept the upload-failed notification. - Stopped treating a file's name as its caption; only real captions are shown, matching how sent messages already behave. - Sized the video upload placeholder to the video's real aspect ratio (16:9 fallback) instead of collapsing to a small generic icon. - Added a local-first-frame fallback, cached to disk keyed by referenceId, for videos whose server preview is unavailable, so they don't show a generic icon indefinitely. - The play button overlay now shows on all video messages (not just ones with a server preview) with a WhatsApp-style semi-transparent dark circle behind it. Assisted-by: Claude:claude-sonnet-5 Signed-off-by: Marcel Hibbe --- .../com/nextcloud/talk/chat/ChatActivity.kt | 5 +- .../talk/chat/data/ChatMessageRepository.kt | 1 + .../network/OfflineFirstChatRepository.kt | 12 +- .../talk/chat/ui/model/ChatMessageUi.kt | 8 +- .../talk/chat/viewmodels/ChatViewModel.kt | 31 +- .../talk/jobs/UploadAndShareFilesWorker.kt | 250 +++---------- .../nextcloud/talk/ui/chat/ChatMessageView.kt | 21 +- .../nextcloud/talk/ui/chat/MediaMessage.kt | 350 ++++++++++++++---- .../upload/chunked/ChunkedFileUploader.kt | 21 +- .../talk/utils/VideoThumbnailCache.kt | 49 +++ 10 files changed, 428 insertions(+), 320 deletions(-) create mode 100644 app/src/main/java/com/nextcloud/talk/utils/VideoThumbnailCache.kt diff --git a/app/src/main/java/com/nextcloud/talk/chat/ChatActivity.kt b/app/src/main/java/com/nextcloud/talk/chat/ChatActivity.kt index 2c02977d29f..700ac56a0f3 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/ChatActivity.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/ChatActivity.kt @@ -170,6 +170,7 @@ import com.nextcloud.talk.ui.chat.ChatView import com.nextcloud.talk.ui.chat.ChatViewCallbacks import com.nextcloud.talk.ui.chat.ChatViewState import com.nextcloud.talk.ui.chat.LocalUploadProgressProvider +import com.nextcloud.talk.ui.chat.LocalUploadedLocalPreviewProvider import com.nextcloud.talk.ui.dialog.DateTimeCompose import com.nextcloud.talk.ui.dialog.GetPinnedOptionsDialog import com.nextcloud.talk.ui.dialog.SaveToStorageDialogFragment @@ -878,12 +879,14 @@ class ChatActivity : SideEffect { chatListState = listState } val uploadProgressMap by chatViewModel.uploadProgressMap.collectAsStateWithLifecycle() + val uploadedLocalPreviewMap by chatViewModel.uploadedLocalPreviewMap.collectAsStateWithLifecycle() CompositionLocalProvider( LocalViewThemeUtils provides viewThemeUtils, LocalMessageUtils provides messageUtils, LocalOpenGraphFetcher provides { url -> chatViewModel.fetchOpenGraph(url) }, - LocalUploadProgressProvider provides { refId -> uploadProgressMap[refId] } + LocalUploadProgressProvider provides { refId -> uploadProgressMap[refId] }, + LocalUploadedLocalPreviewProvider provides { refId -> uploadedLocalPreviewMap[refId] } ) { val isOneToOneConversation by remember { mutableStateOf(uiState.isOneToOneConversation) } Log.d(TAG, "isOneToOneConversation=" + isOneToOneConversation) diff --git a/app/src/main/java/com/nextcloud/talk/chat/data/ChatMessageRepository.kt b/app/src/main/java/com/nextcloud/talk/chat/data/ChatMessageRepository.kt index ce1d99a40fc..7fc3d13c983 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/data/ChatMessageRepository.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/data/ChatMessageRepository.kt @@ -153,6 +153,7 @@ interface ChatMessageRepository : LifecycleAwareManager { @Suppress("LongParameterList") suspend fun addUploadPlaceholderMessage( localFileUri: String, + fileName: String, caption: String, mimeType: String?, fileSize: Long, diff --git a/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt b/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt index 75234876703..5dc973702de 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt @@ -687,6 +687,7 @@ class OfflineFirstChatRepository @Inject constructor( @Suppress("Detekt.TooGenericExceptionCaught", "LongMethod") override suspend fun addUploadPlaceholderMessage( localFileUri: String, + fileName: String, caption: String, mimeType: String?, fileSize: Long, @@ -714,7 +715,7 @@ class OfflineFirstChatRepository @Inject constructor( val fileParams = hashMapOf( "type" to "file", - "name" to caption, + "name" to fileName, "mimetype" to (mimeType ?: ""), "size" to fileSize.toString(), "path" to localFileUri @@ -728,7 +729,8 @@ class OfflineFirstChatRepository @Inject constructor( internalConversationId = internalConversationId, id = placeholderId, threadId = threadId, - message = "{file}", + // "{file}" is the sentinel the server (and rest of this app) uses for "no caption" + message = caption.ifEmpty { "{file}" }, deleted = false, token = conversationModel.token, actorId = currentUser.userId!!, @@ -798,7 +800,11 @@ class OfflineFirstChatRepository @Inject constructor( override suspend fun sendUnsentChatMessages(credentials: String, url: String) { val tempMessages = chatDao.getTempUnsentMessagesForConversation(internalConversationId, threadId).first() - tempMessages.sortedBy { it.internalId }.onEach { + // File-upload placeholders are also temporary messages, but they must never be resent as plain + // text here: their "message" field is just the "{file}" sentinel, and a failed/interrupted upload + // needs a real re-upload, not a bogus text message reusing its referenceId. + val unsentTextMessages = tempMessages.filterNot { it.messageParameters?.containsKey("file") == true } + unsentTextMessages.sortedBy { it.internalId }.onEach { sendChatMessage( credentials, url, diff --git a/app/src/main/java/com/nextcloud/talk/chat/ui/model/ChatMessageUi.kt b/app/src/main/java/com/nextcloud/talk/chat/ui/model/ChatMessageUi.kt index bc045155a01..8a25f459626 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/ui/model/ChatMessageUi.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/ui/model/ChatMessageUi.kt @@ -81,7 +81,8 @@ sealed interface MessageTypeContent { data class UploadingMedia( val localFileUri: String, - val caption: String, + val fileName: String, + val caption: String?, val mimeType: String?, val drawableResourceId: Int ) : MessageTypeContent @@ -276,12 +277,15 @@ fun getMessageTypeContent(user: User, message: ChatMessage, isClassified: Boolea ?: MessageTypeContent.RegularText } +private const val FILE_PLACEHOLDER_MESSAGE = "{file}" + fun getUploadingMediaContent(message: ChatMessage): MessageTypeContent.UploadingMedia { val mimetype = message.fileParameters.mimetype val drawableResourceId = DrawableUtils.getDrawableResourceIdForMimeType(mimetype) return MessageTypeContent.UploadingMedia( localFileUri = message.fileParameters.path.orEmpty(), - caption = message.fileParameters.name.orEmpty(), + fileName = message.fileParameters.name.orEmpty(), + caption = message.message.takeIf { it != FILE_PLACEHOLDER_MESSAGE }, mimeType = mimetype.takeIf { !it.isNullOrEmpty() }, drawableResourceId = drawableResourceId ) diff --git a/app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt b/app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt index e24ee9f1b71..4c90d09947a 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt @@ -224,6 +224,12 @@ class ChatViewModel @AssistedInject constructor( private val _uploadProgressMap = MutableStateFlow>(emptyMap()) val uploadProgressMap: StateFlow> = _uploadProgressMap + // Maps referenceId -> local device fileUri, kept around for a while after the upload finishes so the + // final message can show the file we already have on disk instead of a generic mimetype icon while it + // waits for the server-side preview to load for the first time. + private val _uploadedLocalPreviewMap = MutableStateFlow>(emptyMap()) + val uploadedLocalPreviewMap: StateFlow> = _uploadedLocalPreviewMap + // Maps referenceId -> fileUri for cancellation support private val uploadReferenceToUri = mutableMapOf() @@ -234,6 +240,7 @@ class ChatViewModel @AssistedInject constructor( chatRepository.deleteTempMessageByReferenceId(referenceId) } _uploadProgressMap.update { it - referenceId } + _uploadedLocalPreviewMap.update { it - referenceId } } fun getChatRepository(): ChatMessageRepository = chatRepository @@ -2058,7 +2065,8 @@ class ChatViewModel @AssistedInject constructor( viewModelScope.launch { chatRepository.addUploadPlaceholderMessage( localFileUri = fileUri, - caption = caption.ifEmpty { fileName }, + fileName = fileName, + caption = caption, mimeType = mimeType, fileSize = fileSize, referenceId = referenceId @@ -2068,17 +2076,18 @@ class ChatViewModel @AssistedInject constructor( val internalConversationId = "${currentUser.id}@$chatRoomToken" val workerId = UploadAndShareFilesWorker.upload( - fileUri, - room, - displayName, - metaData, - compressImages, - referenceId, - internalConversationId + fileUri = fileUri, + roomToken = room, + conversationName = displayName, + metaData = metaData, + referenceId = referenceId, + internalConversationId = internalConversationId, + compressImages = compressImages ) if (!isVoiceMessage) { uploadReferenceToUri[referenceId] = fileUri + _uploadedLocalPreviewMap.update { it + (referenceId to fileUri) } observeUploadProgress(workerId, referenceId) } } catch (e: IllegalArgumentException) { @@ -2115,6 +2124,10 @@ class ChatViewModel @AssistedInject constructor( if (workInfo.state.isFinished) { _uploadProgressMap.update { it - referenceId } uploadReferenceToUri.remove(referenceId) + viewModelScope.launch { + delay(LOCAL_PREVIEW_GRACE_PERIOD_MS) + _uploadedLocalPreviewMap.update { it - referenceId } + } } } .launchIn(viewModelScope) @@ -2417,7 +2430,7 @@ class ChatViewModel @AssistedInject constructor( private const val LOAD_MORE_MESSAGES_LIMIT = 100 private const val POST_UPLOAD_FETCH_MAX_ATTEMPTS = 4 private const val POST_UPLOAD_FETCH_RETRY_DELAY_MS = 1_500L - + private const val LOCAL_PREVIEW_GRACE_PERIOD_MS = 15_000L private const val PLAUSIBLE_MESSAGE_ID_BUFFER = 10_000L /** diff --git a/app/src/main/java/com/nextcloud/talk/jobs/UploadAndShareFilesWorker.kt b/app/src/main/java/com/nextcloud/talk/jobs/UploadAndShareFilesWorker.kt index 2661c06e889..222e4c9fd18 100644 --- a/app/src/main/java/com/nextcloud/talk/jobs/UploadAndShareFilesWorker.kt +++ b/app/src/main/java/com/nextcloud/talk/jobs/UploadAndShareFilesWorker.kt @@ -11,25 +11,25 @@ import android.Manifest import android.annotation.SuppressLint import android.app.Activity import android.app.NotificationManager -import android.app.PendingIntent import android.content.Context -import android.content.Intent import android.net.Uri import android.os.Build -import android.os.Bundle import android.os.SystemClock import android.util.Log import androidx.core.app.NotificationCompat import androidx.core.net.toUri +import androidx.work.BackoffPolicy +import androidx.work.Constraints import androidx.work.Data import androidx.work.ExistingWorkPolicy +import androidx.work.NetworkType import androidx.work.OneTimeWorkRequest import androidx.work.WorkManager +import androidx.work.WorkRequest import androidx.work.Worker import androidx.work.WorkerParameters import autodagger.AutoInjector import com.nextcloud.talk.R -import com.nextcloud.talk.activities.MainActivity import com.nextcloud.talk.api.NcApi import com.nextcloud.talk.api.NcApiCoroutines import com.nextcloud.talk.application.NextcloudTalkApplication @@ -50,21 +50,20 @@ import com.nextcloud.talk.utils.ImageCompressor import com.nextcloud.talk.utils.NotificationUtils import com.nextcloud.talk.utils.RemoteFileUtils import com.nextcloud.talk.utils.VideoCompressor -import com.nextcloud.talk.utils.bundle.BundleKeys.KEY_INTERNAL_USER_ID -import com.nextcloud.talk.utils.bundle.BundleKeys.KEY_ROOM_TOKEN import com.nextcloud.talk.utils.database.user.CurrentUserProviderOld import com.nextcloud.talk.utils.permissions.PlatformPermissionUtil import com.nextcloud.talk.utils.preferences.AppPreferences import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.MutableSharedFlow import kotlinx.coroutines.flow.SharedFlow -import kotlinx.coroutines.runBlocking import kotlinx.coroutines.flow.firstOrNull import kotlinx.coroutines.runBlocking import okhttp3.MediaType.Companion.toMediaTypeOrNull import okhttp3.OkHttpClient import java.io.File +import java.io.IOException import java.util.UUID +import java.util.concurrent.TimeUnit import javax.inject.Inject @AutoInjector(NextcloudTalkApplication::class) @@ -99,8 +98,6 @@ class UploadAndShareFilesWorker(val context: Context, workerParameters: WorkerPa lateinit var fileName: String private var mNotifyManager: NotificationManager? = null - private var mBuilder: NotificationCompat.Builder? = null - private var notificationId: Int = 0 lateinit var roomToken: String lateinit var conversationName: String @@ -111,7 +108,7 @@ class UploadAndShareFilesWorker(val context: Context, workerParameters: WorkerPa private var referenceId: String? = null private var internalConversationId: String? = null - @Suppress("Detekt.TooGenericExceptionCaught") + @Suppress("Detekt.TooGenericExceptionCaught", "Detekt.LongMethod") override fun doWork(): Result { NextcloudTalkApplication.sharedApplication!!.componentApplication.inject(this) @@ -153,34 +150,49 @@ class UploadAndShareFilesWorker(val context: Context, workerParameters: WorkerPa ) if (uploadSuccess) { - val shareSuccess = shareFile(remotePath, metaData) - cancelNotification() + // useConversationSubfolders already shares as part of uploadFile() via + // postConversationAttachment, so only share explicitly for the plain upload path. + val shareSuccess = useConversationSubfolders || shareFile(remotePath, metaData) if (shareSuccess) { updatePlaceholderStatus(SendStatus.SENT_PENDING_ACK) - // _uploadCompletedFlow.tryEmit(roomToken) <- Check if this still makes sense! + _uploadCompletedFlow.tryEmit(roomToken) return Result.success() } Log.e(TAG, "Share operation failed after upload") - showFailedToUploadNotification() - updatePlaceholderStatus(SendStatus.FAILED) - return Result.failure() + return failUpload() } else if (isStopped) { // since work is cancelled the result would be ignored anyways return Result.failure() } Log.e(TAG, "Something went wrong when trying to upload file") - showFailedToUploadNotification() - updatePlaceholderStatus(SendStatus.FAILED) - return Result.failure() + failUpload() + } catch (e: IOException) { + // Transient network failures (connection reset, timeout, dropped Wi-Fi, ...) shouldn't + // require the user to manually resend - retry a few times with backoff instead, and only + // give up once we've exhausted the allowed attempts. + Log.w( + TAG, + "Network error while uploading file (attempt ${runAttemptCount + 1}/$MAX_UPLOAD_ATTEMPTS)", + e + ) + if (runAttemptCount < MAX_UPLOAD_ATTEMPTS - 1) { + Result.retry() + } else { + failUpload() + } } catch (e: Exception) { Log.e(TAG, "Something went wrong when trying to upload file", e) - showFailedToUploadNotification() - updatePlaceholderStatus(SendStatus.FAILED) - return Result.failure() + failUpload() } } + private fun failUpload(): Result { + showFailedToUploadNotification() + updatePlaceholderStatus(SendStatus.FAILED) + return Result.failure() + } + /** * Replaces [file] and [fileName] with a compressed copy if [sourceFileUri] points to a * compressible image or video, returning the [Uri] that should be uploaded. @@ -192,7 +204,7 @@ class UploadAndShareFilesWorker(val context: Context, workerParameters: WorkerPa val compressedFile = when { ImageCompressor.isCompressible(mimeType) -> ImageCompressor.compress(context, originalFile) - VideoCompressor.isCompressible(mimeType) -> compressVideoWithProgress(originalFile) + VideoCompressor.isCompressible(mimeType) -> VideoCompressor.compress(context, originalFile) else -> null } ?: return sourceFileUri @@ -201,15 +213,6 @@ class UploadAndShareFilesWorker(val context: Context, workerParameters: WorkerPa return Uri.fromFile(compressedFile) } - /** - * Video compression can take a while, so the upload notification is repurposed to show its - * progress before it transitions into the actual upload progress. - */ - private fun compressVideoWithProgress(originalFile: File): File? { - showCompressionStartedNotification() - return VideoCompressor.compress(context, originalFile, onProgress = ::onCompressionProgress) - } - private fun uploadFile( sourceFileUri: Uri, metaData: String?, @@ -222,16 +225,12 @@ class UploadAndShareFilesWorker(val context: Context, workerParameters: WorkerPa uploadUsingConversationSubfolders(sourceFileUri, metaData) } else if (isChunkedUploading) { Log.d(TAG, "starting chunked upload because size is " + file!!.length()) - initNotificationWithPercentage() val mimeType = context.contentResolver.getType(sourceFileUri)?.toMediaTypeOrNull() chunkedFileUploader = ChunkedFileUploader( okHttpClient, currentUser, - roomToken, - null, this, - ncApiCoroutines, - useConversationSubfolders + ncApiCoroutines ) chunkedFileUploader!!.upload(file!!, mimeType, remotePath) } else { @@ -275,16 +274,12 @@ class UploadAndShareFilesWorker(val context: Context, workerParameters: WorkerPa val tempRemotePath = "/$draftFolderPath/$uploadId-$fileName" val uploadSuccess = if (isChunkedUploading) { - initNotificationWithPercentage() val mimeType = context.contentResolver.getType(sourceFileUri)?.toMediaTypeOrNull() chunkedFileUploader = ChunkedFileUploader( okHttpClient, currentUser, - roomToken, - metaData, this@UploadAndShareFilesWorker, - ncApiCoroutines, - true + ncApiCoroutines ) chunkedFileUploader!!.upload(file!!, mimeType, tempRemotePath) } else { @@ -298,7 +293,7 @@ class UploadAndShareFilesWorker(val context: Context, workerParameters: WorkerPa val params = PostConversationAttachmentRequest().apply { filePath = tempRemotePath - referenceId = uploadId + referenceId = this@UploadAndShareFilesWorker.referenceId.orEmpty() talkMetaData = metaData fileName = predictedName } @@ -344,13 +339,6 @@ class UploadAndShareFilesWorker(val context: Context, workerParameters: WorkerPa override fun onTransferProgress(percentage: Int) { setProgressAsync(Data.Builder().putInt(PROGRESS_KEY, percentage).build()) - - val progressUpdateNotification = mBuilder!! - .setProgress(HUNDRED_PERCENT, percentage, false) - .setContentText(getNotificationContentText(percentage)) - .build() - - mNotifyManager!!.notify(notificationId, progressUpdateNotification) } private fun updatePlaceholderStatus(status: SendStatus) { @@ -362,153 +350,13 @@ class UploadAndShareFilesWorker(val context: Context, workerParameters: WorkerPa override fun onStopped() { if (file != null && isChunkedUploading) { - chunkedFileUploader?.abortUpload { - mNotifyManager?.cancel(notificationId) - } + chunkedFileUploader?.abortUpload {} } super.onStopped() } private fun initNotificationSetup() { mNotifyManager = context.getSystemService(Context.NOTIFICATION_SERVICE) as NotificationManager - mBuilder = NotificationCompat.Builder( - context, - NotificationUtils.NotificationChannels - .NOTIFICATION_CHANNEL_UPLOADS.name - ) - notificationId = SystemClock.uptimeMillis().toInt() - } - - private fun initNotificationWithPercentage() { - val initNotification = mBuilder!! - .setContentTitle(context.resources.getString(R.string.nc_upload_in_progess)) - .setContentText(getNotificationContentText(ZERO_PERCENT)) - .setSmallIcon(R.drawable.upload_white) - .setOngoing(true) - .setProgress(HUNDRED_PERCENT, ZERO_PERCENT, false) - .setPriority(NotificationCompat.PRIORITY_LOW) - .setGroup(NotificationUtils.KEY_UPLOAD_GROUP) - .setContentIntent(getIntentToOpenConversation()) - .addAction( - R.drawable.ic_cancel_white_24dp, - getResourceString(context, R.string.nc_cancel), - getCancelUploadIntent() - ) - .build() - - mNotifyManager!!.notify(notificationId, initNotification) - // only need one summary notification but multiple upload worker can call it more than once but it is safe - // because of the same notification object config and id. - makeSummaryNotification() - } - - /** - * Shows the same upload notification, but reflecting the compression phase that precedes the - * actual upload. Reuses [notificationId] so it later morphs into the upload progress notification - * instead of appearing as a separate entry. - */ - private fun showCompressionStartedNotification() { - val compressionNotification = mBuilder!! - .setContentTitle(context.resources.getString(R.string.nc_compress_in_progress)) - .setContentText(getCompressionNotificationContentText(ZERO_PERCENT)) - .setSmallIcon(R.drawable.upload_white) - .setOngoing(true) - .setProgress(HUNDRED_PERCENT, ZERO_PERCENT, false) - .setPriority(NotificationCompat.PRIORITY_LOW) - .setGroup(NotificationUtils.KEY_UPLOAD_GROUP) - .setContentIntent(getIntentToOpenConversation()) - .addAction( - R.drawable.ic_cancel_white_24dp, - getResourceString(context, R.string.nc_cancel), - getCancelUploadIntent() - ) - .build() - - mNotifyManager!!.notify(notificationId, compressionNotification) - makeSummaryNotification() - } - - private fun onCompressionProgress(percentage: Int) { - val progressUpdateNotification = mBuilder!! - .setProgress(HUNDRED_PERCENT, percentage, false) - .setContentText(getCompressionNotificationContentText(percentage)) - .build() - - mNotifyManager!!.notify(notificationId, progressUpdateNotification) - } - - private fun getCompressionNotificationContentText(percentage: Int): String = - String.format( - getResourceString(context, R.string.nc_compress_notification_text), - getShortenedFileName(), - percentage - ) - - private fun makeSummaryNotification() { - // summary notification encapsulating the group of notifications - val summaryNotification = NotificationCompat.Builder( - context, - NotificationUtils.NotificationChannels - .NOTIFICATION_CHANNEL_UPLOADS.name - ).setSmallIcon(R.drawable.upload_white) - .setGroup(NotificationUtils.KEY_UPLOAD_GROUP) - .setGroupSummary(true) - .build() - - mNotifyManager?.notify(NotificationUtils.GROUP_SUMMARY_NOTIFICATION_ID, summaryNotification) - } - - private fun getActiveUploadNotifications(): Int? { - // filter out active notifications that are upload notifications using group - return mNotifyManager?.activeNotifications?.filter { - it.notification.group == NotificationUtils - .KEY_UPLOAD_GROUP - }?.size - } - - private fun cancelNotification() { - mNotifyManager?.cancel(notificationId) - // summary notification would not get dismissed automatically - // if child notifications are cancelled programmatically - // so check if only 1 notification left if yes - // then cancel it (which is summary notification) - if (getActiveUploadNotifications() == 1) { - mNotifyManager?.cancel(NotificationUtils.GROUP_SUMMARY_NOTIFICATION_ID) - } - } - - private fun getNotificationContentText(percentage: Int): String = - String.format( - getResourceString(context, R.string.nc_upload_notification_text), - getShortenedFileName(), - conversationName, - percentage - ) - - private fun getShortenedFileName(): String = - if (fileName.length > NOTIFICATION_FILE_NAME_MAX_LENGTH) { - THREE_DOTS + fileName.takeLast(NOTIFICATION_FILE_NAME_MAX_LENGTH) - } else { - fileName - } - - private fun getCancelUploadIntent(): PendingIntent = - WorkManager.getInstance(applicationContext) - .createCancelPendingIntent(id) - - private fun getIntentToOpenConversation(): PendingIntent? { - val bundle = Bundle() - val intent = Intent(context, MainActivity::class.java) - intent.flags = Intent.FLAG_ACTIVITY_SINGLE_TOP or Intent.FLAG_ACTIVITY_NEW_TASK - - bundle.putString(KEY_ROOM_TOKEN, roomToken) - bundle.putLong(KEY_INTERNAL_USER_ID, currentUser.id!!) - - intent.putExtras(bundle) - - val requestCode = System.currentTimeMillis().toInt() - val intentFlag = PendingIntent.FLAG_UPDATE_CURRENT or PendingIntent.FLAG_IMMUTABLE - return PendingIntent.getActivity(context, requestCode, intent, intentFlag) } private fun showFailedToUploadNotification() { @@ -529,8 +377,6 @@ class UploadAndShareFilesWorker(val context: Context, workerParameters: WorkerPa .setOngoing(false) .build() - mNotifyManager?.cancel(notificationId) - // update current notification with failure info mNotifyManager!!.notify(SystemClock.uptimeMillis().toInt(), failureNotification) } @@ -547,10 +393,10 @@ class UploadAndShareFilesWorker(val context: Context, workerParameters: WorkerPa const val PROGRESS_KEY = "UPLOAD_PROGRESS" private const val COMPRESS_IMAGES = "COMPRESS_IMAGES" private const val CHUNK_UPLOAD_THRESHOLD_SIZE: Long = 1024 * 1024 - private const val NOTIFICATION_FILE_NAME_MAX_LENGTH = 20 - private const val THREE_DOTS = "…" - private const val HUNDRED_PERCENT = 100 - private const val ZERO_PERCENT = 0 + + // Total attempts allowed for a single upload (1 initial run + retries) before giving up on a + // transient network failure and marking the placeholder FAILED. + private const val MAX_UPLOAD_ATTEMPTS = 4 const val REQUEST_PERMISSION = 3123 private val _uploadCompletedFlow: MutableSharedFlow = MutableSharedFlow( @@ -619,6 +465,16 @@ class UploadAndShareFilesWorker(val context: Context, workerParameters: WorkerPa .build() val uploadWorker: OneTimeWorkRequest = OneTimeWorkRequest.Builder(UploadAndShareFilesWorker::class.java) .setInputData(data) + .setConstraints( + Constraints.Builder() + .setRequiredNetworkType(NetworkType.CONNECTED) + .build() + ) + .setBackoffCriteria( + BackoffPolicy.EXPONENTIAL, + WorkRequest.MIN_BACKOFF_MILLIS, + TimeUnit.MILLISECONDS + ) .build() WorkManager.getInstance().enqueueUniqueWork(fileUri, ExistingWorkPolicy.KEEP, uploadWorker) return uploadWorker.id diff --git a/app/src/main/java/com/nextcloud/talk/ui/chat/ChatMessageView.kt b/app/src/main/java/com/nextcloud/talk/ui/chat/ChatMessageView.kt index 979e1c76851..40304d0f7fa 100644 --- a/app/src/main/java/com/nextcloud/talk/ui/chat/ChatMessageView.kt +++ b/app/src/main/java/com/nextcloud/talk/ui/chat/ChatMessageView.kt @@ -208,17 +208,18 @@ fun ChatMessageView( } is MessageTypeContent.UploadingMedia -> { - UploadingMediaMessage( - typeContent = content, - message = message, - isOneToOneConversation = context.isOneToOneConversation, - conversationThreadId = context.conversationThreadId, - onCancelUpload = callbacks.onCancelUpload - ) - } + UploadingMediaMessage( + typeContent = content, + message = message, + isOneToOneConversation = context.isOneToOneConversation, + conversationThreadId = context.conversationThreadId, + onCancelUpload = callbacks.onCancelUpload + ) + } - else -> { - Log.d("ChatView", "Unknown message type: ${'$'}content")} + else -> { + Log.d("ChatView", "Unknown message type: $content") + } } } val useContainerHighlight = highlightSearchTerm.isNullOrBlank() || isSelected diff --git a/app/src/main/java/com/nextcloud/talk/ui/chat/MediaMessage.kt b/app/src/main/java/com/nextcloud/talk/ui/chat/MediaMessage.kt index 7af06c0c187..f6ac151e283 100644 --- a/app/src/main/java/com/nextcloud/talk/ui/chat/MediaMessage.kt +++ b/app/src/main/java/com/nextcloud/talk/ui/chat/MediaMessage.kt @@ -7,28 +7,32 @@ package com.nextcloud.talk.ui.chat +import android.graphics.Bitmap import android.util.Log +import androidx.compose.foundation.Image +import androidx.compose.foundation.background import androidx.compose.foundation.combinedClickable import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.aspectRatio +import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.size +import androidx.compose.foundation.shape.CircleShape import androidx.compose.foundation.shape.RoundedCornerShape import androidx.compose.material3.CircularProgressIndicator import androidx.compose.material.icons.Icons import androidx.compose.material.icons.filled.Close import androidx.compose.material3.Icon import androidx.compose.material3.IconButton -import androidx.compose.material3.CircularProgressIndicator -import androidx.compose.material3.LinearProgressIndicator import androidx.compose.material3.Text import androidx.compose.runtime.Composable import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableIntStateOf import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.compositionLocalOf +import androidx.compose.runtime.produceState import androidx.compose.runtime.remember import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.runtime.setValue @@ -44,10 +48,14 @@ import androidx.compose.ui.res.painterResource import androidx.compose.ui.res.stringResource import androidx.compose.runtime.CompositionLocalProvider import androidx.compose.ui.draw.blur +import androidx.compose.ui.unit.Dp import androidx.compose.ui.unit.dp import coil.compose.AsyncImage +import coil.compose.rememberAsyncImagePainter import coil.network.HttpException import com.nextcloud.talk.R +import com.nextcloud.talk.attachmentpreview.FileDescription +import com.nextcloud.talk.attachmentpreview.describeFile import com.nextcloud.talk.chat.data.model.FileParameters import com.nextcloud.talk.chat.data.model.decodeBlurhashPlaceholder import com.nextcloud.talk.chat.ui.model.ChatMessageUi @@ -56,12 +64,20 @@ import com.nextcloud.talk.chat.ui.model.MessageTypeContent import com.nextcloud.talk.contacts.load import com.nextcloud.talk.utils.Mimetype import com.nextcloud.talk.utils.MimetypeUtils +import com.nextcloud.talk.utils.VideoThumbnailCache +import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.delay import kotlinx.coroutines.launch +import kotlinx.coroutines.withContext import androidx.core.net.toUri val LocalUploadProgressProvider = compositionLocalOf<(referenceId: String) -> Int?> { { null } } +// Local device URI of a just-finished-uploading message, keyed by referenceId. Used to bridge the gap +// between the upload placeholder disappearing and the server-side preview finishing its first load, so +// we show the image we already have on disk instead of a generic mimetype icon. +val LocalUploadedLocalPreviewProvider = compositionLocalOf<(referenceId: String) -> String?> { { null } } + private const val FILE_PLACEHOLDER_MESSAGE = "{file}" private const val PREVIEW_MAX_RETRIES = 3 private const val PREVIEW_RETRY_DELAY_MS = 2_000L @@ -70,6 +86,20 @@ private const val TAG = "MediaMessage" private val mediaRadiusBig = 8.dp private val mediaRadiusSmall = 2.dp +private val uploadSpinnerSize = 56.dp +private val uploadSpinnerStrokeWidth = 3.dp +private const val UPLOAD_SCRIM_ALPHA = 0.25f +private const val UPLOAD_SPINNER_TRACK_ALPHA = 0.3f + +// Used to size the uploading-video placeholder before its real aspect ratio is known (or if it can't +// be read at all), so the bubble doesn't collapse to icon-size. 16:9 is the most common video shape. +private const val DEFAULT_VIDEO_ASPECT_RATIO = 16f / 9f +private const val VIDEO_PLACEHOLDER_BACKGROUND_ALPHA = 0.4f + +private val playButtonCircleSize = 56.dp +private val playButtonIconSize = 32.dp +private const val PLAY_BUTTON_CIRCLE_ALPHA = 0.45f + @Suppress("Detekt.LongMethod", "LongParameterList", "CyclomaticComplexMethod") @Composable fun MediaMessage( @@ -89,8 +119,50 @@ fun MediaMessage( ) } + val context = LocalContext.current + val isVideo = typeContent.mimeType.startsWith(Mimetype.VIDEO_PREFIX) + val hasServerPreview = !typeContent.previewUrl.isNullOrEmpty() + + val getLocalPreviewUri = LocalUploadedLocalPreviewProvider.current + val localPreviewUri = if (typeContent.mimeType.startsWith(Mimetype.IMAGE_PREFIX) || isVideo) { + message.referenceId?.let(getLocalPreviewUri) + } else { + null + } + val localPreviewPainter = if (!isVideo && !localPreviewUri.isNullOrEmpty()) { + rememberAsyncImagePainter(model = localPreviewUri.toUri()) + } else { + null + } + + // The server didn't generate a preview for this video (unsupported codec, previews disabled, ...) + // - fall back to its first frame instead of a plain icon. Prefer the durable on-disk cache + // (survives leaving/reopening the chat or an app restart, unlike the in-memory localPreviewUri + // bridge, which only lives for the current upload); if it's not cached yet, re-extract from the + // local file while we still have it and cache it for next time. Coil has no built-in video frame + // decoding, so this reads the frame directly via MediaMetadataRetriever, same as the + // uploading-placeholder state. + val localVideoFramePainter = if (isVideo && !hasServerPreview) { + val refId = message.referenceId + val videoFrame by produceState(initialValue = null, key1 = refId, key2 = localPreviewUri) { + value = withContext(Dispatchers.IO) { + refId?.let { VideoThumbnailCache.get(context, it) } + ?: localPreviewUri?.let { uri -> + describeFile(context, uri, compress = false).videoThumbnail?.also { bitmap -> + refId?.let { VideoThumbnailCache.put(context, it, bitmap) } + } + } + } + } + videoFrame?.let { BitmapPainter(it.asImageBitmap()) } + } else { + null + } + + // A video shown via its local first frame counts as "has a preview" too, so the filename caption + // stays suppressed just like it would once the server's own preview becomes available. + val hasPreview = hasServerPreview || localVideoFramePainter != null val hasExplicitCaption = message.plainMessage != FILE_PLACEHOLDER_MESSAGE - val hasPreview = !typeContent.previewUrl.isNullOrEmpty() val captionText = when { hasExplicitCaption -> message.message !hasPreview -> message.message @@ -111,14 +183,17 @@ fun MediaMessage( forceTimeOverlay = !hasCaption, content = { Column { - val context = LocalContext.current val scope = rememberCoroutineScope() val isGif = MimetypeUtils.isGif(typeContent.mimeType) - val showPlayButton = !typeContent.previewUrl.isNullOrEmpty() && + // Every video gets a play button overlay, regardless of whether its preview came from + // the server or our own local-first-frame fallback (or neither, yet). + val showPlayButton = isVideo || ( - typeContent.mimeType.startsWith(Mimetype.VIDEO_PREFIX) || - typeContent.mimeType.startsWith(Mimetype.AUDIO_PREFIX) || - (isGif && !typeContent.animateGif) + !typeContent.previewUrl.isNullOrEmpty() && + ( + typeContent.mimeType.startsWith(Mimetype.AUDIO_PREFIX) || + (isGif && !typeContent.animateGif) + ) ) var retryCount by remember(typeContent.previewUrl) { mutableIntStateOf(0) } @@ -145,7 +220,10 @@ fun MediaMessage( if (w != null && h != null && w > 0 && h > 0) w.toFloat() / h else null } val loadedImage = remember(retryAwarePreviewUrl, typeContent.isClassified) { - if (typeContent.isClassified) { + if (typeContent.isClassified || retryAwarePreviewUrl == null) { + // Passing an ImageRequest built with null data (rather than a null model) here + // would make Coil resolve its own null-data handling instead of ever showing the + // fallback painter passed to AsyncImage below. null } else { load( @@ -158,58 +236,82 @@ fun MediaMessage( } val fallbackPainter = painterResource(typeContent.drawableResourceId) + val ownUploadPlaceholder = blurhashPainter ?: localPreviewPainter ?: fallbackPainter + + val mediaModifier = Modifier + .fillMaxWidth() + .then(if (aspectRatio != null) Modifier.aspectRatio(aspectRatio) else Modifier) + .padding(mediaInset) + .clip(mediaShape) + Box(modifier = Modifier.fillMaxWidth()) { val messageLongClickHandler = LocalMessageLongClickHandler.current - AsyncImage( - model = loadedImage, - contentDescription = stringResource(R.string.media_message_content_description), - placeholder = blurhashPainter ?: fallbackPainter, - error = blurhashPainter ?: fallbackPainter, - fallback = blurhashPainter ?: fallbackPainter, - modifier = Modifier - .fillMaxWidth() - .then(if (aspectRatio != null) Modifier.aspectRatio(aspectRatio) else Modifier) - .padding(mediaInset) - .clip(mediaShape) - .combinedClickable( - onClick = { onImageClick(message.id) }, - onLongClick = { messageLongClickHandler(message.id) } - ), - contentScale = ContentScale.FillWidth, - onError = { state -> - val cause = state.result.throwable - val isServerError = cause is HttpException && cause.response.code in 500..599 - if ( - isServerError && - !typeContent.previewUrl.isNullOrEmpty() && - retryCount < PREVIEW_MAX_RETRIES && - !retryPending - ) { - retryPending = true - scope.launch { - Log.d( - TAG, - "Preview returned HTTP ${(cause as HttpException).response.code}, " + - "scheduling retry ${retryCount + 1}/$PREVIEW_MAX_RETRIES " + - "for ${typeContent.previewUrl}" - ) - delay(PREVIEW_RETRY_DELAY_MS) - retryCount++ - retryPending = false + val clickableModifier = mediaModifier.combinedClickable( + onClick = { onImageClick(message.id) }, + onLongClick = { messageLongClickHandler(message.id) } + ) + + // Rendered directly instead of routed through Coil's placeholder/fallback painters, + // since Coil's own null-data handling on a pre-built ImageRequest (see load() below) + // takes priority and never shows a composable-supplied fallback painter here. + if (localVideoFramePainter != null) { + Image( + painter = localVideoFramePainter, + contentDescription = stringResource(R.string.media_message_content_description), + modifier = clickableModifier, + contentScale = ContentScale.FillWidth + ) + } else { + AsyncImage( + model = loadedImage, + contentDescription = stringResource(R.string.media_message_content_description), + placeholder = ownUploadPlaceholder, + error = ownUploadPlaceholder, + fallback = ownUploadPlaceholder, + modifier = clickableModifier, + contentScale = ContentScale.FillWidth, + onError = { state -> + val cause = state.result.throwable + val isServerError = cause is HttpException && cause.response.code in 500..599 + if ( + isServerError && + !typeContent.previewUrl.isNullOrEmpty() && + retryCount < PREVIEW_MAX_RETRIES && + !retryPending + ) { + retryPending = true + scope.launch { + Log.d( + TAG, + "Preview returned HTTP ${(cause as HttpException).response.code}, " + + "scheduling retry ${retryCount + 1}/$PREVIEW_MAX_RETRIES " + + "for ${typeContent.previewUrl}" + ) + delay(PREVIEW_RETRY_DELAY_MS) + retryCount++ + retryPending = false + } } } - } - ) + ) + } if (showPlayButton) { - Icon( - painter = painterResource(R.drawable.ic_baseline_play_arrow_voice_message_24), - contentDescription = stringResource(R.string.media_message_content_play), + Box( modifier = Modifier .align(Alignment.Center) - .size(48.dp), - tint = Color.White - ) + .size(playButtonCircleSize) + .clip(CircleShape) + .background(Color.Black.copy(alpha = PLAY_BUTTON_CIRCLE_ALPHA)), + contentAlignment = Alignment.Center + ) { + Icon( + painter = painterResource(R.drawable.ic_baseline_play_arrow_voice_message_24), + contentDescription = stringResource(R.string.media_message_content_play), + modifier = Modifier.size(playButtonIconSize), + tint = Color.White + ) + } } if (chatViewDownloadingFileState.contains(fileParameters.id)) { @@ -239,6 +341,7 @@ fun UploadingMediaMessage( val progress = getProgress(message.referenceId.orEmpty()) val isFailed = message.statusIcon == MessageStatusIcon.FAILED val isSent = message.statusIcon == MessageStatusIcon.SENT + val hasCaption = typeContent.caption != null val mediaInset = 4.dp val mediaShape = remember(message.incoming) { @@ -251,14 +354,16 @@ fun UploadingMediaMessage( conversationThreadId = conversationThreadId, includePadding = false, captionText = typeContent.caption, + forceTimeOverlay = !hasCaption, content = { Column(modifier = Modifier.fillMaxWidth()) { Box(modifier = Modifier.fillMaxWidth()) { - val isImage = typeContent.mimeType?.startsWith("image") == true + val isImage = typeContent.mimeType?.startsWith(Mimetype.IMAGE_PREFIX) == true + val isVideo = typeContent.mimeType?.startsWith(Mimetype.VIDEO_PREFIX) == true if (isImage && typeContent.localFileUri.isNotEmpty()) { AsyncImage( model = typeContent.localFileUri.toUri(), - contentDescription = typeContent.caption, + contentDescription = typeContent.fileName, modifier = Modifier .fillMaxWidth() .blur(4.dp) @@ -266,10 +371,17 @@ fun UploadingMediaMessage( .clip(mediaShape), contentScale = ContentScale.FillWidth ) + } else if (isVideo && typeContent.localFileUri.isNotEmpty()) { + UploadingVideoPreview( + typeContent = typeContent, + referenceId = message.referenceId, + mediaInset = mediaInset, + mediaShape = mediaShape + ) } else { Icon( painter = painterResource(typeContent.drawableResourceId), - contentDescription = typeContent.caption, + contentDescription = typeContent.fileName, modifier = Modifier .size(64.dp) .padding(mediaInset) @@ -286,15 +398,42 @@ fun UploadingMediaMessage( .size(24.dp) ) } else if (!isFailed) { - IconButton( - onClick = { onCancelUpload(message.referenceId.orEmpty()) }, - modifier = Modifier.align(Alignment.TopEnd) + Box( + modifier = Modifier + .matchParentSize() + .background(Color.Black.copy(alpha = UPLOAD_SCRIM_ALPHA)) + ) + Box( + modifier = Modifier + .align(Alignment.Center) + .size(uploadSpinnerSize) ) { - Icon( - imageVector = Icons.Default.Close, - contentDescription = stringResource(R.string.nc_cancel), - tint = Color.White - ) + if (progress != null) { + CircularProgressIndicator( + progress = { progress / 100f }, + modifier = Modifier.fillMaxSize(), + color = Color.White, + trackColor = Color.White.copy(alpha = UPLOAD_SPINNER_TRACK_ALPHA), + strokeWidth = uploadSpinnerStrokeWidth + ) + } else { + CircularProgressIndicator( + modifier = Modifier.fillMaxSize(), + color = Color.White, + trackColor = Color.White.copy(alpha = UPLOAD_SPINNER_TRACK_ALPHA), + strokeWidth = uploadSpinnerStrokeWidth + ) + } + IconButton( + onClick = { onCancelUpload(message.referenceId.orEmpty()) }, + modifier = Modifier.align(Alignment.Center) + ) { + Icon( + imageVector = Icons.Default.Close, + contentDescription = stringResource(R.string.nc_cancel), + tint = Color.White + ) + } } } } @@ -305,27 +444,74 @@ fun UploadingMediaMessage( modifier = Modifier.padding(horizontal = 8.dp, vertical = 2.dp), color = androidx.compose.ui.graphics.Color.Red ) - } else if (!isSent) { - if (progress != null) { - LinearProgressIndicator( - progress = { progress / 100f }, - modifier = Modifier - .fillMaxWidth() - .padding(horizontal = 8.dp, vertical = 4.dp) - ) - } else { - LinearProgressIndicator( - modifier = Modifier - .fillMaxWidth() - .padding(horizontal = 8.dp, vertical = 4.dp) - ) - } } } } ) } +/** + * Sized to the video's real aspect ratio (read locally from the file being uploaded, so it matches + * what the final sent message will look like) with its first frame as a blurred thumbnail. Falls + * back to a fixed 16:9 box with the generic file icon while that's being read, or if it can't be + * read at all. + */ +@Composable +private fun UploadingVideoPreview( + typeContent: MessageTypeContent.UploadingMedia, + referenceId: String?, + mediaInset: Dp, + mediaShape: RoundedCornerShape +) { + val context = LocalContext.current + val videoDescription by produceState( + initialValue = null, + key1 = typeContent.localFileUri + ) { + value = withContext(Dispatchers.IO) { + describeFile(context, typeContent.localFileUri, compress = false).also { description -> + description.videoThumbnail?.let { bitmap -> + referenceId?.let { VideoThumbnailCache.put(context, it, bitmap) } + } + } + } + } + val aspectRatio = videoDescription?.aspectRatio ?: DEFAULT_VIDEO_ASPECT_RATIO + val thumbnail = videoDescription?.videoThumbnail + + if (thumbnail != null) { + Image( + bitmap = thumbnail.asImageBitmap(), + contentDescription = typeContent.fileName, + modifier = Modifier + .fillMaxWidth() + .aspectRatio(aspectRatio) + .blur(4.dp) + .padding(mediaInset) + .clip(mediaShape), + contentScale = ContentScale.Crop + ) + } else { + Box( + modifier = Modifier + .fillMaxWidth() + .aspectRatio(aspectRatio) + .padding(mediaInset) + .clip(mediaShape) + .background(Color.Black.copy(alpha = VIDEO_PLACEHOLDER_BACKGROUND_ALPHA)) + ) { + Icon( + painter = painterResource(typeContent.drawableResourceId), + contentDescription = typeContent.fileName, + modifier = Modifier + .size(64.dp) + .align(Alignment.Center), + tint = Color.Unspecified + ) + } + } +} + fun shape(incoming: Boolean): RoundedCornerShape = if (incoming) { RoundedCornerShape( @@ -346,7 +532,8 @@ fun shape(incoming: Boolean): RoundedCornerShape = private fun previewUploadingContent(mimeType: String? = "image/jpeg") = MessageTypeContent.UploadingMedia( localFileUri = "", - caption = "photo.jpg", + fileName = "photo.jpg", + caption = null, mimeType = mimeType, drawableResourceId = R.drawable.ic_mimetype_image ) @@ -426,7 +613,8 @@ private fun UploadingMediaMessageNonImagePreview() { UploadingMediaMessage( typeContent = MessageTypeContent.UploadingMedia( localFileUri = "", - caption = "document.pdf", + fileName = "document.pdf", + caption = null, mimeType = "application/pdf", drawableResourceId = R.drawable.ic_mimetype_application_pdf ), diff --git a/app/src/main/java/com/nextcloud/talk/upload/chunked/ChunkedFileUploader.kt b/app/src/main/java/com/nextcloud/talk/upload/chunked/ChunkedFileUploader.kt index 7e8b2b1e347..48475401401 100644 --- a/app/src/main/java/com/nextcloud/talk/upload/chunked/ChunkedFileUploader.kt +++ b/app/src/main/java/com/nextcloud/talk/upload/chunked/ChunkedFileUploader.kt @@ -31,7 +31,6 @@ import com.nextcloud.talk.filebrowser.models.properties.NCPreview import com.nextcloud.talk.filebrowser.models.properties.OCFavorite import com.nextcloud.talk.filebrowser.models.properties.OCId import com.nextcloud.talk.filebrowser.models.properties.OCSize -import com.nextcloud.talk.jobs.ShareOperationWorker import com.nextcloud.talk.remotefilebrowser.model.RemoteFileBrowserItem import com.nextcloud.talk.utils.ApiUtils import com.nextcloud.talk.utils.FileUtils @@ -51,11 +50,8 @@ import java.util.Locale class ChunkedFileUploader( okHttpClient: OkHttpClient, val currentUser: User, - val roomToken: String, - val metaData: String?, val listener: OnDataTransferProgressListener, - val ncApiCoroutines: NcApiCoroutines, - val supportsConversationFolders: Boolean + val ncApiCoroutines: NcApiCoroutines ) { private var okHttpClientNoRedirects: OkHttpClient? = null @@ -95,7 +91,7 @@ class ChunkedFileUploader( } if (isUploadSuccessful) { - assembleChunks(uploadFolderUri, targetPath, supportsConversationFolders) + assembleChunks(uploadFolderUri, targetPath) } return isUploadSuccessful } catch (e: Exception) { @@ -298,7 +294,7 @@ class ChunkedFileUploader( this.okHttpClientNoRedirects = builder.build() } - private fun assembleChunks(uploadFolderUri: String, targetPath: String, useConversationSubfolders: Boolean) { + private fun assembleChunks(uploadFolderUri: String, targetPath: String) { val destinationUri = ApiUtils.getUrlForFileUpload( currentUser.baseUrl!!, currentUser.userId!!, @@ -315,16 +311,7 @@ class ChunkedFileUploader( destinationUri.toHttpUrlOrNull()!!, true ) { response: Response -> - if (response.isSuccessful) { - if (!useConversationSubfolders) { - ShareOperationWorker.shareFile( - roomToken, - currentUser, - targetPath, - metaData - ) - } - } else { + if (!response.isSuccessful) { throw IOException("Failed to assemble chunks. response code: " + response.code) } } diff --git a/app/src/main/java/com/nextcloud/talk/utils/VideoThumbnailCache.kt b/app/src/main/java/com/nextcloud/talk/utils/VideoThumbnailCache.kt new file mode 100644 index 00000000000..b9cc9d0a644 --- /dev/null +++ b/app/src/main/java/com/nextcloud/talk/utils/VideoThumbnailCache.kt @@ -0,0 +1,49 @@ +/* + * Nextcloud Talk - Android Client + * + * SPDX-FileCopyrightText: 2017-2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: GPL-3.0-or-later + */ +package com.nextcloud.talk.utils + +import android.content.Context +import android.graphics.Bitmap +import android.graphics.BitmapFactory +import android.util.Log +import java.io.File +import java.io.FileOutputStream + +/** + * Disk cache for locally-extracted video first frames, keyed by the message's referenceId. + * + * Used when a video's server-side preview is unavailable: the frame extracted from the local file + * right after upload would otherwise only live in memory for the current chat session, disappearing + * as soon as the chat is left and reopened. Persisting it here lets that fallback survive across + * chat sessions (and app restarts) without needing the original local file anymore. + */ +object VideoThumbnailCache { + private val TAG = VideoThumbnailCache::class.simpleName + private const val CACHE_DIR_NAME = "video_thumbnails" + private const val JPEG_QUALITY = 80 + + private fun cacheDir(context: Context): File = + File(context.cacheDir, CACHE_DIR_NAME) + .apply { mkdirs() } + + fun get(context: Context, referenceId: String): Bitmap? { + val file = File(cacheDir(context), "$referenceId.jpg") + if (!file.exists()) return null + return BitmapFactory.decodeFile(file.absolutePath) + } + + @Suppress("TooGenericExceptionCaught") + fun put(context: Context, referenceId: String, bitmap: Bitmap) { + try { + FileOutputStream(File(cacheDir(context), "$referenceId.jpg")).use { out -> + bitmap.compress(Bitmap.CompressFormat.JPEG, JPEG_QUALITY, out) + } + } catch (e: Exception) { + Log.w(TAG, "Failed to cache video thumbnail for referenceId=$referenceId", e) + } + } +} From ad108a2ba4d28e71fe918af4424954ac4e991f5e Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Mon, 17 Aug 2026 16:30:33 +0200 Subject: [PATCH 04/26] set negative placeholderId (not tested) Assisted-by: Claude:claude-sonnet-5 Signed-off-by: Marcel Hibbe --- .../talk/chat/data/network/OfflineFirstChatRepository.kt | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt b/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt index 5dc973702de..0727e980bb3 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt @@ -697,15 +697,14 @@ class OfflineFirstChatRepository @Inject constructor( try { val currentTimeMillis = System.currentTimeMillis() - // Use the first 15 hex chars so the value always fits in a signed Long. // Use referenceId.hashCode() as the placeholder id so that: // 1. It is unique per file even when multiple files are selected simultaneously // 2. It fits in an Int, so it survives the Long→Int cast in ChatMessageUi.id without // truncation, keeping DB lookups consistent when the message is tapped. - // 3. It is always positive, because getMessagesEqualOrNewerThan expects it to be larger - // than oldestMessageId + // 3. It is always negative -> sending the lastReadMessage to server checks "-1 < messageId" + // to avoid temporary/placeholder messages (see createChatMessageEntity) @Suppress("MagicNumber") - val placeholderId = (referenceId.hashCode().toLong() and 0x7FFF_FFFFL) + val placeholderId = -(referenceId.hashCode().toLong() and 0x7FFF_FFFFL) Log.d( TAG, From 6e607b4395dd2953cd37e77aba46db10e472e4b8 Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Wed, 19 Aug 2026 20:15:47 +0200 Subject: [PATCH 05/26] fix: cancelling an upload could still share the file Cancel relied solely on WorkManager's isStopped, but cancelUniqueWork() dispatches asynchronously through WorkManager's own executor and Room DB - a small/compressed image could finish uploading and get shared to the room before that cancellation ever became visible to doWork(). Track cancelled referenceIds in an in-memory set instead, set synchronously by the UI before cancelUniqueWork() is called, so the worker can see it immediately. Also close a second gap: once the upload succeeded, the local placeholder stayed flagged as temporary indefinitely, so a "too late" cancel could still delete it locally - making an already-sent message disappear from the UI until the next sync brought it back. Guard the placeholder deletion to only fire while the message is still PENDING. Assisted-by: Claude:claude-sonnet-5 Signed-off-by: Marcel Hibbe --- .../talk/chat/data/ChatMessageRepository.kt | 7 +- .../network/OfflineFirstChatRepository.kt | 5 +- .../talk/chat/viewmodels/ChatViewModel.kt | 2 +- .../talk/data/database/dao/ChatMessagesDao.kt | 17 ++++ .../talk/jobs/UploadAndShareFilesWorker.kt | 88 +++++++++++++++++-- .../utils/preview/ComposePreviewUtilsDaos.kt | 2 + 6 files changed, 110 insertions(+), 11 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/chat/data/ChatMessageRepository.kt b/app/src/main/java/com/nextcloud/talk/chat/data/ChatMessageRepository.kt index 7fc3d13c983..c50c84e8aad 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/data/ChatMessageRepository.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/data/ChatMessageRepository.kt @@ -160,7 +160,12 @@ interface ChatMessageRepository : LifecycleAwareManager { referenceId: String ): Flow> - suspend fun deleteTempMessageByReferenceId(referenceId: String) + /** + * Deletes the local upload placeholder for [referenceId], but only while it's still PENDING. + * Returns false without deleting anything if the upload already finished (and was shared) by + * the time this is called, so a late cancel can't hide an already-sent message. + */ + suspend fun deleteTempMessageByReferenceId(referenceId: String): Boolean suspend fun editChatMessage(credentials: String, url: String, text: String): Flow> diff --git a/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt b/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt index 0727e980bb3..1a077fa36e9 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt @@ -755,9 +755,8 @@ class OfflineFirstChatRepository @Inject constructor( } } - override suspend fun deleteTempMessageByReferenceId(referenceId: String) { - chatDao.deleteTempChatMessages(internalConversationId, listOf(referenceId)) - } + override suspend fun deleteTempMessageByReferenceId(referenceId: String): Boolean = + chatDao.deleteTempChatMessageIfPending(internalConversationId, referenceId) > 0 @Suppress("Detekt.TooGenericExceptionCaught") override suspend fun editChatMessage( diff --git a/app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt b/app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt index 4c90d09947a..992144d286f 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt @@ -235,7 +235,7 @@ class ChatViewModel @AssistedInject constructor( fun cancelUpload(referenceId: String) { val fileUri = uploadReferenceToUri.remove(referenceId) ?: return - WorkManager.getInstance(NextcloudTalkApplication.sharedApplication!!).cancelUniqueWork(fileUri) + UploadAndShareFilesWorker.cancelUpload(referenceId, fileUri) viewModelScope.launch { chatRepository.deleteTempMessageByReferenceId(referenceId) } diff --git a/app/src/main/java/com/nextcloud/talk/data/database/dao/ChatMessagesDao.kt b/app/src/main/java/com/nextcloud/talk/data/database/dao/ChatMessagesDao.kt index 95c1fe48e85..352c54f2f65 100644 --- a/app/src/main/java/com/nextcloud/talk/data/database/dao/ChatMessagesDao.kt +++ b/app/src/main/java/com/nextcloud/talk/data/database/dao/ChatMessagesDao.kt @@ -177,6 +177,23 @@ interface ChatMessagesDao { ) fun deleteTempChatMessages(internalConversationId: String, referenceIds: List) + /* + * Used when the user cancels an upload - unlike deleteTempChatMessages, this must NOT delete + * the placeholder once sendStatus has already moved past PENDING (i.e. the upload already + * finished and was shared), since by then the message has actually been sent and deleting the + * local placeholder would only hide it until the next sync brings it back. + */ + @Query( + value = """ + DELETE FROM ChatMessages + WHERE internalConversationId = :internalConversationId + AND referenceId = :referenceId + AND isTemporary = 1 + AND sendStatus = 'PENDING' + """ + ) + fun deleteTempChatMessageIfPending(internalConversationId: String, referenceId: String): Int + @Update fun updateChatMessage(message: ChatMessageEntity) diff --git a/app/src/main/java/com/nextcloud/talk/jobs/UploadAndShareFilesWorker.kt b/app/src/main/java/com/nextcloud/talk/jobs/UploadAndShareFilesWorker.kt index 222e4c9fd18..f759ce4bc74 100644 --- a/app/src/main/java/com/nextcloud/talk/jobs/UploadAndShareFilesWorker.kt +++ b/app/src/main/java/com/nextcloud/talk/jobs/UploadAndShareFilesWorker.kt @@ -53,6 +53,8 @@ import com.nextcloud.talk.utils.VideoCompressor import com.nextcloud.talk.utils.database.user.CurrentUserProviderOld import com.nextcloud.talk.utils.permissions.PlatformPermissionUtil import com.nextcloud.talk.utils.preferences.AppPreferences +import io.reactivex.Observable +import io.reactivex.disposables.Disposable import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.MutableSharedFlow import kotlinx.coroutines.flow.SharedFlow @@ -62,7 +64,10 @@ import okhttp3.MediaType.Companion.toMediaTypeOrNull import okhttp3.OkHttpClient import java.io.File import java.io.IOException +import java.util.Collections import java.util.UUID +import java.util.concurrent.ConcurrentHashMap +import java.util.concurrent.CountDownLatch import java.util.concurrent.TimeUnit import javax.inject.Inject @@ -107,11 +112,35 @@ class UploadAndShareFilesWorker(val context: Context, workerParameters: WorkerPa private var chunkedFileUploader: ChunkedFileUploader? = null private var referenceId: String? = null private var internalConversationId: String? = null + private var uploadDisposable: Disposable? = null + private var uploadLatch: CountDownLatch? = null + + /** + * WorkManager's own `isStopped`/`onStopped()` only becomes true/fires after + * cancelUniqueWork() round-trips through WorkManager's internal executor and Room DB - for a + * small/compressed image the whole upload+share can finish faster than that round-trip, so + * isStopped alone arrives too late. [cancelledReferenceIds] is set synchronously by the UI + * before cancelUniqueWork() is even called, so it's visible to doWork() immediately. + */ + private fun isCancelled(): Boolean = referenceId?.let { cancelledReferenceIds.contains(it) } == true - @Suppress("Detekt.TooGenericExceptionCaught", "Detekt.LongMethod") override fun doWork(): Result { NextcloudTalkApplication.sharedApplication!!.componentApplication.inject(this) + try { + return doUpload() + } finally { + referenceId?.let { cancelledReferenceIds.remove(it) } + } + } + + @Suppress( + "Detekt.TooGenericExceptionCaught", + "Detekt.LongMethod", + "Detekt.CyclomaticComplexMethod", + "Detekt.ReturnCount" + ) + private fun doUpload(): Result { return try { currentUser = currentUserProvider.currentUser.blockingGet() val sourceFile = inputData.getString(DEVICE_SOURCE_FILE) @@ -149,6 +178,11 @@ class UploadAndShareFilesWorker(val context: Context, workerParameters: WorkerPa useConversationSubfolders = useConversationSubfolders ) + if (uploadSuccess && (isStopped || isCancelled())) { + // Cancelled right as the upload finished - don't share a cancelled upload. + return Result.failure() + } + if (uploadSuccess) { // useConversationSubfolders already shares as part of uploadFile() via // postConversationAttachment, so only share explicitly for the plain upload path. @@ -160,7 +194,7 @@ class UploadAndShareFilesWorker(val context: Context, workerParameters: WorkerPa } Log.e(TAG, "Share operation failed after upload") return failUpload() - } else if (isStopped) { + } else if (isStopped || isCancelled()) { // since work is cancelled the result would be ignored anyways return Result.failure() } @@ -176,7 +210,9 @@ class UploadAndShareFilesWorker(val context: Context, workerParameters: WorkerPa "Network error while uploading file (attempt ${runAttemptCount + 1}/$MAX_UPLOAD_ATTEMPTS)", e ) - if (runAttemptCount < MAX_UPLOAD_ATTEMPTS - 1) { + if (isStopped || isCancelled()) { + Result.failure() + } else if (runAttemptCount < MAX_UPLOAD_ATTEMPTS - 1) { Result.retry() } else { failUpload() @@ -235,7 +271,7 @@ class UploadAndShareFilesWorker(val context: Context, workerParameters: WorkerPa chunkedFileUploader!!.upload(file!!, mimeType, remotePath) } else { Log.d(TAG, "starting normal upload (not chunked) of $fileName") - FileUploader( + val observable = FileUploader( okHttpClient, context, currentUser, @@ -245,9 +281,37 @@ class UploadAndShareFilesWorker(val context: Context, workerParameters: WorkerPa ncApiCoroutines ) .upload(sourceFileUri, fileName, remotePath, null) - .blockingFirst() + blockingUpload(observable) } + // unlike .blockingFirst(), keeps a Disposable so onStopped() can cancel the underlying OkHttp call + private fun blockingUpload(observable: Observable): Boolean { + val latch = CountDownLatch(1) + uploadLatch = latch + var result = false + var error: Throwable? = null + uploadDisposable = observable.subscribe( + { success -> + result = success + latch.countDown() + }, + { throwable -> + error = throwable + latch.countDown() + }, + { latch.countDown() } + ) + latch.await() + uploadDisposable = null + uploadLatch = null + + if (isStopped || isCancelled()) { + return false + } + error?.let { throw it } + return result + } + private fun uploadUsingConversationSubfolders(sourceFileUri: Uri, metaData: String?): Boolean = runBlocking { val credentials = ApiUtils.getCredentials( @@ -287,7 +351,7 @@ class UploadAndShareFilesWorker(val context: Context, workerParameters: WorkerPa .uploadToConversationSubfolder(sourceFileUri, tempRemotePath) } - if (!uploadSuccess) { + if (!uploadSuccess || isStopped || isCancelled()) { return@runBlocking false } @@ -352,6 +416,8 @@ class UploadAndShareFilesWorker(val context: Context, workerParameters: WorkerPa if (file != null && isChunkedUploading) { chunkedFileUploader?.abortUpload {} } + uploadDisposable?.dispose() + uploadLatch?.countDown() super.onStopped() } @@ -399,6 +465,11 @@ class UploadAndShareFilesWorker(val context: Context, workerParameters: WorkerPa private const val MAX_UPLOAD_ATTEMPTS = 4 const val REQUEST_PERMISSION = 3123 + // referenceIds the user cancelled - set synchronously here, before cancelUniqueWork() is + // even called, so doWork() can see it immediately instead of waiting for isStopped, which + // only becomes true once WorkManager's own async cancellation dispatch completes. + private val cancelledReferenceIds: MutableSet = Collections.newSetFromMap(ConcurrentHashMap()) + private val _uploadCompletedFlow: MutableSharedFlow = MutableSharedFlow( replay = 1, extraBufferCapacity = 1 @@ -479,5 +550,10 @@ class UploadAndShareFilesWorker(val context: Context, workerParameters: WorkerPa WorkManager.getInstance().enqueueUniqueWork(fileUri, ExistingWorkPolicy.KEEP, uploadWorker) return uploadWorker.id } + + fun cancelUpload(referenceId: String, fileUri: String) { + cancelledReferenceIds.add(referenceId) + WorkManager.getInstance().cancelUniqueWork(fileUri) + } } } diff --git a/app/src/main/java/com/nextcloud/talk/utils/preview/ComposePreviewUtilsDaos.kt b/app/src/main/java/com/nextcloud/talk/utils/preview/ComposePreviewUtilsDaos.kt index d13ac5ca901..842d4bebbcb 100644 --- a/app/src/main/java/com/nextcloud/talk/utils/preview/ComposePreviewUtilsDaos.kt +++ b/app/src/main/java/com/nextcloud/talk/utils/preview/ComposePreviewUtilsDaos.kt @@ -84,6 +84,8 @@ class DummyChatMessagesDaoImpl : ChatMessagesDao { /* */ } + override fun deleteTempChatMessageIfPending(internalConversationId: String, referenceId: String): Int = 0 + override fun updateChatMessage(message: ChatMessageEntity) { /* */ } From 6e5596a1cfb2513dd69d4bd5349783a099c6ab21 Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Wed, 19 Aug 2026 23:00:54 +0200 Subject: [PATCH 06/26] backoff handling for fetchNewMessagesWithRetry Signed-off-by: Marcel Hibbe --- .../talk/chat/viewmodels/ChatViewModel.kt | 47 +++++++++++-------- 1 file changed, 27 insertions(+), 20 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt b/app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt index 992144d286f..6f1360ebade 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt @@ -30,8 +30,7 @@ import com.nextcloud.talk.chat.data.network.ChatNetworkDataSource import com.nextcloud.talk.chat.ui.model.ChatMessageUi import com.nextcloud.talk.chat.ui.model.MessageTypeContent import com.nextcloud.talk.chat.ui.model.toUiModel -import com.nextcloud.talk.chat.viewmodels.ChatViewModel.Companion.POST_UPLOAD_FETCH_MAX_ATTEMPTS -import com.nextcloud.talk.chat.viewmodels.ChatViewModel.Companion.POST_UPLOAD_FETCH_RETRY_DELAY_MS +import com.nextcloud.talk.chat.viewmodels.ChatViewModel.Companion.POST_UPLOAD_FETCH_RETRY_DELAYS_MS import com.nextcloud.talk.conversationlist.DirectShareHelper import com.nextcloud.talk.conversationlist.data.OfflineConversationsRepository import com.nextcloud.talk.conversationlist.data.network.OfflineFirstConversationsRepository @@ -1358,29 +1357,34 @@ class ChatViewModel @AssistedInject constructor( } /** - * Retries [ChatMessageRepository.fetchNewMessages] up to [POST_UPLOAD_FETCH_MAX_ATTEMPTS] times, - * waiting [POST_UPLOAD_FETCH_RETRY_DELAY_MS] ms between attempts. Stops as soon as at least one - * new message is received so that the happy-path (server responds quickly) has no unnecessary - * delay, while a slow server still gets a few extra chances before we fall back to the regular - * insurance-request cycle. + * Retries [ChatMessageRepository.fetchNewMessages], waiting [POST_UPLOAD_FETCH_RETRY_DELAYS_MS] + * between attempts (one attempt per delay, plus the initial immediate one). Stops as soon as at + * least one new message is received so that the happy-path (server responds quickly) has no + * unnecessary delay, while a slow/indexing-lagged server still gets ~20s of extra chances + * (increasing backoff) before we fall back to the regular insurance-request cycle - which can + * otherwise take up to 2 minutes to tick again, leaving the just-sent message stuck showing its + * "sent, not yet confirmed" spinner in the meantime. */ private suspend fun fetchNewMessagesWithRetry() { - repeat(POST_UPLOAD_FETCH_MAX_ATTEMPTS) { attempt -> - if (attempt > 0) { - Log.d( - TAG, - "fetchNewMessagesWithRetry: attempt ${attempt + 1}, " + - "waiting ${POST_UPLOAD_FETCH_RETRY_DELAY_MS}ms" - ) - delay(POST_UPLOAD_FETCH_RETRY_DELAY_MS) - } + val gotFirst = chatRepository.fetchNewMessages() + if (gotFirst) { + Log.d(TAG, "fetchNewMessagesWithRetry: new messages received on initial attempt") + return + } + POST_UPLOAD_FETCH_RETRY_DELAYS_MS.forEachIndexed { index, delayMs -> + Log.d(TAG, "fetchNewMessagesWithRetry: attempt ${index + 2}, waiting ${delayMs}ms") + delay(delayMs) val gotMessages = chatRepository.fetchNewMessages() if (gotMessages) { - Log.d(TAG, "fetchNewMessagesWithRetry: new messages received on attempt ${attempt + 1}") + Log.d(TAG, "fetchNewMessagesWithRetry: new messages received on attempt ${index + 2}") return } } - Log.d(TAG, "fetchNewMessagesWithRetry: no new messages after $POST_UPLOAD_FETCH_MAX_ATTEMPTS attempts") + Log.w( + TAG, + "fetchNewMessagesWithRetry: no new messages after " + + "${POST_UPLOAD_FETCH_RETRY_DELAYS_MS.size + 1} attempts, deferring to the insurance-request cycle" + ) } private fun handleSystemMessages(chatMessageList: List, isChannel: Boolean): List { if (isChannel) { @@ -2428,8 +2432,11 @@ class ChatViewModel @AssistedInject constructor( private const val MIN_CHARS_FOR_SEARCH = 2 private const val CONTEXT_MESSAGES_LIMIT = 50 private const val LOAD_MORE_MESSAGES_LIMIT = 100 - private const val POST_UPLOAD_FETCH_MAX_ATTEMPTS = 4 - private const val POST_UPLOAD_FETCH_RETRY_DELAY_MS = 1_500L + + // Increasing backoff for fetchNewMessagesWithRetry() - covers realistic server indexing + // lag (~20s total) so a just-sent upload doesn't sit stuck until the next insurance-request + // cycle (up to 2 minutes later) before its "sent, not yet confirmed" spinner clears. + private val POST_UPLOAD_FETCH_RETRY_DELAYS_MS = listOf(1_000L, 1_500L, 2_000L, 3_000L, 4_000L, 5_000L, 5_000L) private const val LOCAL_PREVIEW_GRACE_PERIOD_MS = 15_000L private const val PLAUSIBLE_MESSAGE_ID_BUFFER = 10_000L From 3126020a66c488097569a4d379e6a07493ea709a Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Wed, 19 Aug 2026 23:22:12 +0200 Subject: [PATCH 07/26] remove right corner loading spinner Signed-off-by: Marcel Hibbe --- .../main/java/com/nextcloud/talk/ui/chat/MediaMessage.kt | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/ui/chat/MediaMessage.kt b/app/src/main/java/com/nextcloud/talk/ui/chat/MediaMessage.kt index f6ac151e283..1e1c66b8cb1 100644 --- a/app/src/main/java/com/nextcloud/talk/ui/chat/MediaMessage.kt +++ b/app/src/main/java/com/nextcloud/talk/ui/chat/MediaMessage.kt @@ -390,14 +390,7 @@ fun UploadingMediaMessage( ) } - if (isSent) { - CircularProgressIndicator( - modifier = Modifier - .align(Alignment.TopEnd) - .padding(8.dp) - .size(24.dp) - ) - } else if (!isFailed) { + if (!isSent && !isFailed) { Box( modifier = Modifier .matchParentSize() From 1b71ecb0418fde4eba5e92506a472d3c8ea1f0c8 Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Thu, 20 Aug 2026 00:03:10 +0200 Subject: [PATCH 08/26] fix: media message flicker, padding/corner mismatch, and unread-marker bug - uploadFile() never called onMessageSent(), so the unread-marker cache could latch onto the sender's own message once the upload's temp placeholder synced to its real (higher) message id - it wasn't excluded the way a regular text send already was. - The real preview replacing the upload placeholder could flash instead of fade: Coil's built-in crossfade doesn't reliably animate from a plain Compose Painter placeholder (as opposed to a Coil-managed Drawable). Replaced with an explicit alpha crossfade so the transition is fully controlled by Compose. - ContentScale.FillWidth left an uneven letterboxing gap whenever the server-reported aspect ratio didn't exactly match the loaded preview's own (confirmed via pixel-level screenshot measurement on a real device) - switched to ContentScale.Crop, and corrected the clip radius to 6dp (bubble's 10dp minus the 4dp inset) so the two corners nest concentrically instead of visibly mismatching. - The media clip shape used a fixed corner pattern regardless of the message's grouping state, unlike the bubble itself - shape() now mirrors the bubble's own per-corner grouping logic (groupedSideTop/groupedSideBottom), with the now-degenerate grouped corner rounded to a small 2dp instead of a jarring hard edge. Assisted-by: Claude:claude-sonnet-5 Signed-off-by: Marcel Hibbe --- .../talk/chat/viewmodels/ChatViewModel.kt | 12 ++- .../nextcloud/talk/contacts/ImageRequest.kt | 1 + .../nextcloud/talk/ui/chat/MediaMessage.kt | 87 ++++++++++++++----- 3 files changed, 78 insertions(+), 22 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt b/app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt index 6f1360ebade..6b1af5d000b 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt @@ -2042,6 +2042,11 @@ class ChatViewModel @AssistedInject constructor( return } + // Same as a regular text send: once the user has sent something themselves, the unread + // marker must never latch onto it once it syncs back with its real (higher) message id - + // otherwise the marker can end up sitting right above the file the user just sent. + onMessageSent() + if (replyToMessageId != 0) { metaDataMap["replyTo"] = replyToMessageId.toString() } @@ -2490,7 +2495,12 @@ class ChatViewModel @AssistedInject constructor( fun stableKey(): Any = when (this) { - is MessageItem -> "msg_${uiMessage.id}" + // Prefer referenceId when present: it survives the swap from the local upload + // placeholder (negative placeholderId) to the real synced message (real server id), + // so Compose recomposes the existing list slot in place instead of removing and + // re-inserting a new one - which is what caused the visible flicker/pop. + is MessageItem -> uiMessage.referenceId?.takeIf { it.isNotBlank() }?.let { "msg_ref_$it" } + ?: "msg_${uiMessage.id}" is DateHeaderItem -> "header_$date" is UnreadMessagesMarkerItem -> "last_read_$date" is LoadGapItem -> "load_gap_$anchorMessageId" diff --git a/app/src/main/java/com/nextcloud/talk/contacts/ImageRequest.kt b/app/src/main/java/com/nextcloud/talk/contacts/ImageRequest.kt index 8446159d3a0..e6511ea11f6 100644 --- a/app/src/main/java/com/nextcloud/talk/contacts/ImageRequest.kt +++ b/app/src/main/java/com/nextcloud/talk/contacts/ImageRequest.kt @@ -36,6 +36,7 @@ fun load( .size(Size.ORIGINAL) .error(errorPlaceholderImage) .placeholder(errorPlaceholderImage) + .crossfade(true) if (!animated) { builder.transformations(RoundedCornersTransformation()) } diff --git a/app/src/main/java/com/nextcloud/talk/ui/chat/MediaMessage.kt b/app/src/main/java/com/nextcloud/talk/ui/chat/MediaMessage.kt index 1e1c66b8cb1..e5b53efa143 100644 --- a/app/src/main/java/com/nextcloud/talk/ui/chat/MediaMessage.kt +++ b/app/src/main/java/com/nextcloud/talk/ui/chat/MediaMessage.kt @@ -9,6 +9,8 @@ package com.nextcloud.talk.ui.chat import android.graphics.Bitmap import android.util.Log +import androidx.compose.animation.core.animateFloatAsState +import androidx.compose.animation.core.tween import androidx.compose.foundation.Image import androidx.compose.foundation.background import androidx.compose.foundation.combinedClickable @@ -47,10 +49,12 @@ import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.res.painterResource import androidx.compose.ui.res.stringResource import androidx.compose.runtime.CompositionLocalProvider +import androidx.compose.ui.draw.alpha import androidx.compose.ui.draw.blur import androidx.compose.ui.unit.Dp import androidx.compose.ui.unit.dp import coil.compose.AsyncImage +import coil.compose.AsyncImagePainter import coil.compose.rememberAsyncImagePainter import coil.network.HttpException import com.nextcloud.talk.R @@ -81,9 +85,16 @@ val LocalUploadedLocalPreviewProvider = compositionLocalOf<(referenceId: String) private const val FILE_PLACEHOLDER_MESSAGE = "{file}" private const val PREVIEW_MAX_RETRIES = 3 private const val PREVIEW_RETRY_DELAY_MS = 2_000L +private const val MEDIA_CROSSFADE_DURATION_MS = 300 private const val TAG = "MediaMessage" -private val mediaRadiusBig = 8.dp +// bubbleRadiusBig (ChatMessageScaffold.kt) minus mediaInset below, so the media's own rounded +// corner nests concentrically inside the bubble's corner instead of a visibly mismatched arc. +private val mediaRadiusBig = 6.dp + +// bubbleRadiusSmall (2.dp) minus mediaInset would go negative, so this can't nest concentrically +// like mediaRadiusBig does - a small fixed rounding instead of a hard 0 corner still looks better +// than a perfectly sharp edge sitting inside the bubble's own (slightly rounded) grouped corner. private val mediaRadiusSmall = 2.dp private val uploadSpinnerSize = 56.dp @@ -170,8 +181,8 @@ fun MediaMessage( } val hasCaption = captionText != null val mediaInset = 4.dp - val mediaShape = remember(message.incoming) { - shape(message.incoming) + val mediaShape = remember(message.incoming, message.isGrouped, message.isGroupedWithNext) { + shape(message.incoming, message.isGrouped, message.isGroupedWithNext) } MessageScaffold( @@ -236,7 +247,11 @@ fun MediaMessage( } val fallbackPainter = painterResource(typeContent.drawableResourceId) - val ownUploadPlaceholder = blurhashPainter ?: localPreviewPainter ?: fallbackPainter + // Prefer the local file over blurhash when both are available: blurhash is a rough + // color-blob approximation meant for messages from other users where we don't have + // the actual bytes - for our own just-uploaded file we already have the real image on + // disk, so showing blurhash instead would be a needless downgrade. + val ownUploadPlaceholder = localPreviewPainter ?: blurhashPainter ?: fallbackPainter val mediaModifier = Modifier .fillMaxWidth() @@ -259,17 +274,11 @@ fun MediaMessage( painter = localVideoFramePainter, contentDescription = stringResource(R.string.media_message_content_description), modifier = clickableModifier, - contentScale = ContentScale.FillWidth + contentScale = ContentScale.Crop ) } else { - AsyncImage( + val loadedPainter = rememberAsyncImagePainter( model = loadedImage, - contentDescription = stringResource(R.string.media_message_content_description), - placeholder = ownUploadPlaceholder, - error = ownUploadPlaceholder, - fallback = ownUploadPlaceholder, - modifier = clickableModifier, - contentScale = ContentScale.FillWidth, onError = { state -> val cause = state.result.throwable val isServerError = cause is HttpException && cause.response.code in 500..599 @@ -294,6 +303,35 @@ fun MediaMessage( } } ) + val isLoaded = loadedPainter.state is AsyncImagePainter.State.Success + val loadedAlpha by animateFloatAsState( + targetValue = if (isLoaded) 1f else 0f, + animationSpec = tween(durationMillis = MEDIA_CROSSFADE_DURATION_MS), + label = "mediaLoadedAlpha" + ) + + // Own explicit crossfade instead of relying on Coil's built-in one: the + // placeholder is a Compose-supplied Painter (not a Coil-managed Drawable), so + // Coil's crossfade transition doesn't reliably fade from what's actually on + // screen - it can jump straight to the loaded image, reading as a flash rather + // than a fade. Keeping the placeholder as a permanent base layer and fading the + // loaded image in on top guarantees a smooth, controllable transition instead. + Box(modifier = clickableModifier) { + Image( + painter = ownUploadPlaceholder, + contentDescription = stringResource(R.string.media_message_content_description), + modifier = Modifier.matchParentSize(), + contentScale = ContentScale.Crop + ) + Image( + painter = loadedPainter, + contentDescription = stringResource(R.string.media_message_content_description), + modifier = Modifier + .matchParentSize() + .alpha(loadedAlpha), + contentScale = ContentScale.Crop + ) + } } if (showPlayButton) { @@ -344,8 +382,8 @@ fun UploadingMediaMessage( val hasCaption = typeContent.caption != null val mediaInset = 4.dp - val mediaShape = remember(message.incoming) { - shape(message.incoming) + val mediaShape = remember(message.incoming, message.isGrouped, message.isGroupedWithNext) { + shape(message.incoming, message.isGrouped, message.isGroupedWithNext) } MessageScaffold( @@ -369,7 +407,7 @@ fun UploadingMediaMessage( .blur(4.dp) .padding(mediaInset) .clip(mediaShape), - contentScale = ContentScale.FillWidth + contentScale = ContentScale.Crop ) } else if (isVideo && typeContent.localFileUri.isNotEmpty()) { UploadingVideoPreview( @@ -505,22 +543,29 @@ private fun UploadingVideoPreview( } } -fun shape(incoming: Boolean): RoundedCornerShape = - if (incoming) { +// Mirrors ChatMessageScaffold's own bubble-shape logic (groupedSideTop/groupedSideBottom) exactly, +// so the media's clip always nests inside whichever corner radius the bubble actually rendered for +// this specific message's grouping state - a fixed shape here would only ever match one of the two +// possible bubble shapes and visibly mismatch on the other. +fun shape(incoming: Boolean, isGrouped: Boolean, isGroupedWithNext: Boolean): RoundedCornerShape { + val groupedSideTop = if (isGrouped) mediaRadiusSmall else mediaRadiusBig + val groupedSideBottom = if (isGroupedWithNext) mediaRadiusSmall else mediaRadiusBig + return if (incoming) { RoundedCornerShape( - topStart = mediaRadiusSmall, + topStart = groupedSideTop, topEnd = mediaRadiusBig, bottomEnd = mediaRadiusBig, - bottomStart = mediaRadiusBig + bottomStart = groupedSideBottom ) } else { RoundedCornerShape( topStart = mediaRadiusBig, - topEnd = mediaRadiusSmall, - bottomEnd = mediaRadiusBig, + topEnd = groupedSideTop, + bottomEnd = groupedSideBottom, bottomStart = mediaRadiusBig ) } +} private fun previewUploadingContent(mimeType: String? = "image/jpeg") = MessageTypeContent.UploadingMedia( From fb0df2d7f8f57e6083e475515b9ba7c1233c1abf Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Thu, 20 Aug 2026 10:39:18 +0200 Subject: [PATCH 09/26] feat: shrink portrait media messages to 80% width Portrait (taller-than-wide) photos/videos filling the full bubble width read as oversized in the chat. Landscape media is already reasonably sized at full width, so only portrait media (aspect ratio < 1) is now capped to 75% width via mediaWidthFraction(), applied consistently to the synced message, the local upload placeholder, and the video-preview placeholder so there's no resize when one becomes the other. This required fixing ChatMessageScaffold's OVERLAY metadata layout mode (used only by media messages) to stop forcing Modifier.fillMaxWidth() on its wrapping Box - that Box's width is what the bubble itself wraps to, so forcing it full-width meant the bubble stayed at full size with empty space beside the now-narrower image instead of shrinking to match. OverlayMetadataBadge's alignment is relative to the Box's own bounds, so it's unaffected either way. Assisted-by: Claude:claude-sonnet-5 Signed-off-by: Marcel Hibbe --- .../talk/ui/chat/ChatMessageScaffold.kt | 7 ++- .../nextcloud/talk/ui/chat/MediaMessage.kt | 47 +++++++++++++++---- 2 files changed, 45 insertions(+), 9 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/ui/chat/ChatMessageScaffold.kt b/app/src/main/java/com/nextcloud/talk/ui/chat/ChatMessageScaffold.kt index 08dfe600a90..a1c3bf8fe4b 100644 --- a/app/src/main/java/com/nextcloud/talk/ui/chat/ChatMessageScaffold.kt +++ b/app/src/main/java/com/nextcloud/talk/ui/chat/ChatMessageScaffold.kt @@ -422,7 +422,12 @@ private fun ColumnScope.MessageBodyWithMetadata( } MetadataLayoutMode.OVERLAY -> { - Box(modifier = Modifier.fillMaxWidth()) { + // Not fillMaxWidth(): OVERLAY is media-only, and media's own content already decides + // its width (e.g. shrinking narrower for portrait) - forcing full width here would + // leave the bubble at full size with empty space beside a narrower image instead of + // letting the bubble shrink to match. OverlayMetadataBadge's alignment is relative to + // this Box's own bounds either way, so it still lands correctly on the content's corner. + Box { content() if (!suppressMetadata) { OverlayMetadataBadge(uiMessage = uiMessage) diff --git a/app/src/main/java/com/nextcloud/talk/ui/chat/MediaMessage.kt b/app/src/main/java/com/nextcloud/talk/ui/chat/MediaMessage.kt index e5b53efa143..a15107d28a2 100644 --- a/app/src/main/java/com/nextcloud/talk/ui/chat/MediaMessage.kt +++ b/app/src/main/java/com/nextcloud/talk/ui/chat/MediaMessage.kt @@ -111,6 +111,13 @@ private val playButtonCircleSize = 56.dp private val playButtonIconSize = 32.dp private const val PLAY_BUTTON_CIRCLE_ALPHA = 0.45f +// Portrait media (taller than wide) filling the full bubble width reads as oversized in the chat - +// shrink just that case, since landscape media is already reasonably sized at full width. +private const val PORTRAIT_WIDTH_FRACTION = 0.80f + +private fun mediaWidthFraction(aspectRatio: Float?): Float = + if (aspectRatio != null && aspectRatio < 1f) PORTRAIT_WIDTH_FRACTION else 1f + @Suppress("Detekt.LongMethod", "LongParameterList", "CyclomaticComplexMethod") @Composable fun MediaMessage( @@ -259,7 +266,11 @@ fun MediaMessage( .padding(mediaInset) .clip(mediaShape) - Box(modifier = Modifier.fillMaxWidth()) { + // The bubble wraps to this Box's requested width (ChatMessageScaffold's Surface has + // no fillMaxWidth of its own), so shrinking this - not just mediaModifier - is what + // actually shrinks the bubble along with the media, instead of leaving empty space + // beside a smaller image inside an unchanged-size bubble. + Box(modifier = Modifier.fillMaxWidth(mediaWidthFraction(aspectRatio))) { val messageLongClickHandler = LocalMessageLongClickHandler.current val clickableModifier = mediaModifier.combinedClickable( onClick = { onImageClick(message.id) }, @@ -375,17 +386,33 @@ fun UploadingMediaMessage( conversationThreadId: Long? = null, onCancelUpload: (referenceId: String) -> Unit = {} ) { + val context = LocalContext.current val getProgress = LocalUploadProgressProvider.current val progress = getProgress(message.referenceId.orEmpty()) val isFailed = message.statusIcon == MessageStatusIcon.FAILED val isSent = message.statusIcon == MessageStatusIcon.SENT val hasCaption = typeContent.caption != null + val isImage = typeContent.mimeType?.startsWith(Mimetype.IMAGE_PREFIX) == true + val isVideo = typeContent.mimeType?.startsWith(Mimetype.VIDEO_PREFIX) == true val mediaInset = 4.dp val mediaShape = remember(message.incoming, message.isGrouped, message.isGroupedWithNext) { shape(message.incoming, message.isGrouped, message.isGroupedWithNext) } + // Read locally so the placeholder is already sized the same way the final MediaMessage will be + // (portrait shrunk to mediaWidthFraction) - otherwise the bubble would visibly resize once the + // real message replaces this placeholder. + val imageAspectRatio by produceState(initialValue = null, key1 = typeContent.localFileUri) { + value = if (isImage) { + withContext(Dispatchers.IO) { + describeFile(context, typeContent.localFileUri, compress = false).aspectRatio + } + } else { + null + } + } + MessageScaffold( uiMessage = message, isOneToOneConversation = isOneToOneConversation, @@ -394,16 +421,19 @@ fun UploadingMediaMessage( captionText = typeContent.caption, forceTimeOverlay = !hasCaption, content = { - Column(modifier = Modifier.fillMaxWidth()) { - Box(modifier = Modifier.fillMaxWidth()) { - val isImage = typeContent.mimeType?.startsWith(Mimetype.IMAGE_PREFIX) == true - val isVideo = typeContent.mimeType?.startsWith(Mimetype.VIDEO_PREFIX) == true + // Not fillMaxWidth(): for image/video, whichever content renders below decides the + // width itself (shrinking for portrait), and this wraps to match - forcing full width + // here would leave empty space beside a narrower image instead of shrinking the bubble. + Column { + Box( + modifier = if (isImage || isVideo) Modifier else Modifier.fillMaxWidth() + ) { if (isImage && typeContent.localFileUri.isNotEmpty()) { AsyncImage( model = typeContent.localFileUri.toUri(), contentDescription = typeContent.fileName, modifier = Modifier - .fillMaxWidth() + .fillMaxWidth(mediaWidthFraction(imageAspectRatio)) .blur(4.dp) .padding(mediaInset) .clip(mediaShape), @@ -509,13 +539,14 @@ private fun UploadingVideoPreview( } val aspectRatio = videoDescription?.aspectRatio ?: DEFAULT_VIDEO_ASPECT_RATIO val thumbnail = videoDescription?.videoThumbnail + val widthFraction = mediaWidthFraction(aspectRatio) if (thumbnail != null) { Image( bitmap = thumbnail.asImageBitmap(), contentDescription = typeContent.fileName, modifier = Modifier - .fillMaxWidth() + .fillMaxWidth(widthFraction) .aspectRatio(aspectRatio) .blur(4.dp) .padding(mediaInset) @@ -525,7 +556,7 @@ private fun UploadingVideoPreview( } else { Box( modifier = Modifier - .fillMaxWidth() + .fillMaxWidth(widthFraction) .aspectRatio(aspectRatio) .padding(mediaInset) .clip(mediaShape) From b1645b87deb76ac0f4277d4a0a280446129db0a8 Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Thu, 20 Aug 2026 10:54:13 +0200 Subject: [PATCH 10/26] fix: uploaded files could appear out of the order they were sent in The attachment preview screen already respects the user's drag-to- reorder choice all the way through to the per-file upload calls, but each file was then enqueued as a fully independent unique WorkManager request (enqueueUniqueWork(fileUri, KEEP, ...)). WorkManager gives no ordering guarantee across independent unique work, so a smaller/faster file queued later could finish its upload+share network calls (and so get a server-assigned message position) before an earlier, larger one - silently reordering the messages in chat regardless of what the user picked in the preview screen. Uploads within the same conversation are now chained under one shared unique-work name via ExistingWorkPolicy.APPEND_OR_REPLACE, so they upload and share strictly in enqueue (i.e. send) order. APPEND_OR_REPLACE rather than APPEND so a cancelled/failed upload starts a fresh chain instead of cascade-failing every file queued behind it. Since the unique-work name is now shared across a conversation's uploads rather than per-file, cancellation switches from cancelUniqueWork(fileUri) (which would now cancel the whole queue) to cancelWorkById() using the upload's own WorkRequest id. Assisted-by: Claude:claude-sonnet-5 Signed-off-by: Marcel Hibbe --- .../talk/chat/viewmodels/ChatViewModel.kt | 16 +++++++++----- .../talk/jobs/UploadAndShareFilesWorker.kt | 22 ++++++++++++++++--- 2 files changed, 29 insertions(+), 9 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt b/app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt index 6b1af5d000b..6bac1cd9f0d 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt @@ -229,12 +229,16 @@ class ChatViewModel @AssistedInject constructor( private val _uploadedLocalPreviewMap = MutableStateFlow>(emptyMap()) val uploadedLocalPreviewMap: StateFlow> = _uploadedLocalPreviewMap - // Maps referenceId -> fileUri for cancellation support - private val uploadReferenceToUri = mutableMapOf() + // Maps referenceId -> the upload's own WorkRequest id for cancellation support. Uploads within + // a conversation are now chained under one shared unique-work name (see + // UploadAndShareFilesWorker.upload()) so they run - and therefore get shared to the server - in + // send order, so cancellation has to target this specific request's id rather than that shared + // name, which would otherwise cancel every other queued upload too. + private val uploadReferenceToWorkId = mutableMapOf() fun cancelUpload(referenceId: String) { - val fileUri = uploadReferenceToUri.remove(referenceId) ?: return - UploadAndShareFilesWorker.cancelUpload(referenceId, fileUri) + val workId = uploadReferenceToWorkId.remove(referenceId) ?: return + UploadAndShareFilesWorker.cancelUpload(referenceId, workId) viewModelScope.launch { chatRepository.deleteTempMessageByReferenceId(referenceId) } @@ -2095,7 +2099,7 @@ class ChatViewModel @AssistedInject constructor( ) if (!isVoiceMessage) { - uploadReferenceToUri[referenceId] = fileUri + uploadReferenceToWorkId[referenceId] = workerId _uploadedLocalPreviewMap.update { it + (referenceId to fileUri) } observeUploadProgress(workerId, referenceId) } @@ -2132,7 +2136,7 @@ class ChatViewModel @AssistedInject constructor( } if (workInfo.state.isFinished) { _uploadProgressMap.update { it - referenceId } - uploadReferenceToUri.remove(referenceId) + uploadReferenceToWorkId.remove(referenceId) viewModelScope.launch { delay(LOCAL_PREVIEW_GRACE_PERIOD_MS) _uploadedLocalPreviewMap.update { it - referenceId } diff --git a/app/src/main/java/com/nextcloud/talk/jobs/UploadAndShareFilesWorker.kt b/app/src/main/java/com/nextcloud/talk/jobs/UploadAndShareFilesWorker.kt index f759ce4bc74..e3c2929f954 100644 --- a/app/src/main/java/com/nextcloud/talk/jobs/UploadAndShareFilesWorker.kt +++ b/app/src/main/java/com/nextcloud/talk/jobs/UploadAndShareFilesWorker.kt @@ -547,13 +547,29 @@ class UploadAndShareFilesWorker(val context: Context, workerParameters: WorkerPa TimeUnit.MILLISECONDS ) .build() - WorkManager.getInstance().enqueueUniqueWork(fileUri, ExistingWorkPolicy.KEEP, uploadWorker) + // Chained per conversation (not enqueueUniqueWork(fileUri, ...), which ran every upload + // fully independently) so multiple files - whether from one multi-file share or several + // sends in a row - actually upload and share to the server in the order they were sent, + // instead of each finishing (and so appearing in chat) whenever its own network calls + // happen to complete. APPEND_OR_REPLACE rather than APPEND: if the file ahead in the + // queue was cancelled or failed, this starts a fresh chain instead of cascading that + // failure onto every file queued behind it. + WorkManager.getInstance().enqueueUniqueWork( + uploadQueueName(internalConversationId), + ExistingWorkPolicy.APPEND_OR_REPLACE, + uploadWorker + ) return uploadWorker.id } - fun cancelUpload(referenceId: String, fileUri: String) { + private fun uploadQueueName(internalConversationId: String) = "upload_queue_$internalConversationId" + + // Cancellation is by the WorkRequest's own id (not enqueueUniqueWork's name) since that name + // is now shared by every file queued in the same conversation - cancelling by name would + // cancel the whole queue instead of just this one upload. + fun cancelUpload(referenceId: String, workerId: UUID) { cancelledReferenceIds.add(referenceId) - WorkManager.getInstance().cancelUniqueWork(fileUri) + WorkManager.getInstance().cancelWorkById(workerId) } } } From 22bc7f4b0d47b3652cd75830d4297e47caa81af5 Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Thu, 20 Aug 2026 11:22:01 +0200 Subject: [PATCH 11/26] fix: upload placeholders could show out of send order before syncing Chat message ordering is "timestamp ASC, id ASC" (ChatMessagesDao), but addUploadPlaceholderMessage's timestamp was truncated to whole seconds - near-guaranteed to tie when several files are sent at once - and the tiebreaker id is a hash of a random referenceId, unrelated to call order. So placeholders for a multi-file send could display in an arbitrary (hash-order) sequence instead of the order they were actually sent in, even before any upload/share completes and regardless of the WorkManager send-order fix already in place. nextPlaceholderTimestampSeconds() tracks the last timestamp handed out and never repeats one, so placeholders created within the same wall-clock second still get strictly increasing values matching call order. Self-corrects once each placeholder is replaced by its server-synced message with the real timestamp. Assisted-by: Claude:claude-sonnet-5 Signed-off-by: Marcel Hibbe --- .../network/OfflineFirstChatRepository.kt | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt b/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt index 1a077fa36e9..35ae48ee462 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt @@ -63,6 +63,21 @@ class OfflineFirstChatRepository @Inject constructor( lateinit var currentUser: User + // Placeholder rows sort by "timestamp ASC, id ASC" (ChatMessagesDao) same as real messages, but + // id here is a hash of a random referenceId with no relation to call order - so if several + // files are sent at once and their (second-granularity) timestamps tie, as is near-certain, + // they'd otherwise sort arbitrarily instead of in the order they were actually sent. Tracking + // the last-used value and never handing out the same one twice keeps placeholders strictly in + // call order regardless of how many land within the same wall-clock second. + private var lastPlaceholderTimestampSeconds = 0L + + private fun nextPlaceholderTimestampSeconds(): Long = + synchronized(this) { + val timestamp = maxOf(System.currentTimeMillis() / MILLIES, lastPlaceholderTimestampSeconds + 1) + lastPlaceholderTimestampSeconds = timestamp + timestamp + } + override val messageFlow: Flow< Triple< @@ -695,8 +710,6 @@ class OfflineFirstChatRepository @Inject constructor( ): Flow> = flow { try { - val currentTimeMillis = System.currentTimeMillis() - // Use referenceId.hashCode() as the placeholder id so that: // 1. It is unique per file even when multiple files are selected simultaneously // 2. It fits in an Int, so it survives the Long→Int cast in ChatMessageUi.id without @@ -740,7 +753,7 @@ class OfflineFirstChatRepository @Inject constructor( parentMessageId = null, systemMessageType = ChatMessage.SystemMessageType.DUMMY, replyable = false, - timestamp = currentTimeMillis / MILLIES, + timestamp = nextPlaceholderTimestampSeconds(), expirationTimestamp = 0, actorDisplayName = currentUser.displayName!!, referenceId = referenceId, From 183d3081716e7142cfdd013fa111c35ff7643505 Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Thu, 20 Aug 2026 12:04:53 +0200 Subject: [PATCH 12/26] fix: chat order could flip while some batch-sent files were still uploading Chained per-conversation uploads (a prior fix, for send-order correctness) mean a file can take real seconds to actually finish uploading and sharing once its predecessors in the same batch are ahead of it in the queue. By the time its real message syncs in, the server's own timestamp for it can be later than the still-pending, instantly-assigned placeholder timestamps of files queued after it in the same batch - flipping their relative order in chat (the still-pending files would sort before the one that just finished, even though it was sent first). persistChatMessagesAndHandleSystemMessages now looks up the matching local placeholder (if still present) before persisting each incoming real message, and keeps the placeholder's timestamp instead of the server's if it's earlier - preserving the originally-established send order for the rest of the still-in-flight batch. Needed a one-shot (non-Flow) DAO query since the placeholder must be read before upsertChatMessagesAndDeleteTemp overwrites/removes it in the same transaction. Assisted-by: Claude:claude-sonnet-5 Signed-off-by: Marcel Hibbe --- .../chat/data/network/ChatMessageSyncer.kt | 21 +++++++++++++++++- .../talk/data/database/dao/ChatMessagesDao.kt | 22 +++++++++++++++++++ .../utils/preview/ComposePreviewUtilsDaos.kt | 6 +++++ 3 files changed, 48 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt b/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt index 99566ff5543..81f45358aad 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt @@ -790,7 +790,26 @@ class ChatMessageSyncer @Inject constructor( handleSystemMessagesThatAffectDatabase(target, chatMessages, events) val chatMessageEntities = chatMessages.map { - it.asEntity(target.accountId) + val entity = it.asEntity(target.accountId) + // If this message has a still-present local placeholder, keep the placeholder's + // (earlier) timestamp instead of the server's own. Uploads within a conversation are + // chained sequentially, so a file can take real seconds/minutes to actually finish + // uploading and sharing - by then, its server timestamp can be later than the + // instantly-assigned, still-pending placeholder timestamps of files queued after it in + // the same batch, which would otherwise flip their relative chat order once this one + // syncs in while those are still just placeholders. + val referenceId = entity.referenceId + if (referenceId != null) { + val placeholder = chatDao.getTempMessageForConversationOnce( + target.internalConversationId, + referenceId, + target.threadId + ) + if (placeholder != null && placeholder.timestamp < entity.timestamp) { + entity.timestamp = placeholder.timestamp + } + } + entity } try { diff --git a/app/src/main/java/com/nextcloud/talk/data/database/dao/ChatMessagesDao.kt b/app/src/main/java/com/nextcloud/talk/data/database/dao/ChatMessagesDao.kt index 352c54f2f65..2d4a98ea619 100644 --- a/app/src/main/java/com/nextcloud/talk/data/database/dao/ChatMessagesDao.kt +++ b/app/src/main/java/com/nextcloud/talk/data/database/dao/ChatMessagesDao.kt @@ -95,6 +95,28 @@ interface ChatMessagesDao { threadId: Long? ): Flow + // One-shot (not Flow) variant used when syncing a real message in: its placeholder's original + // timestamp is needed synchronously, before the placeholder itself is overwritten/deleted, to + // decide whether the real message should keep that earlier timestamp (see + // ChatMessageSyncer.persistChatMessagesAndHandleSystemMessages). + @Query( + """ + SELECT * + FROM ChatMessages + WHERE internalConversationId = :internalConversationId + AND referenceId = :referenceId + AND isTemporary = 1 + AND (:threadId IS NULL OR threadId = :threadId) + ORDER BY timestamp DESC, id DESC + LIMIT 1 + """ + ) + suspend fun getTempMessageForConversationOnce( + internalConversationId: String, + referenceId: String, + threadId: Long? + ): ChatMessageEntity? + @Insert(onConflict = OnConflictStrategy.REPLACE) suspend fun upsertChatMessages(chatMessages: List) diff --git a/app/src/main/java/com/nextcloud/talk/utils/preview/ComposePreviewUtilsDaos.kt b/app/src/main/java/com/nextcloud/talk/utils/preview/ComposePreviewUtilsDaos.kt index 842d4bebbcb..da8bbb2d7dc 100644 --- a/app/src/main/java/com/nextcloud/talk/utils/preview/ComposePreviewUtilsDaos.kt +++ b/app/src/main/java/com/nextcloud/talk/utils/preview/ComposePreviewUtilsDaos.kt @@ -51,6 +51,12 @@ class DummyChatMessagesDaoImpl : ChatMessagesDao { threadId: Long? ): Flow = flowOf() + override suspend fun getTempMessageForConversationOnce( + internalConversationId: String, + referenceId: String, + threadId: Long? + ): ChatMessageEntity? = null + override suspend fun upsertChatMessages(chatMessages: List) { /* */ } From 49c0581dc14dddbbd828702fb778b7b037f1aefd Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Thu, 20 Aug 2026 12:11:26 +0200 Subject: [PATCH 13/26] Revert "fix: chat order could flip while some batch-sent files were still uploading" This reverts commit 0ec924cfbcef6034ebc87061ed3154b429e09bc5. Assisted-by: Claude:claude-sonnet-5 Signed-off-by: Marcel Hibbe --- .../chat/data/network/ChatMessageSyncer.kt | 21 +----------------- .../talk/data/database/dao/ChatMessagesDao.kt | 22 ------------------- .../utils/preview/ComposePreviewUtilsDaos.kt | 6 ----- 3 files changed, 1 insertion(+), 48 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt b/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt index 81f45358aad..99566ff5543 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/data/network/ChatMessageSyncer.kt @@ -790,26 +790,7 @@ class ChatMessageSyncer @Inject constructor( handleSystemMessagesThatAffectDatabase(target, chatMessages, events) val chatMessageEntities = chatMessages.map { - val entity = it.asEntity(target.accountId) - // If this message has a still-present local placeholder, keep the placeholder's - // (earlier) timestamp instead of the server's own. Uploads within a conversation are - // chained sequentially, so a file can take real seconds/minutes to actually finish - // uploading and sharing - by then, its server timestamp can be later than the - // instantly-assigned, still-pending placeholder timestamps of files queued after it in - // the same batch, which would otherwise flip their relative chat order once this one - // syncs in while those are still just placeholders. - val referenceId = entity.referenceId - if (referenceId != null) { - val placeholder = chatDao.getTempMessageForConversationOnce( - target.internalConversationId, - referenceId, - target.threadId - ) - if (placeholder != null && placeholder.timestamp < entity.timestamp) { - entity.timestamp = placeholder.timestamp - } - } - entity + it.asEntity(target.accountId) } try { diff --git a/app/src/main/java/com/nextcloud/talk/data/database/dao/ChatMessagesDao.kt b/app/src/main/java/com/nextcloud/talk/data/database/dao/ChatMessagesDao.kt index 2d4a98ea619..352c54f2f65 100644 --- a/app/src/main/java/com/nextcloud/talk/data/database/dao/ChatMessagesDao.kt +++ b/app/src/main/java/com/nextcloud/talk/data/database/dao/ChatMessagesDao.kt @@ -95,28 +95,6 @@ interface ChatMessagesDao { threadId: Long? ): Flow - // One-shot (not Flow) variant used when syncing a real message in: its placeholder's original - // timestamp is needed synchronously, before the placeholder itself is overwritten/deleted, to - // decide whether the real message should keep that earlier timestamp (see - // ChatMessageSyncer.persistChatMessagesAndHandleSystemMessages). - @Query( - """ - SELECT * - FROM ChatMessages - WHERE internalConversationId = :internalConversationId - AND referenceId = :referenceId - AND isTemporary = 1 - AND (:threadId IS NULL OR threadId = :threadId) - ORDER BY timestamp DESC, id DESC - LIMIT 1 - """ - ) - suspend fun getTempMessageForConversationOnce( - internalConversationId: String, - referenceId: String, - threadId: Long? - ): ChatMessageEntity? - @Insert(onConflict = OnConflictStrategy.REPLACE) suspend fun upsertChatMessages(chatMessages: List) diff --git a/app/src/main/java/com/nextcloud/talk/utils/preview/ComposePreviewUtilsDaos.kt b/app/src/main/java/com/nextcloud/talk/utils/preview/ComposePreviewUtilsDaos.kt index da8bbb2d7dc..842d4bebbcb 100644 --- a/app/src/main/java/com/nextcloud/talk/utils/preview/ComposePreviewUtilsDaos.kt +++ b/app/src/main/java/com/nextcloud/talk/utils/preview/ComposePreviewUtilsDaos.kt @@ -51,12 +51,6 @@ class DummyChatMessagesDaoImpl : ChatMessagesDao { threadId: Long? ): Flow = flowOf() - override suspend fun getTempMessageForConversationOnce( - internalConversationId: String, - referenceId: String, - threadId: Long? - ): ChatMessageEntity? = null - override suspend fun upsertChatMessages(chatMessages: List) { /* */ } From 5cfcfa3f129cc4563b75c912eee37bda53e51ded Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Thu, 20 Aug 2026 12:17:59 +0200 Subject: [PATCH 14/26] fix: chat order could flip while some batch-sent files were still uploading Alternative to the reverted commit 0ec924cfb, which fixed this by overwriting a synced message's server-provided timestamp with its placeholder's - risky since other logic may treat that timestamp as the server's authoritative value (message expiry, etc.). Same root cause: uploads within a conversation are chained sequentially, so a file can take real seconds to actually finish uploading and sharing once its predecessors in the same batch are ahead of it in the queue. By the time its real message syncs in, the server's own timestamp for it can be later than the still-pending, instantly-assigned placeholder timestamps of files queued after it in the same batch, flipping their relative order in the timestamp-sorted list. This time the DB values are left untouched. ChatViewModel now tracks, purely in memory for the lifetime of the screen, the order uploads were started in per referenceId, and reorderKnownSendSequence() corrects just the relative order of messages it has a hint for when building the displayed list - every other message (older history, other users, prior sessions) keeps its exact DB position. Once a whole batch finishes syncing, chaining already guarantees the server's own timestamps land in the right order on their own, so this only matters during the transient partially-synced window. Assisted-by: Claude:claude-sonnet-5 Signed-off-by: Marcel Hibbe --- .../talk/chat/viewmodels/ChatViewModel.kt | 57 +++++++++++++++---- 1 file changed, 47 insertions(+), 10 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt b/app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt index 6bac1cd9f0d..45d5be8b0c7 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt @@ -236,6 +236,40 @@ class ChatViewModel @AssistedInject constructor( // name, which would otherwise cancel every other queued upload too. private val uploadReferenceToWorkId = mutableMapOf() + // Maps referenceId -> the order files were sent in during this screen's session, assigned the + // moment each upload starts. Deliberately session-local and never persisted or sent anywhere: + // uploads within a conversation are chained sequentially (see UploadAndShareFilesWorker.upload), + // so a file can still be mid-upload when an earlier one in the same batch has already synced + // back with its real, server-assigned timestamp - which, purely because of how long the earlier + // upload took, can be later than the still-pending file's instantly-assigned placeholder + // timestamp, flipping their relative order in the timestamp-sorted list. Nudging the DB's own + // "timestamp"/"id" values to fix this would risk side effects on anything else that treats + // timestamp as the server's authoritative value (message expiry, etc.), so instead this is + // applied only as a display-order correction, on top of the otherwise-unmodified DB order. + private val referenceIdSendSequence = mutableMapOf() + private var nextSendSequenceValue = 0L + + // Corrects the relative order of messages we know the local send sequence for (see + // referenceIdSendSequence above), leaving every other message's position untouched - so this + // never reorders anything relative to messages with no known hint (other users' messages, + // messages sent before this screen session, older history, ...), only fixes the relative order + // among messages from the SAME batch while some are still catching up on syncing. + @Suppress("Detekt.ReturnCount") + private fun reorderKnownSendSequence(messages: List): List { + if (referenceIdSendSequence.isEmpty()) return messages + + val hintedIndices = messages.indices.filter { referenceIdSendSequence.containsKey(messages[it].referenceId) } + if (hintedIndices.size < 2) return messages + + val hintedItems = hintedIndices.map { messages[it] } + val sortedItems = hintedItems.sortedBy { referenceIdSendSequence[it.referenceId] } + if (sortedItems == hintedItems) return messages + + val result = messages.toMutableList() + hintedIndices.forEachIndexed { position, index -> result[index] = sortedItems[position] } + return result + } + fun cancelUpload(referenceId: String) { val workId = uploadReferenceToWorkId.remove(referenceId) ?: return UploadAndShareFilesWorker.cancelUpload(referenceId, workId) @@ -1109,16 +1143,18 @@ class ChatViewModel @AssistedInject constructor( val user = currentUserFlow.value applyMessageGrouping(messages) applySystemMessageGrouping(messages) - val uiMessages = messages.map { message -> - val parent: ChatMessage? = combinedMap[message.parentMessageId] - message.toUiModel( - user = user ?: currentUser, - chatMessage = message, - lastCommonReadMessageId = lastCommonRead, - parentMessage = parent, - isClassified = isClassified - ) - } + val uiMessages = reorderKnownSendSequence( + messages.map { message -> + val parent: ChatMessage? = combinedMap[message.parentMessageId] + message.toUiModel( + user = user ?: currentUser, + chatMessage = message, + lastCommonReadMessageId = lastCommonRead, + parentMessage = parent, + isClassified = isClassified + ) + } + ) val items = buildChatItems(uiMessages, conversationLastRead, expandedParents) ProcessedMessages(items = items, missingParentIds = missingParentIds) @@ -2065,6 +2101,7 @@ class ChatViewModel @AssistedInject constructor( val referenceId = UUID.randomUUID().toString().replace("-", "") metaDataMap["referenceId"] = referenceId + referenceIdSendSequence[referenceId] = nextSendSequenceValue++ val metaData = Gson().toJson(metaDataMap) From e75044c642c394f7a98965393ed288f06c1f00a1 Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Thu, 20 Aug 2026 13:43:29 +0200 Subject: [PATCH 15/26] refactor: apply reorderKnownSendSequence the same way as the grouping passes Moved from operating on the mapped List (returning a new list) to operating in place on the raw MutableList, the same way applyMessageGrouping()/applySystemMessageGrouping() already do - all three now run as consistent preprocessing steps before the single .map { toUiModel(...) } call, instead of the reorder wrapping that map call separately. No behavior change, just consistency with the existing pattern. Assisted-by: Claude:claude-sonnet-5 Signed-off-by: Marcel Hibbe --- .../talk/chat/viewmodels/ChatViewModel.kt | 45 ++++++++++--------- 1 file changed, 24 insertions(+), 21 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt b/app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt index 45d5be8b0c7..1514e22ccc7 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt @@ -253,21 +253,21 @@ class ChatViewModel @AssistedInject constructor( // referenceIdSendSequence above), leaving every other message's position untouched - so this // never reorders anything relative to messages with no known hint (other users' messages, // messages sent before this screen session, older history, ...), only fixes the relative order - // among messages from the SAME batch while some are still catching up on syncing. + // among messages from the SAME batch while some are still catching up on syncing. Applied the + // same way as applyMessageGrouping()/applySystemMessageGrouping() above it: in place, on the raw + // ChatMessage list, before it's mapped to ChatMessageUi. @Suppress("Detekt.ReturnCount") - private fun reorderKnownSendSequence(messages: List): List { - if (referenceIdSendSequence.isEmpty()) return messages + private fun reorderKnownSendSequence(messages: MutableList) { + if (referenceIdSendSequence.isEmpty()) return val hintedIndices = messages.indices.filter { referenceIdSendSequence.containsKey(messages[it].referenceId) } - if (hintedIndices.size < 2) return messages + if (hintedIndices.size < 2) return val hintedItems = hintedIndices.map { messages[it] } val sortedItems = hintedItems.sortedBy { referenceIdSendSequence[it.referenceId] } - if (sortedItems == hintedItems) return messages + if (sortedItems == hintedItems) return - val result = messages.toMutableList() - hintedIndices.forEachIndexed { position, index -> result[index] = sortedItems[position] } - return result + hintedIndices.forEachIndexed { position, index -> messages[index] = sortedItems[position] } } fun cancelUpload(referenceId: String) { @@ -1123,7 +1123,7 @@ class ChatViewModel @AssistedInject constructor( .debounce(MESSAGES_REBUILD_DEBOUNCE_MS) .map { input -> val ( - messages, + rawMessages, lastCommonRead, parentMap, conversationLastRead, @@ -1131,6 +1131,10 @@ class ChatViewModel @AssistedInject constructor( conversation, capabilities ) = input + // Mutable so reorderKnownSendSequence() can correct send order in place, the same + // way applyMessageGrouping()/applySystemMessageGrouping() annotate grouping in place + // - all three run before mapping to ChatMessageUi below. + val messages = rawMessages.toMutableList() val messageMap: Map = messages.associateBy { it.jsonMessageId.toLong() } val combinedMap: Map = messageMap + parentMap @@ -1143,18 +1147,17 @@ class ChatViewModel @AssistedInject constructor( val user = currentUserFlow.value applyMessageGrouping(messages) applySystemMessageGrouping(messages) - val uiMessages = reorderKnownSendSequence( - messages.map { message -> - val parent: ChatMessage? = combinedMap[message.parentMessageId] - message.toUiModel( - user = user ?: currentUser, - chatMessage = message, - lastCommonReadMessageId = lastCommonRead, - parentMessage = parent, - isClassified = isClassified - ) - } - ) + reorderKnownSendSequence(messages) + val uiMessages = messages.map { message -> + val parent: ChatMessage? = combinedMap[message.parentMessageId] + message.toUiModel( + user = user ?: currentUser, + chatMessage = message, + lastCommonReadMessageId = lastCommonRead, + parentMessage = parent, + isClassified = isClassified + ) + } val items = buildChatItems(uiMessages, conversationLastRead, expandedParents) ProcessedMessages(items = items, missingParentIds = missingParentIds) From cc75dd6c0ac2d44bbb878f8da4ab4c7ac1213f6c Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Thu, 20 Aug 2026 14:04:56 +0200 Subject: [PATCH 16/26] fix: match uploading image placeholder height to its final aspect ratio UploadingMediaMessage's AsyncImage only constrained width, leaving its height to Coil's own intrinsic sizing instead of the already-computed local aspect ratio. MediaMessage (the synced state) and even the sibling UploadingVideoPreview both lock height via Modifier.aspectRatio, so the image case was the odd one out - causing a visible resize once the placeholder was replaced by the final message. Assisted-by: Claude:claude-sonnet-5 Signed-off-by: Marcel Hibbe --- app/src/main/java/com/nextcloud/talk/ui/chat/MediaMessage.kt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/com/nextcloud/talk/ui/chat/MediaMessage.kt b/app/src/main/java/com/nextcloud/talk/ui/chat/MediaMessage.kt index a15107d28a2..015dce7cb29 100644 --- a/app/src/main/java/com/nextcloud/talk/ui/chat/MediaMessage.kt +++ b/app/src/main/java/com/nextcloud/talk/ui/chat/MediaMessage.kt @@ -429,11 +429,13 @@ fun UploadingMediaMessage( modifier = if (isImage || isVideo) Modifier else Modifier.fillMaxWidth() ) { if (isImage && typeContent.localFileUri.isNotEmpty()) { + val ratio = imageAspectRatio AsyncImage( model = typeContent.localFileUri.toUri(), contentDescription = typeContent.fileName, modifier = Modifier - .fillMaxWidth(mediaWidthFraction(imageAspectRatio)) + .fillMaxWidth(mediaWidthFraction(ratio)) + .then(if (ratio != null) Modifier.aspectRatio(ratio) else Modifier) .blur(4.dp) .padding(mediaInset) .clip(mediaShape), From 071b4683cfeeaac1fd363d79150666e03206d40c Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Thu, 20 Aug 2026 14:15:07 +0200 Subject: [PATCH 17/26] fix: confine upload placeholder blur within the image's clipped bounds blur() was applied before padding()/clip() in the modifier chain, so it drew over the whole outer box (including the inset margin) and only got clipped afterwards - letting the blurred edge bleed out to the message bubble's own border. Reordering to padding().clip().blur() confines the blur to the actual clipped image area, matching how the synced MediaMessage's crossfade layers are bounded. Assisted-by: Claude:claude-sonnet-5 Signed-off-by: Marcel Hibbe --- .../main/java/com/nextcloud/talk/ui/chat/MediaMessage.kt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/ui/chat/MediaMessage.kt b/app/src/main/java/com/nextcloud/talk/ui/chat/MediaMessage.kt index 015dce7cb29..ec377356309 100644 --- a/app/src/main/java/com/nextcloud/talk/ui/chat/MediaMessage.kt +++ b/app/src/main/java/com/nextcloud/talk/ui/chat/MediaMessage.kt @@ -436,9 +436,9 @@ fun UploadingMediaMessage( modifier = Modifier .fillMaxWidth(mediaWidthFraction(ratio)) .then(if (ratio != null) Modifier.aspectRatio(ratio) else Modifier) - .blur(4.dp) .padding(mediaInset) - .clip(mediaShape), + .clip(mediaShape) + .blur(4.dp), contentScale = ContentScale.Crop ) } else if (isVideo && typeContent.localFileUri.isNotEmpty()) { @@ -550,9 +550,9 @@ private fun UploadingVideoPreview( modifier = Modifier .fillMaxWidth(widthFraction) .aspectRatio(aspectRatio) - .blur(4.dp) .padding(mediaInset) - .clip(mediaShape), + .clip(mediaShape) + .blur(4.dp), contentScale = ContentScale.Crop ) } else { From 5c843ef40a104bab7226167753192d7e79872908 Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Thu, 20 Aug 2026 14:49:31 +0200 Subject: [PATCH 18/26] fix: media bubble collapses to zero height when server omits width/height Some incoming file messages never get width/height in their JSON parameters (observed for certain file types). MediaMessage's two crossfade layers both use Modifier.matchParentSize() with no other sized sibling, so without an aspectRatio to size the Box by, it collapses to zero height - hiding the image entirely even though the preview loads successfully (confirmed via Coil logs: MEMORY_CACHE hits for the exact affected fileIds). Fall back to the loaded image's own intrinsic aspect ratio when the server doesn't report one, so the bubble sizes correctly once the preview loads instead of staying permanently collapsed. The server-reported ratio still always takes priority when present, so the normal path is unaffected. Assisted-by: Claude:claude-sonnet-5 Signed-off-by: Marcel Hibbe --- .../nextcloud/talk/ui/chat/MediaMessage.kt | 90 ++++++++++++------- 1 file changed, 56 insertions(+), 34 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/ui/chat/MediaMessage.kt b/app/src/main/java/com/nextcloud/talk/ui/chat/MediaMessage.kt index ec377356309..9f020918a54 100644 --- a/app/src/main/java/com/nextcloud/talk/ui/chat/MediaMessage.kt +++ b/app/src/main/java/com/nextcloud/talk/ui/chat/MediaMessage.kt @@ -41,6 +41,7 @@ import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.draw.clip +import androidx.compose.ui.geometry.isSpecified import androidx.compose.ui.graphics.Color import androidx.compose.ui.graphics.asImageBitmap import androidx.compose.ui.graphics.painter.BitmapPainter @@ -232,7 +233,7 @@ fun MediaMessage( ?.asImageBitmap() ?.let { BitmapPainter(it) } } - val aspectRatio = remember(typeContent.width, typeContent.height) { + val serverAspectRatio = remember(typeContent.width, typeContent.height) { val w = typeContent.width val h = typeContent.height if (w != null && h != null && w > 0 && h > 0) w.toFloat() / h else null @@ -260,6 +261,60 @@ fun MediaMessage( // disk, so showing blurhash instead would be a needless downgrade. val ownUploadPlaceholder = localPreviewPainter ?: blurhashPainter ?: fallbackPainter + // Created unconditionally (even for the local-video-frame branch below, where + // loadedImage is always null and this just sits in Coil's cheap Empty state) so its + // intrinsic size is available as a fallback aspect ratio - see loadedAspectRatio below. + val loadedPainter = rememberAsyncImagePainter( + model = loadedImage, + onError = { state -> + val cause = state.result.throwable + val isServerError = cause is HttpException && cause.response.code in 500..599 + if ( + isServerError && + !typeContent.previewUrl.isNullOrEmpty() && + retryCount < PREVIEW_MAX_RETRIES && + !retryPending + ) { + retryPending = true + scope.launch { + Log.d( + TAG, + "Preview returned HTTP ${(cause as HttpException).response.code}, " + + "scheduling retry ${retryCount + 1}/$PREVIEW_MAX_RETRIES " + + "for ${typeContent.previewUrl}" + ) + delay(PREVIEW_RETRY_DELAY_MS) + retryCount++ + retryPending = false + } + } + } + ) + val isLoaded = loadedPainter.state is AsyncImagePainter.State.Success + val loadedAlpha by animateFloatAsState( + targetValue = if (isLoaded) 1f else 0f, + animationSpec = tween(durationMillis = MEDIA_CROSSFADE_DURATION_MS), + label = "mediaLoadedAlpha" + ) + + // The server doesn't always report width/height (observed for some file types, e.g. + // screenshots) - without either value, the two matchParentSize() crossfade layers below + // have nothing to size the bubble by and it collapses to zero height, hiding the image + // even though it loaded successfully. Falling back to the loaded image's own intrinsic + // size once available fixes that case without affecting the normal (server-reported) + // path, which always takes priority when present. + val loadedIntrinsicSize = loadedPainter.intrinsicSize + val loadedAspectRatio = if ( + loadedIntrinsicSize.isSpecified && + loadedIntrinsicSize.width > 0f && + loadedIntrinsicSize.height > 0f + ) { + loadedIntrinsicSize.width / loadedIntrinsicSize.height + } else { + null + } + val aspectRatio = serverAspectRatio ?: loadedAspectRatio + val mediaModifier = Modifier .fillMaxWidth() .then(if (aspectRatio != null) Modifier.aspectRatio(aspectRatio) else Modifier) @@ -288,39 +343,6 @@ fun MediaMessage( contentScale = ContentScale.Crop ) } else { - val loadedPainter = rememberAsyncImagePainter( - model = loadedImage, - onError = { state -> - val cause = state.result.throwable - val isServerError = cause is HttpException && cause.response.code in 500..599 - if ( - isServerError && - !typeContent.previewUrl.isNullOrEmpty() && - retryCount < PREVIEW_MAX_RETRIES && - !retryPending - ) { - retryPending = true - scope.launch { - Log.d( - TAG, - "Preview returned HTTP ${(cause as HttpException).response.code}, " + - "scheduling retry ${retryCount + 1}/$PREVIEW_MAX_RETRIES " + - "for ${typeContent.previewUrl}" - ) - delay(PREVIEW_RETRY_DELAY_MS) - retryCount++ - retryPending = false - } - } - } - ) - val isLoaded = loadedPainter.state is AsyncImagePainter.State.Success - val loadedAlpha by animateFloatAsState( - targetValue = if (isLoaded) 1f else 0f, - animationSpec = tween(durationMillis = MEDIA_CROSSFADE_DURATION_MS), - label = "mediaLoadedAlpha" - ) - // Own explicit crossfade instead of relying on Coil's built-in one: the // placeholder is a Compose-supplied Painter (not a Coil-managed Drawable), so // Coil's crossfade transition doesn't reliably fade from what's actually on From 742ba46de7e006f71d94b55d4ed2a206f5bf7aed Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Thu, 20 Aug 2026 16:21:52 +0200 Subject: [PATCH 19/26] set min size for generic files Signed-off-by: Marcel Hibbe --- .../nextcloud/talk/ui/chat/MediaMessage.kt | 40 ++++++++++++++----- 1 file changed, 31 insertions(+), 9 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/ui/chat/MediaMessage.kt b/app/src/main/java/com/nextcloud/talk/ui/chat/MediaMessage.kt index 9f020918a54..83caae953bd 100644 --- a/app/src/main/java/com/nextcloud/talk/ui/chat/MediaMessage.kt +++ b/app/src/main/java/com/nextcloud/talk/ui/chat/MediaMessage.kt @@ -17,6 +17,7 @@ import androidx.compose.foundation.combinedClickable import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.aspectRatio +import androidx.compose.foundation.layout.defaultMinSize import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.padding @@ -119,6 +120,8 @@ private const val PORTRAIT_WIDTH_FRACTION = 0.80f private fun mediaWidthFraction(aspectRatio: Float?): Float = if (aspectRatio != null && aspectRatio < 1f) PORTRAIT_WIDTH_FRACTION else 1f +private val noPreviewMinSize = 160.dp + @Suppress("Detekt.LongMethod", "LongParameterList", "CyclomaticComplexMethod") @Composable fun MediaMessage( @@ -315,27 +318,46 @@ fun MediaMessage( } val aspectRatio = serverAspectRatio ?: loadedAspectRatio + val showsGenericIcon = localVideoFramePainter == null && aspectRatio == null + val mediaModifier = Modifier .fillMaxWidth() .then(if (aspectRatio != null) Modifier.aspectRatio(aspectRatio) else Modifier) .padding(mediaInset) .clip(mediaShape) - // The bubble wraps to this Box's requested width (ChatMessageScaffold's Surface has - // no fillMaxWidth of its own), so shrinking this - not just mediaModifier - is what - // actually shrinks the bubble along with the media, instead of leaving empty space - // beside a smaller image inside an unchanged-size bubble. - Box(modifier = Modifier.fillMaxWidth(mediaWidthFraction(aspectRatio))) { + Box( + modifier = Modifier + .fillMaxWidth(mediaWidthFraction(aspectRatio)) + .then( + if (showsGenericIcon) { + Modifier.defaultMinSize(minWidth = noPreviewMinSize, minHeight = noPreviewMinSize) + } else { + Modifier + } + ) + ) { val messageLongClickHandler = LocalMessageLongClickHandler.current val clickableModifier = mediaModifier.combinedClickable( onClick = { onImageClick(message.id) }, onLongClick = { messageLongClickHandler(message.id) } ) - // Rendered directly instead of routed through Coil's placeholder/fallback painters, - // since Coil's own null-data handling on a pre-built ImageRequest (see load() below) - // takes priority and never shows a composable-supplied fallback painter here. - if (localVideoFramePainter != null) { + if (showsGenericIcon) { + Icon( + painter = fallbackPainter, + contentDescription = stringResource(R.string.media_message_content_description), + modifier = Modifier + .size(120.dp) + .padding(mediaInset) + .align(Alignment.Center) + .combinedClickable( + onClick = { onImageClick(message.id) }, + onLongClick = { messageLongClickHandler(message.id) } + ), + tint = Color.Unspecified + ) + } else if (localVideoFramePainter != null) { Image( painter = localVideoFramePainter, contentDescription = stringResource(R.string.media_message_content_description), From bdfd76e01162225ca735703352153f7b83e6e618 Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Thu, 20 Aug 2026 17:14:26 +0200 Subject: [PATCH 20/26] style: correctly tint the package/folder icon Signed-off-by: Andy Scherzinger --- .../main/java/com/nextcloud/talk/ui/chat/MediaMessage.kt | 7 ++++--- .../nextcloud/talk/ui/theme/TalkSpecificViewThemeUtils.kt | 6 +++++- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/ui/chat/MediaMessage.kt b/app/src/main/java/com/nextcloud/talk/ui/chat/MediaMessage.kt index 83caae953bd..8994f4d8aca 100644 --- a/app/src/main/java/com/nextcloud/talk/ui/chat/MediaMessage.kt +++ b/app/src/main/java/com/nextcloud/talk/ui/chat/MediaMessage.kt @@ -68,6 +68,7 @@ import com.nextcloud.talk.chat.ui.model.ChatMessageUi import com.nextcloud.talk.chat.ui.model.MessageStatusIcon import com.nextcloud.talk.chat.ui.model.MessageTypeContent import com.nextcloud.talk.contacts.load +import com.nextcloud.talk.ui.theme.mimetypeIconTint import com.nextcloud.talk.utils.Mimetype import com.nextcloud.talk.utils.MimetypeUtils import com.nextcloud.talk.utils.VideoThumbnailCache @@ -355,7 +356,7 @@ fun MediaMessage( onClick = { onImageClick(message.id) }, onLongClick = { messageLongClickHandler(message.id) } ), - tint = Color.Unspecified + tint = mimetypeIconTint(typeContent.drawableResourceId) ) } else if (localVideoFramePainter != null) { Image( @@ -500,7 +501,7 @@ fun UploadingMediaMessage( .size(64.dp) .padding(mediaInset) .align(Alignment.Center), - tint = Color.Unspecified + tint = mimetypeIconTint(typeContent.drawableResourceId) ) } @@ -614,7 +615,7 @@ private fun UploadingVideoPreview( modifier = Modifier .size(64.dp) .align(Alignment.Center), - tint = Color.Unspecified + tint = mimetypeIconTint(typeContent.drawableResourceId) ) } } diff --git a/app/src/main/java/com/nextcloud/talk/ui/theme/TalkSpecificViewThemeUtils.kt b/app/src/main/java/com/nextcloud/talk/ui/theme/TalkSpecificViewThemeUtils.kt index 78fd8801fbf..54c8334078a 100644 --- a/app/src/main/java/com/nextcloud/talk/ui/theme/TalkSpecificViewThemeUtils.kt +++ b/app/src/main/java/com/nextcloud/talk/ui/theme/TalkSpecificViewThemeUtils.kt @@ -225,7 +225,7 @@ class TalkSpecificViewThemeUtils @Inject constructor( context, drawableResourceId ) - if (drawable != null && THEMEABLE_PLACEHOLDER_IDS.contains(drawableResourceId)) { + if (drawable != null && isThemeablePlaceholder(drawableResourceId)) { colorDrawable(context, drawable) } return drawable @@ -498,6 +498,10 @@ class TalkSpecificViewThemeUtils @Inject constructor( R.drawable.ic_mimetype_folder ) + /** Whether the mimetype placeholder [drawableResourceId] is monochrome and picks up the theme color. */ + fun isThemeablePlaceholder(@DrawableRes drawableResourceId: Int): Boolean = + THEMEABLE_PLACEHOLDER_IDS.contains(drawableResourceId) + private val ALPHA_80_INT: Int = (255 * 0.8).roundToInt() private const val HALF_ALPHA_INT: Int = 255 / 2 From 25a2596ad913744a12e17eed72245fc65ddaa33c Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Thu, 20 Aug 2026 17:30:34 +0200 Subject: [PATCH 21/26] chore: add Compose previews for ThumbnailStrip and use outline variant icons Assisted-by: Claude Code:claude-opus-5[1m] Signed-off-by: Andy Scherzinger --- .../attachmentpreview/FileThumbnailImage.kt | 9 +- .../talk/attachmentpreview/ThumbnailStrip.kt | 15 +-- .../ThumbnailStripPreviews.kt | 115 ++++++++++++++++++ .../nextcloud/talk/ui/theme/MimetypeIcons.kt | 27 ++++ 4 files changed, 156 insertions(+), 10 deletions(-) create mode 100644 app/src/main/java/com/nextcloud/talk/attachmentpreview/ThumbnailStripPreviews.kt create mode 100644 app/src/main/java/com/nextcloud/talk/ui/theme/MimetypeIcons.kt diff --git a/app/src/main/java/com/nextcloud/talk/attachmentpreview/FileThumbnailImage.kt b/app/src/main/java/com/nextcloud/talk/attachmentpreview/FileThumbnailImage.kt index ddae262909f..0f3d0b8a480 100644 --- a/app/src/main/java/com/nextcloud/talk/attachmentpreview/FileThumbnailImage.kt +++ b/app/src/main/java/com/nextcloud/talk/attachmentpreview/FileThumbnailImage.kt @@ -25,6 +25,7 @@ import androidx.compose.ui.unit.Dp import androidx.compose.ui.unit.dp import coil.compose.AsyncImage import com.nextcloud.talk.R +import com.nextcloud.talk.ui.theme.mimetypeIconTint import com.nextcloud.talk.utils.DrawableUtils internal const val THUMBNAIL_CORNER_RADIUS_DP = 16 @@ -74,17 +75,19 @@ internal fun FileThumbnailImage( } } - MediaKind.OTHER -> + MediaKind.OTHER -> { + val mimetypeIcon = DrawableUtils.getDrawableResourceIdForMimeType(description.mimeType) Box( modifier = modifier.clip(shape).background(backgroundColor), contentAlignment = Alignment.Center ) { Icon( - painter = painterResource(DrawableUtils.getDrawableResourceIdForMimeType(description.mimeType)), + painter = painterResource(mimetypeIcon), contentDescription = description.name, - tint = Color.Unspecified, + tint = mimetypeIconTint(mimetypeIcon), modifier = Modifier.size(iconSize) ) } + } } } diff --git a/app/src/main/java/com/nextcloud/talk/attachmentpreview/ThumbnailStrip.kt b/app/src/main/java/com/nextcloud/talk/attachmentpreview/ThumbnailStrip.kt index 7d4feb80aff..c976226d047 100644 --- a/app/src/main/java/com/nextcloud/talk/attachmentpreview/ThumbnailStrip.kt +++ b/app/src/main/java/com/nextcloud/talk/attachmentpreview/ThumbnailStrip.kt @@ -25,9 +25,10 @@ import androidx.compose.foundation.lazy.itemsIndexed import androidx.compose.foundation.shape.CircleShape import androidx.compose.foundation.shape.RoundedCornerShape import androidx.compose.material.icons.Icons -import androidx.compose.material.icons.filled.Delete -import androidx.compose.material.icons.filled.PhotoCamera -import androidx.compose.material.icons.filled.Videocam +import androidx.compose.material.icons.outlined.Delete +import androidx.compose.material.icons.outlined.PhotoCamera +import androidx.compose.material.icons.outlined.PhotoLibrary +import androidx.compose.material.icons.outlined.Videocam import androidx.compose.material3.Icon import androidx.compose.material3.MaterialTheme import androidx.compose.runtime.Composable @@ -270,7 +271,7 @@ private fun StripThumbnail( contentAlignment = Alignment.Center ) { Icon( - imageVector = Icons.Filled.Delete, + imageVector = Icons.Outlined.Delete, contentDescription = stringResource(R.string.nc_remove_file), tint = Color.White, modifier = Modifier.size(STRIP_ICON_SIZE_DP.dp) @@ -291,7 +292,7 @@ private fun AddMoreTile(onClick: () -> Unit) { contentAlignment = Alignment.Center ) { Icon( - painter = painterResource(R.drawable.baseline_photo_library_24), + imageVector = Icons.Outlined.PhotoLibrary, contentDescription = stringResource(R.string.nc_add_more_files), modifier = Modifier.size(STRIP_ICON_SIZE_DP.dp) ) @@ -309,7 +310,7 @@ private fun TakePhotoTile(onClick: () -> Unit) { contentAlignment = Alignment.Center ) { Icon( - imageVector = Icons.Filled.PhotoCamera, + imageVector = Icons.Outlined.PhotoCamera, contentDescription = stringResource(R.string.take_photo), modifier = Modifier.size(STRIP_ICON_SIZE_DP.dp) ) @@ -327,7 +328,7 @@ private fun TakeVideoTile(onClick: () -> Unit) { contentAlignment = Alignment.Center ) { Icon( - imageVector = Icons.Filled.Videocam, + imageVector = Icons.Outlined.Videocam, contentDescription = stringResource(R.string.nc_take_video), modifier = Modifier.size(STRIP_ICON_SIZE_DP.dp) ) diff --git a/app/src/main/java/com/nextcloud/talk/attachmentpreview/ThumbnailStripPreviews.kt b/app/src/main/java/com/nextcloud/talk/attachmentpreview/ThumbnailStripPreviews.kt new file mode 100644 index 00000000000..d2a57e987b8 --- /dev/null +++ b/app/src/main/java/com/nextcloud/talk/attachmentpreview/ThumbnailStripPreviews.kt @@ -0,0 +1,115 @@ +/* + * Nextcloud Talk - Android Client + * + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: GPL-3.0-or-later + */ +package com.nextcloud.talk.attachmentpreview + +import android.content.res.Configuration +import android.graphics.Bitmap +import androidx.compose.foundation.isSystemInDarkTheme +import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.Surface +import androidx.compose.material3.darkColorScheme +import androidx.compose.material3.lightColorScheme +import androidx.compose.runtime.Composable +import androidx.compose.ui.tooling.preview.Preview +import androidx.core.graphics.createBitmap + +private const val PREVIEW_THUMBNAIL_PX = 64 + +// Coil can't load the image tiles' content in a preview, so only the video tile gets a stand-in +// bitmap - the flat color is enough to tell "has a thumbnail" apart from the icon fallback. +private fun previewVideoThumbnail(): Bitmap = + createBitmap(PREVIEW_THUMBNAIL_PX, PREVIEW_THUMBNAIL_PX) + .apply { eraseColor(android.graphics.Color.DKGRAY) } + +private fun previewDescription( + uri: String, + name: String, + mimeType: String, + detail: String, + videoThumbnail: Bitmap? = null +) = FileDescription( + uri = uri, + name = name, + kind = mediaKind(mimeType), + mimeType = mimeType, + detail = detail, + videoThumbnail = videoThumbnail +) + +private fun previewDescriptions() = + listOf( + previewDescription("file:///sdcard/DCIM/photo.jpg", "photo.jpg", "image/jpeg", "2048×1152, 210 kB"), + previewDescription( + uri = "file:///sdcard/DCIM/clip.mp4", + name = "clip.mp4", + mimeType = "video/mp4", + detail = "0:42, 8 MB", + videoThumbnail = previewVideoThumbnail() + ), + previewDescription("file:///sdcard/Download/archive.zip", "archive.zip", "application/zip", "4 MB"), + previewDescription("file:///sdcard/Documents/report.pdf", "report.pdf", "application/pdf", "820 kB") + ) + +@Composable +private fun ThumbnailStripPreviewContainer(descriptions: List, selectedIndex: Int) { + val colorScheme = if (isSystemInDarkTheme()) darkColorScheme() else lightColorScheme() + MaterialTheme(colorScheme = colorScheme) { + Surface { + ThumbnailStrip( + descriptions = descriptions, + selectedIndex = selectedIndex, + onSelect = {}, + onRemove = {}, + onReorder = { _, _ -> }, + onAddMore = {}, + onTakePhoto = {}, + onTakeVideo = {} + ) + } + } +} + +@Preview(name = "Light Mode", showBackground = true) +@Preview( + name = "Dark Mode", + showBackground = true, + uiMode = Configuration.UI_MODE_NIGHT_YES or Configuration.UI_MODE_TYPE_NORMAL +) +@Composable +private fun ThumbnailStripPreview() { + ThumbnailStripPreviewContainer(descriptions = previewDescriptions(), selectedIndex = 0) +} + +/** The mimetype-icon tiles, where [mimetypeIconTint] decides whether an icon is themed or keeps its own colors. */ +@Preview(name = "Documents Light", showBackground = true) +@Preview( + name = "Documents Dark", + showBackground = true, + uiMode = Configuration.UI_MODE_NIGHT_YES or Configuration.UI_MODE_TYPE_NORMAL +) +@Composable +private fun ThumbnailStripDocumentsPreview() { + ThumbnailStripPreviewContainer( + descriptions = listOf( + previewDescription("file:///sdcard/Download/archive.zip", "archive.zip", "application/zip", "4 MB"), + previewDescription("file:///sdcard/Documents/report.pdf", "report.pdf", "application/pdf", "820 kB"), + previewDescription("file:///sdcard/Documents/notes.txt", "notes.txt", "text/plain", "2 kB"), + previewDescription("file:///sdcard/Documents/letter.doc", "letter.doc", "application/msword", "34 kB") + ), + selectedIndex = 1 + ) +} + +/** A single file shows no thumbnails at all - just the centered action tiles. */ +@Preview(name = "Single File", showBackground = true) +@Composable +private fun ThumbnailStripSingleFilePreview() { + ThumbnailStripPreviewContainer( + descriptions = previewDescriptions().take(1), + selectedIndex = 0 + ) +} diff --git a/app/src/main/java/com/nextcloud/talk/ui/theme/MimetypeIcons.kt b/app/src/main/java/com/nextcloud/talk/ui/theme/MimetypeIcons.kt new file mode 100644 index 00000000000..ff74ac0cf50 --- /dev/null +++ b/app/src/main/java/com/nextcloud/talk/ui/theme/MimetypeIcons.kt @@ -0,0 +1,27 @@ +/* + * Nextcloud Talk - Android Client + * + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: GPL-3.0-or-later + */ + +package com.nextcloud.talk.ui.theme + +import androidx.annotation.DrawableRes +import androidx.compose.material3.MaterialTheme +import androidx.compose.runtime.Composable +import androidx.compose.runtime.ReadOnlyComposable +import androidx.compose.ui.graphics.Color + +/** + * Tint for a mimetype icon rendered as an [androidx.compose.material3.Icon]. Monochrome placeholders get + * the theme color, every other mimetype icon keeps its own colors ([Color.Unspecified]). + */ +@Composable +@ReadOnlyComposable +fun mimetypeIconTint(@DrawableRes drawableResourceId: Int): Color = + if (TalkSpecificViewThemeUtils.isThemeablePlaceholder(drawableResourceId)) { + MaterialTheme.colorScheme.primary + } else { + Color.Unspecified + } From ad2cfbcbd68686ae49ac44ece683f0579056f476 Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Thu, 20 Aug 2026 17:41:07 +0200 Subject: [PATCH 22/26] style: theme the attachment action tiles as Material3 tonal buttons The gallery, photo and video tiles in the attachment thumbnail strip used a surfaceVariant background with no explicit icon tint, so the icons fell back to LocalContentColor - onSurface, inherited from the screen's Surface. That is not a matching Material3 pair and left the contrast up to chance. Use the tonal button pairing instead: secondaryContainer for the container and onSecondaryContainer for the icon. Assisted-by: Claude Code:claude-opus-5 Signed-off-by: Andy Scherzinger --- .../talk/attachmentpreview/ThumbnailStrip.kt | 71 ++++++++----------- 1 file changed, 28 insertions(+), 43 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/attachmentpreview/ThumbnailStrip.kt b/app/src/main/java/com/nextcloud/talk/attachmentpreview/ThumbnailStrip.kt index c976226d047..9acac64bb0c 100644 --- a/app/src/main/java/com/nextcloud/talk/attachmentpreview/ThumbnailStrip.kt +++ b/app/src/main/java/com/nextcloud/talk/attachmentpreview/ThumbnailStrip.kt @@ -6,6 +6,7 @@ */ package com.nextcloud.talk.attachmentpreview +import androidx.annotation.StringRes import androidx.compose.foundation.background import androidx.compose.foundation.border import androidx.compose.foundation.clickable @@ -41,6 +42,7 @@ import androidx.compose.ui.Modifier import androidx.compose.ui.draw.clip import androidx.compose.ui.graphics.Color import androidx.compose.ui.graphics.graphicsLayer +import androidx.compose.ui.graphics.vector.ImageVector import androidx.compose.ui.input.pointer.pointerInput import androidx.compose.ui.platform.LocalDensity import androidx.compose.ui.res.painterResource @@ -108,9 +110,27 @@ internal fun ThumbnailStrip( ) } } - item { AddMoreTile(onClick = onAddMore) } - item { TakePhotoTile(onClick = onTakePhoto) } - item { TakeVideoTile(onClick = onTakeVideo) } + item { + ActionTile( + icon = Icons.Outlined.PhotoLibrary, + contentDescription = R.string.nc_add_more_files, + onClick = onAddMore + ) + } + item { + ActionTile( + icon = Icons.Outlined.PhotoCamera, + contentDescription = R.string.take_photo, + onClick = onTakePhoto + ) + } + item { + ActionTile( + icon = Icons.Outlined.Videocam, + contentDescription = R.string.nc_take_video, + onClick = onTakeVideo + ) + } } } @@ -282,54 +302,19 @@ private fun StripThumbnail( } @Composable -private fun AddMoreTile(onClick: () -> Unit) { - Box( - modifier = Modifier - .size(STRIP_THUMBNAIL_SIZE_DP.dp) - .clip(RoundedCornerShape(THUMBNAIL_CORNER_RADIUS_DP.dp)) - .background(MaterialTheme.colorScheme.surfaceVariant) - .clickable(onClick = onClick), - contentAlignment = Alignment.Center - ) { - Icon( - imageVector = Icons.Outlined.PhotoLibrary, - contentDescription = stringResource(R.string.nc_add_more_files), - modifier = Modifier.size(STRIP_ICON_SIZE_DP.dp) - ) - } -} - -@Composable -private fun TakePhotoTile(onClick: () -> Unit) { - Box( - modifier = Modifier - .size(STRIP_THUMBNAIL_SIZE_DP.dp) - .clip(RoundedCornerShape(THUMBNAIL_CORNER_RADIUS_DP.dp)) - .background(MaterialTheme.colorScheme.surfaceVariant) - .clickable(onClick = onClick), - contentAlignment = Alignment.Center - ) { - Icon( - imageVector = Icons.Outlined.PhotoCamera, - contentDescription = stringResource(R.string.take_photo), - modifier = Modifier.size(STRIP_ICON_SIZE_DP.dp) - ) - } -} - -@Composable -private fun TakeVideoTile(onClick: () -> Unit) { +private fun ActionTile(icon: ImageVector, @StringRes contentDescription: Int, onClick: () -> Unit) { Box( modifier = Modifier .size(STRIP_THUMBNAIL_SIZE_DP.dp) .clip(RoundedCornerShape(THUMBNAIL_CORNER_RADIUS_DP.dp)) - .background(MaterialTheme.colorScheme.surfaceVariant) + .background(MaterialTheme.colorScheme.secondaryContainer) .clickable(onClick = onClick), contentAlignment = Alignment.Center ) { Icon( - imageVector = Icons.Outlined.Videocam, - contentDescription = stringResource(R.string.nc_take_video), + imageVector = icon, + contentDescription = stringResource(contentDescription), + tint = MaterialTheme.colorScheme.onSecondaryContainer, modifier = Modifier.size(STRIP_ICON_SIZE_DP.dp) ) } From 181491291e6578fc5f840a46e665d06b1e2c076f Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Thu, 20 Aug 2026 17:42:26 +0200 Subject: [PATCH 23/26] chore: add an RTL preview variant for the attachment preview screen The screen's previews only covered light and dark mode, so mirrored layout was never visible at design time. Reuse the locale = "ar" form the chat message previews already use. Assisted-by: Claude Code:claude-opus-5 Signed-off-by: Andy Scherzinger --- .../talk/attachmentpreview/FileAttachmentPreviewScreen.kt | 1 + 1 file changed, 1 insertion(+) diff --git a/app/src/main/java/com/nextcloud/talk/attachmentpreview/FileAttachmentPreviewScreen.kt b/app/src/main/java/com/nextcloud/talk/attachmentpreview/FileAttachmentPreviewScreen.kt index 584d8d331c3..ce2e8a11c42 100644 --- a/app/src/main/java/com/nextcloud/talk/attachmentpreview/FileAttachmentPreviewScreen.kt +++ b/app/src/main/java/com/nextcloud/talk/attachmentpreview/FileAttachmentPreviewScreen.kt @@ -236,6 +236,7 @@ private fun rememberPreviewViewModel(files: List): FileAttachmentPreview showBackground = true, uiMode = Configuration.UI_MODE_NIGHT_YES or Configuration.UI_MODE_TYPE_NORMAL ) +@Preview(name = "RTL Arabic", showBackground = true, locale = "ar") @Composable private fun FileAttachmentPreviewContentPreview() { val colorScheme = if (isSystemInDarkTheme()) darkColorScheme() else lightColorScheme() From bf6fba55b33c5218a6a5c5198afe67bf9f5203a1 Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Thu, 20 Aug 2026 17:52:28 +0200 Subject: [PATCH 24/26] style: match the attachment preview app bar to the M3 headline/subline spec Assisted-by: Claude Code:claude-opus-5 Signed-off-by: Andy Scherzinger --- .../FileAttachmentPreviewScreen.kt | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/attachmentpreview/FileAttachmentPreviewScreen.kt b/app/src/main/java/com/nextcloud/talk/attachmentpreview/FileAttachmentPreviewScreen.kt index ce2e8a11c42..ada48869f6f 100644 --- a/app/src/main/java/com/nextcloud/talk/attachmentpreview/FileAttachmentPreviewScreen.kt +++ b/app/src/main/java/com/nextcloud/talk/attachmentpreview/FileAttachmentPreviewScreen.kt @@ -16,6 +16,7 @@ import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.height import androidx.compose.foundation.layout.imePadding import androidx.compose.foundation.layout.navigationBarsPadding import androidx.compose.foundation.layout.padding @@ -58,6 +59,9 @@ import kotlinx.coroutines.launch private const val MAX_ADD_MORE_FILES = 10 +private const val APP_BAR_HEIGHT_DP = 64 +private const val APP_BAR_HORIZONTAL_PADDING_DP = 4 + /** * Full-screen dialog content for reviewing, reordering and captioning files picked for upload, * hosted by [FileAttachmentPreviewFragment]. [viewModel] owns the file list and its (IO-derived) @@ -161,7 +165,8 @@ private fun PreviewTopBar(conversationName: String, onDismiss: () -> Unit) { verticalAlignment = Alignment.CenterVertically, modifier = Modifier .fillMaxWidth() - .padding(16.dp) + .height(APP_BAR_HEIGHT_DP.dp) + .padding(horizontal = APP_BAR_HORIZONTAL_PADDING_DP.dp) ) { IconButton(onClick = onDismiss) { Icon( @@ -170,16 +175,21 @@ private fun PreviewTopBar(conversationName: String, onDismiss: () -> Unit) { ) } - Column(modifier = Modifier.weight(1f)) { + Column( + modifier = Modifier + .weight(1f) + .padding(horizontal = APP_BAR_HORIZONTAL_PADDING_DP.dp) + ) { Text( text = conversationName, - style = MaterialTheme.typography.titleMedium, + style = MaterialTheme.typography.titleLarge, + color = MaterialTheme.colorScheme.onSurface, maxLines = 1, overflow = TextOverflow.Ellipsis ) Text( text = stringResource(R.string.nc_add_file), - style = MaterialTheme.typography.bodyMedium, + style = MaterialTheme.typography.labelMedium, color = MaterialTheme.colorScheme.onSurfaceVariant, maxLines = 1, overflow = TextOverflow.Ellipsis From 9f2ab880f0bb51e4cbce6a8cbc326aa18516bbd6 Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Thu, 20 Aug 2026 18:17:21 +0200 Subject: [PATCH 25/26] fix: upload placeholder vanished mid-upload for large images The uploading image placeholder only constrains its height via aspectRatio, which stays absent until the local file's ratio has been read - and reading it copies the whole file out of its content:// uri first, so on a large upload that window covers most of the transfer. With no aspect ratio, the bubble's height came solely from the AsyncImage's intrinsic size, so any moment Coil was not holding a decoded bitmap it collapsed to zero height and the placeholder disappeared from the chat until the real message arrived. Give it the same floor the other media bubbles already have. The uploading video branch falls back to a default aspect ratio and the generic-file branch is a fixed-size icon, so the image branch was the only one without one. Assisted-by: Claude Code:claude-opus-5 Signed-off-by: Andy Scherzinger --- .../main/java/com/nextcloud/talk/ui/chat/MediaMessage.kt | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/app/src/main/java/com/nextcloud/talk/ui/chat/MediaMessage.kt b/app/src/main/java/com/nextcloud/talk/ui/chat/MediaMessage.kt index 8994f4d8aca..8b593065a8a 100644 --- a/app/src/main/java/com/nextcloud/talk/ui/chat/MediaMessage.kt +++ b/app/src/main/java/com/nextcloud/talk/ui/chat/MediaMessage.kt @@ -481,6 +481,13 @@ fun UploadingMediaMessage( modifier = Modifier .fillMaxWidth(mediaWidthFraction(ratio)) .then(if (ratio != null) Modifier.aspectRatio(ratio) else Modifier) + // Without a floor, height comes solely from the AsyncImage's intrinsic + // size while no aspect ratio is known yet: any moment Coil isn't holding + // a decoded bitmap the bubble collapses to zero height and the + // placeholder disappears from the chat mid-upload. Reading the ratio + // needs the whole file copied out of its content:// uri first, so on a + // large upload that window lasts for most of the transfer. + .defaultMinSize(minHeight = noPreviewMinSize) .padding(mediaInset) .clip(mediaShape) .blur(4.dp), From 347f82d20bbade571b8951d68e4433da90afb7fd Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Thu, 20 Aug 2026 18:53:26 +0200 Subject: [PATCH 26/26] ci(detekt): Bump score Signed-off-by: Andy Scherzinger --- detekt.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/detekt.yml b/detekt.yml index fa2c4d8c5d5..4888905d22d 100644 --- a/detekt.yml +++ b/detekt.yml @@ -1,7 +1,7 @@ # SPDX-FileCopyrightText: 2017-2026 Nextcloud GmbH and Nextcloud contributors # SPDX-License-Identifier: GPL-3.0-or-later build: - maxIssues: 105 + maxIssues: 110 weights: # complexity: 2 # LongParameterList: 1