Skip to content

Commit 7f9a0bf

Browse files
committed
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
1 parent 604f591 commit 7f9a0bf

10 files changed

Lines changed: 428 additions & 320 deletions

File tree

app/src/main/java/com/nextcloud/talk/chat/ChatActivity.kt

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,7 @@ import com.nextcloud.talk.ui.chat.ChatView
160160
import com.nextcloud.talk.ui.chat.ChatViewCallbacks
161161
import com.nextcloud.talk.ui.chat.ChatViewState
162162
import com.nextcloud.talk.ui.chat.LocalUploadProgressProvider
163+
import com.nextcloud.talk.ui.chat.LocalUploadedLocalPreviewProvider
163164
import com.nextcloud.talk.ui.dialog.DateTimeCompose
164165
import com.nextcloud.talk.ui.dialog.GetPinnedOptionsDialog
165166
import com.nextcloud.talk.ui.dialog.SaveToStorageDialogFragment
@@ -792,12 +793,14 @@ class ChatActivity :
792793
SideEffect { chatListState = listState }
793794

794795
val uploadProgressMap by chatViewModel.uploadProgressMap.collectAsStateWithLifecycle()
796+
val uploadedLocalPreviewMap by chatViewModel.uploadedLocalPreviewMap.collectAsStateWithLifecycle()
795797

