Skip to content

fix: stabilize message media loading [WPB-18914] #4687

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Garzas wants to merge 3 commits into
base: develop
Choose a base branch
from
+494 −134
Open

fix: stabilize message media loading [WPB-18914] #4687

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3632c05
fix: message list loading
Garzas Mar 20, 2026
70c24f0
Merge branch 'develop' into fix/message-list-loading
Garzas Mar 30, 2026
0622a92
fix: stabilize message media loading by removing local asset path fro…
Garzas Apr 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions app/src/main/kotlin/com/wire/android/mapper/RegularMessageContentMapper.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ import com.wire.android.ui.home.conversations.model.messagetypes.image.VisualMed
import com.wire.android.ui.markdown.toMarkdownDocument
import com.wire.android.ui.markdown.toMarkdownTextWithMentions
import com.wire.android.ui.theme.Accent
import com.wire.android.util.getVideoMetaData
import com.wire.android.util.time.ISOFormatter
import com.wire.android.util.ui.UIText
import com.wire.kalium.logic.data.asset.AttachmentType
Expand Down Expand Up @@ -297,12 +296,11 @@ class RegularMessageMapper @Inject constructor(
}

assetMessageContentMetadata.isVideo() -> {
val metaData = localData?.assetDataPath?.let { getVideoMetaData(it) }
val metaData = metadata as? AssetContent.AssetMetadata.Video
UIMessageContent.VideoMessage(
assetName = name ?: "",
assetExtension = name?.split(".")?.last() ?: "",
assetId = AssetId(remoteData.assetId, remoteData.assetDomain.orEmpty()),
assetDataPath = localData?.assetDataPath,
assetSizeInBytes = sizeInBytes,
deliveryStatus = mapRecipientsFailure(userList, deliveryStatus),
params = VisualMediaParams(metaData?.width ?: 0, metaData?.height ?: 0),
Expand All @@ -317,7 +315,6 @@ class RegularMessageMapper @Inject constructor(
assetExtension = name?.split(".")?.last() ?: "",
assetId = AssetId(remoteData.assetId, remoteData.assetDomain.orEmpty()),
assetSizeInBytes = sizeInBytes,
assetDataPath = localData?.assetDataPath,
deliveryStatus = mapRecipientsFailure(userList, deliveryStatus)
)
}
Expand Down
31 changes: 27 additions & 4 deletions app/src/main/kotlin/com/wire/android/media/audiomessage/AudioMessageViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ import kotlinx.coroutines.flow.map
import kotlinx.coroutines.flow.mapNotNull
import kotlinx.coroutines.launch
import kotlinx.serialization.Serializable
import java.util.concurrent.ConcurrentHashMap
import java.util.concurrent.ConcurrentMap
import javax.inject.Inject

@ViewModelScopedPreview
Expand All @@ -58,7 +60,9 @@ class AudioMessageViewModelImpl @Inject constructor(

private val args: AudioMessageArgs = savedStateHandle.scopedArgs()

override var state: AudioMessageState by mutableStateOf(AudioMessageState())
override var state: AudioMessageState by mutableStateOf(
AudioMessageState(wavesMask = cachedWavesMasks[args.key])
)
private set

init {
Expand Down Expand Up @@ -92,9 +96,15 @@ class AudioMessageViewModelImpl @Inject constructor(
}

private fun preloadAudioMessage() {
if (preloadStates.putIfAbsent(args.key, PreloadState.InFlight) != null) return
viewModelScope.launch {
// calls preload to initially fetch the audio asset data to be ready and schedule waves mask generation if needed
audioMessagePlayer.preloadAudioMessage(args.conversationId, args.messageId)
try {
// calls preload to initially fetch the audio asset data to be ready and schedule waves mask generation if needed
audioMessagePlayer.preloadAudioMessage(args.conversationId, args.messageId)
Copy link
Copy Markdown
Contributor

@saleniuk saleniuk Apr 1, 2026

Choose a reason for hiding this comment

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

ConversationAudioMessagePlayer is a singleton, wouldn't be better to have this logic of checking and not preloading the same audio message again inside this singleton player instead of having this not so pretty approach here with keeping a map in companion object?

preloadStates[args.key] = PreloadState.Succeeded
} catch (_: Throwable) {
preloadStates.remove(args.key, PreloadState.InFlight)
}
}
}

Expand All @@ -110,7 +120,10 @@ class AudioMessageViewModelImpl @Inject constructor(
}
.distinctUntilChanged()
.firstOrNull { it != null } // wait for the first non-null value
.let { state = state.copy(wavesMask = it) }
?.let {
cachedWavesMasks[args.key] = it
Copy link
Copy Markdown
Contributor

@saleniuk saleniuk Apr 1, 2026

Choose a reason for hiding this comment

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

Waves masks are being generated only once and then are stored in the DB, so there isn't much overhead here. fetchWavesMask waits until the waves mask is generated, or just takes the value from DB if it's already there.

I don't understand this idea of having cachedWavesMasks - fetchWavesMask method is still executed, even if we initially already have a wave mask in AudioMessageState, so there's no advantage.

state = state.copy(wavesMask = it)
}
}
}

Expand All @@ -131,6 +144,16 @@ class AudioMessageViewModelImpl @Inject constructor(
audioMessagePlayer.setSpeed(audioSpeed)
}
}