796798
CompositionLocalProvider(
797799
LocalViewThemeUtils provides viewThemeUtils,
798800
LocalMessageUtils provides messageUtils,
799801
LocalOpenGraphFetcher provides { url -> chatViewModel.fetchOpenGraph(url) },
800-
LocalUploadProgressProvider provides { refId -> uploadProgressMap[refId] }
802+
LocalUploadProgressProvider provides { refId -> uploadProgressMap[refId] },
803+
LocalUploadedLocalPreviewProvider provides { refId -> uploadedLocalPreviewMap[refId] }
801804
) {
802805
val currentlyPlayingId by chatViewModel.currentlyPlayedMessageId.collectAsState(null)
803806

app/src/main/java/com/nextcloud/talk/chat/data/ChatMessageRepository.kt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,7 @@ interface ChatMessageRepository : LifecycleAwareManager {
135135
@Suppress("LongParameterList")
136136
suspend fun addUploadPlaceholderMessage(
137137
localFileUri: String,
138+
fileName: String,
138139
caption: String,
139140
mimeType: String?,
140141
fileSize: Long,

app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -666,6 +666,7 @@ class OfflineFirstChatRepository @Inject constructor(
666666
@Suppress("Detekt.TooGenericExceptionCaught", "LongMethod")
667667
override suspend fun addUploadPlaceholderMessage(
668668
localFileUri: String,
669+
fileName: String,
669670
caption: String,
670671
mimeType: String?,
671672
fileSize: Long,
@@ -693,7 +694,7 @@ class OfflineFirstChatRepository @Inject constructor(
693694

694695
val fileParams = hashMapOf<String?, String?>(
695696
"type" to "file",
696-
"name" to caption,
697+
"name" to fileName,
697698
"mimetype" to (mimeType ?: ""),
698699
"size" to fileSize.toString(),
699700
"path" to localFileUri
@@ -707,7 +708,8 @@ class OfflineFirstChatRepository @Inject constructor(
707708
internalConversationId = internalConversationId,
708709
id = placeholderId,
709710
threadId = threadId,
710-
message = "{file}",
711+
// "{file}" is the sentinel the server (and rest of this app) uses for "no caption"
712+
message = caption.ifEmpty { "{file}" },
711713
deleted = false,
712714
token = conversationModel.token,
713715
actorId = currentUser.userId!!,
@@ -777,7 +779,11 @@ class OfflineFirstChatRepository @Inject constructor(
777779

778780
override suspend fun sendUnsentChatMessages(credentials: String, url: String) {
779781
val tempMessages = chatDao.getTempUnsentMessagesForConversation(internalConversationId, threadId).first()
780-
tempMessages.sortedBy { it.internalId }.onEach {
782+
// File-upload placeholders are also temporary messages, but they must never be resent as plain
783+
// text here: their "message" field is just the "{file}" sentinel, and a failed/interrupted upload
784+
// needs a real re-upload, not a bogus text message reusing its referenceId.
785+
val unsentTextMessages = tempMessages.filterNot { it.messageParameters?.containsKey("file") == true }
786+
unsentTextMessages.sortedBy { it.internalId }.onEach {
781787
sendChatMessage(
782788
credentials,
783789
url,

app/src/main/java/com/nextcloud/talk/chat/ui/model/ChatMessageUi.kt

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,8 @@ sealed interface MessageTypeContent {
8181

8282
data class UploadingMedia(
8383
val localFileUri: String,
84-
val caption: String,
84+
val fileName: String,
85+
val caption: String?,
8586
val mimeType: String?,
8687
val drawableResourceId: Int
8788
) : MessageTypeContent
@@ -276,12 +277,15 @@ fun getMessageTypeContent(user: User, message: ChatMessage, isClassified: Boolea
276277
?: MessageTypeContent.RegularText
277278
}
278279

280+
private const val FILE_PLACEHOLDER_MESSAGE = "{file}"
281+
279282
fun getUploadingMediaContent(message: ChatMessage): MessageTypeContent.UploadingMedia {
280283
val mimetype = message.fileParameters.mimetype
281284
val drawableResourceId = DrawableUtils.getDrawableResourceIdForMimeType(mimetype)
282285
return MessageTypeContent.UploadingMedia(
283286
localFileUri = message.fileParameters.path.orEmpty(),
284-
caption = message.fileParameters.name.orEmpty(),
287+
fileName = message.fileParameters.name.orEmpty(),
288+
caption = message.message.takeIf { it != FILE_PLACEHOLDER_MESSAGE },
285289
mimeType = mimetype.takeIf { !it.isNullOrEmpty() },
286290
drawableResourceId = drawableResourceId
287291
)

app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,12 @@ class ChatViewModel @AssistedInject constructor(
225225
private val _uploadProgressMap = MutableStateFlow<Map<String, Int>>(emptyMap())
226226
val uploadProgressMap: StateFlow<Map<String, Int>> = _uploadProgressMap
227227

228+
// Maps referenceId -> local device fileUri, kept around for a while after the upload finishes so the
229+
// final message can show the file we already have on disk instead of a generic mimetype icon while it
230+
// waits for the server-side preview to load for the first time.
231+
private val _uploadedLocalPreviewMap = MutableStateFlow<Map<String, String>>(emptyMap())
232+
val uploadedLocalPreviewMap: StateFlow<Map<String, String>> = _uploadedLocalPreviewMap
233+
228234
// Maps referenceId -> fileUri for cancellation support
229235
private val uploadReferenceToUri = mutableMapOf<String, String>()
230236

@@ -235,6 +241,7 @@ class ChatViewModel @AssistedInject constructor(
235241
chatRepository.deleteTempMessageByReferenceId(referenceId)
236242
}
237243
_uploadProgressMap.update { it - referenceId }
244+
_uploadedLocalPreviewMap.update { it - referenceId }
238245
}
239246

240247
fun getChatRepository(): ChatMessageRepository = chatRepository
@@ -2077,7 +2084,8 @@ class ChatViewModel @AssistedInject constructor(
20772084
viewModelScope.launch {
20782085
chatRepository.addUploadPlaceholderMessage(
20792086
localFileUri = fileUri,
2080-
caption = caption.ifEmpty { fileName },
2087+
fileName = fileName,
2088+
caption = caption,
20812089
mimeType = mimeType,
20822090
fileSize = fileSize,
20832091
referenceId = referenceId
@@ -2087,17 +2095,18 @@ class ChatViewModel @AssistedInject constructor(
20872095

20882096
val internalConversationId = "${currentUser.id}@$chatRoomToken"
20892097
val workerId = UploadAndShareFilesWorker.upload(
2090-
fileUri,
2091-
room,
2092-
displayName,
2093-
metaData,
2094-
compressImages,
2095-
referenceId,
2096-
internalConversationId
2098+
fileUri = fileUri,
2099+
roomToken = room,
2100+
conversationName = displayName,
2101+
metaData = metaData,
2102+
referenceId = referenceId,
2103+
internalConversationId = internalConversationId,
2104+
compressImages = compressImages
20972105
)
20982106

20992107
if (!isVoiceMessage) {
21002108
uploadReferenceToUri[referenceId] = fileUri
2109+
_uploadedLocalPreviewMap.update { it + (referenceId to fileUri) }
21012110
observeUploadProgress(workerId, referenceId)
21022111
}
21032112
} catch (e: IllegalArgumentException) {
@@ -2134,6 +2143,10 @@ class ChatViewModel @AssistedInject constructor(
21342143
if (workInfo.state.isFinished) {
21352144
_uploadProgressMap.update { it - referenceId }
21362145
uploadReferenceToUri.remove(referenceId)
2146+
viewModelScope.launch {
2147+
delay(LOCAL_PREVIEW_GRACE_PERIOD_MS)
2148+
_uploadedLocalPreviewMap.update { it - referenceId }
2149+
}
21372150
}
21382151
}
21392152
.launchIn(viewModelScope)
@@ -2493,7 +2506,7 @@ class ChatViewModel @AssistedInject constructor(
24932506
private const val LOAD_MORE_MESSAGES_LIMIT = 100
24942507
private const val POST_UPLOAD_FETCH_MAX_ATTEMPTS = 4
24952508
private const val POST_UPLOAD_FETCH_RETRY_DELAY_MS = 1_500L
2496-
2509+
private const val LOCAL_PREVIEW_GRACE_PERIOD_MS = 15_000L
24972510
private const val PLAUSIBLE_MESSAGE_ID_BUFFER = 10_000L
24982511

24992512
fun isPlausibleLastReadMessageId(messageId: Int, newestKnownRealMessageId: Long?): Boolean =

0 commit comments

Comments
 (0)