private companion object {
val cachedWavesMasks = ConcurrentHashMap<String, List<Int>>()
val preloadStates: ConcurrentMap<String, PreloadState> = ConcurrentHashMap()
}

private enum class PreloadState {
InFlight,
Succeeded,
}
}

@Serializable
Expand Down
2 changes: 1 addition & 1 deletion app/src/main/kotlin/com/wire/android/ui/common/multipart/MultipartAttachmentUi.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ fun AssetContent.toUiModel(progress: Float?) = MultipartAttachmentUi(
uuid = this.remoteData.assetId,
source = AssetSource.ASSET_STORAGE,
fileName = this.name,
localPath = this.localData?.assetDataPath,
localPath = null,
previewUrl = null,
mimeType = this.mimeType,
assetType = AttachmentFileType.fromMimeType(mimeType),
Expand Down
132 changes: 102 additions & 30 deletions app/src/main/kotlin/com/wire/android/ui/home/conversations/ConversationScreen.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
* You should have received a copy of the GNU General Public License
* along with this program. If not, see http://www.gnu.org/licenses/.
*/
@file:Suppress("TooManyFunctions")

package com.wire.android.ui.home.conversations

Expand All @@ -28,11 +29,13 @@ import androidx.compose.animation.scaleIn
import androidx.compose.animation.scaleOut
import androidx.compose.foundation.background
import androidx.compose.foundation.clickable
import androidx.compose.foundation.gestures.stopScroll
import androidx.compose.foundation.layout.Box
import androidx.compose.foundation.layout.BoxScope
import androidx.compose.foundation.layout.Column
import androidx.compose.foundation.layout.PaddingValues
import androidx.compose.foundation.layout.Row
import androidx.compose.foundation.layout.Spacer
import androidx.compose.foundation.layout.consumeWindowInsets
import androidx.compose.foundation.layout.fillMaxSize
import androidx.compose.foundation.layout.fillMaxWidth
Expand Down Expand Up @@ -1257,13 +1260,16 @@ fun MessageList(
val readLastMessageAtStartTriggered = remember { mutableStateOf(false) }
val shouldTriggerOldestMessageFetch = remember { mutableStateOf(true) }
val currentTime by currentTimeInMillisFlow.collectAsState(initial = System.currentTimeMillis())
val isPrependLoading = lazyPagingMessages.loadState.prepend is LoadState.Loading
val isPrependCompleted = lazyPagingMessages.loadState.prepend.endOfPaginationReached

LaunchedEffect(lazyPagingMessages.itemCount) {
if (lazyPagingMessages.itemCount > prevItemCount.value && selectedMessageId == null) {
val canScrollToLastMessage = prevItemCount.value > 0
&& lazyListState.firstVisibleItemIndex > 0
&& lazyListState.firstVisibleItemIndex <= MAXIMUM_SCROLLED_MESSAGES_UNTIL_AUTOSCROLL_STOPS
if (canScrollToLastMessage) {
lazyListState.stopScroll()
lazyListState.animateScrollToItem(0)
}
prevItemCount.value = lazyPagingMessages.itemCount
Expand Down Expand Up @@ -1328,20 +1334,19 @@ fun MessageList(
items(
count = lazyPagingMessages.itemCount,
key = lazyPagingMessages.itemKey { it.header.messageId },
contentType = lazyPagingMessages.itemContentType { it }
contentType = lazyPagingMessages.itemContentType { message ->
when (message) {
is UIMessage.Regular -> "regular_message"
is UIMessage.System -> "system_message"
}
}
) { index ->
val message: UIMessage = lazyPagingMessages[index]
?: return@items Box(
contentAlignment = Alignment.Center,
?: return@items Spacer(
modifier = Modifier
.fillMaxWidth()
.height(dimensions().spacing56x),
) {
WireCircularProgressIndicator(
progressColor = MaterialTheme.wireColorScheme.secondaryText,
size = dimensions().spacing24x
)
}
.height(dimensions().spacing56x)
)

val showAuthor = if (!isBubbleUiEnabled || conversationDetailsData is ConversationDetailsData.Group) {
rememberShouldShowHeader(index, message, lazyPagingMessages)
Expand Down Expand Up @@ -1370,8 +1375,8 @@ fun MessageList(
}
}

val swipeableConfiguration = remember(message) {
if (message is UIMessage.Regular && message.isSwipeable) {
val swipeableConfiguration = remember(message, lazyListState.isScrollInProgress) {
if (!lazyListState.isScrollInProgress && message is UIMessage.Regular && message.isSwipeable) {
SwipeableMessageConfiguration.Swipeable(
onSwipedRight = { onSwipedToReply(message) }.takeIf { message.isReplyable },
onSwipedLeft = { onSwipedToReact(message) }.takeIf { message.isReactionAllowed },
Expand Down Expand Up @@ -1411,7 +1416,9 @@ fun MessageList(
}
}
// reverse layout, so prepend needs to be added after all messages to be displayed at the top of the list
if (showHistoryLoadingIndicator && lazyPagingMessages.itemCount > 0) {
if (lazyPagingMessages.itemCount > 0 &&
(isPrependLoading || (showHistoryLoadingIndicator && isPrependCompleted))
) {
item(
key = "prepend_loading_indicator",
contentType = "prepend_loading_indicator",
Expand All @@ -1422,30 +1429,34 @@ fun MessageList(
.fillMaxWidth()
.padding(dimensions().spacing16x),
) {
var allMessagesPrepended by remember { mutableStateOf(false) }
LaunchedEffect(lazyPagingMessages.loadState) {
// When the list is being refreshed, the load state for prepend is cleared so the app doesn't know if
// the end of pagination is reached or not for prepend until the refresh is done, so we don't want to
// update the allMessagesPrepended state while refreshing and in that case keep the last updated state,
// otherwise the indicator will flicker while refreshing.
if (lazyPagingMessages.loadState.refresh is LoadState.NotLoading) {
allMessagesPrepended = lazyPagingMessages.loadState.prepend.endOfPaginationReached
}
val (text, prefixIconResId) = when {
showHistoryLoadingIndicator && isPrependCompleted ->
stringResource(R.string.conversation_history_loaded) to null
showHistoryLoadingIndicator ->
stringResource(R.string.conversation_history_loading) to R.drawable.ic_undo
else -> "" to null
}

val (text, prefixIconResId) = when (allMessagesPrepended) {
true -> stringResource(R.string.conversation_history_loaded) to null
false -> stringResource(R.string.conversation_history_loading) to R.drawable.ic_undo
if (showHistoryLoadingIndicator) {
PageLoadingIndicator(
text = text,
prefixIconResId = prefixIconResId,
)
} else {
WireCircularProgressIndicator(
progressColor = MaterialTheme.wireColorScheme.secondaryText,
size = dimensions().spacing24x
)
}
PageLoadingIndicator(
text = text,
prefixIconResId = prefixIconResId,
)
}
}
)
}
}
ScrollDateOverlay(
lazyListState = lazyListState,
lazyPagingMessages = lazyPagingMessages
)
JumpToPlayingAudioButton(
lazyListState = lazyListState,
lazyPagingMessages = lazyPagingMessages,
Expand All @@ -1456,6 +1467,62 @@ fun MessageList(
)
}

@Composable
private fun BoxScope.ScrollDateOverlay(
lazyListState: LazyListState,
lazyPagingMessages: LazyPagingItems<UIMessage>,
) {
val context = LocalContext.current
val dateLabel by remember(lazyListState, lazyPagingMessages) {
derivedStateOf {
if (!lazyListState.isScrollInProgress) return@derivedStateOf null

val visibleItems = lazyListState.layoutInfo.visibleItemsInfo
var message: UIMessage? = null
for (index in visibleItems.lastIndex downTo 0) {
message = lazyPagingMessages.peekOrNull(visibleItems[index].index)
if (message != null) break
}
message ?: return@derivedStateOf null
val messageDate = message.header.messageTime.utcISO.serverDate() ?: return@derivedStateOf null

DateUtils.formatDateTime(
context,
messageDate.time,
DateUtils.FORMAT_SHOW_WEEKDAY or DateUtils.FORMAT_SHOW_DATE
)
}
}

AnimatedVisibility(
visible = dateLabel != null,
modifier = Modifier
.align(Alignment.TopCenter)
.fillMaxWidth()
) {
Column(
modifier = Modifier
.fillMaxWidth()
.background(
color = MaterialTheme.wireColorScheme.surface.copy(alpha = 0.8f)
)
.padding(vertical = dimensions().spacing2x, horizontal = dimensions().spacing4x),
horizontalAlignment = Alignment.CenterHorizontally
) {
Text(
text = dateLabel.orEmpty(),
style = MaterialTheme.wireTypography.title03,
color = MaterialTheme.wireColorScheme.onSurface,
maxLines = 1,
overflow = TextOverflow.Ellipsis
)
}
}
}

private fun LazyPagingItems<UIMessage>.peekOrNull(index: Int): UIMessage? =
if (index in 0 until itemCount) peek(index) else null

@Composable
private fun MessageGroupDateTime(
now: Long,
Expand Down Expand Up @@ -1583,7 +1650,12 @@ fun JumpToLastMessageButton(
exit = scaleOut(),
) {
SmallFloatingActionButton(
onClick = { coroutineScope.launch { lazyListState.animateScrollToItem(0) } },
onClick = {
coroutineScope.launch {
lazyListState.stopScroll()
lazyListState.animateScrollToItem(0)
}
},
containerColor = MaterialTheme.wireColorScheme.secondaryText,
contentColor = MaterialTheme.wireColorScheme.onPrimaryButtonEnabled,
shape = CircleShape,
Expand Down
14 changes: 2 additions & 12 deletions ...n/kotlin/com/wire/android/ui/home/conversations/messages/ConversationMessagesViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,6 @@ import kotlinx.coroutines.flow.shareIn
import kotlinx.coroutines.launch
import kotlinx.coroutines.withContext
import okio.Path
import okio.Path.Companion.toPath
import javax.inject.Inject
import kotlin.math.max
import kotlin.time.Duration.Companion.seconds
Expand Down Expand Up @@ -258,10 +257,8 @@ class ConversationMessagesViewModel @Inject constructor(

fun openOrFetchAsset(messageId: String) = viewModelScope.launch(dispatchers.io()) {
if (isCellEnabledForConversation) {
val asset = getMessageByIdUseCase(conversationId, messageId).getAssetContent()

asset?.localAssetPath()?.let {
onOpenFileWithExternalApp(it.toPath(), asset.value.name)
assetDataPath(conversationId, messageId)?.let { (path, assetName) ->
onOpenFileWithExternalApp(path, assetName)
} ?: run {
attemptDownloadOfAsset(messageId)
}
Expand Down Expand Up @@ -443,11 +440,4 @@ class ConversationMessagesViewModel @Inject constructor(
}
}

private fun GetMessageByIdUseCase.Result.getAssetContent(): MessageContent.Asset? = when (this) {
is GetMessageByIdUseCase.Result.Success -> this.message.content as? MessageContent.Asset
else -> null
}

private fun MessageContent.Asset.localAssetPath(): String? = value.localData?.assetDataPath

private fun ConversationDetails.isWireCellEnabled() = (this as? ConversationDetails.Group)?.wireCell != null
Loading