Fix marking a room as read re-instantiates its timeline (#5628)

* Add `Timeline.markAsRead` to avoid reinstantiating the timeline using `Room.markAsRead`

* Mark as read when exiting the room screen, destroy the timeline when fully closed

* Ensure `MarkAsFullyReadAndExit` event can only be processed once

* Fix `DelayedVisibility` not being displayed in previews
This commit is contained in:
Jorge Martin Espinosa 2025-10-30 08:39:06 +01:00 committed by GitHub
parent bb61126c96
commit 6c3b280ecd
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
24 changed files with 281 additions and 89 deletions

View file

@ -0,0 +1,46 @@
/*
* Copyright 2025 New Vector Ltd.
*
* SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial
* Please see LICENSE files in the repository root for full details.
*/
package io.element.android.libraries.designsystem.utils
import androidx.compose.animation.AnimatedVisibility
import androidx.compose.runtime.Composable
import androidx.compose.runtime.LaunchedEffect
import androidx.compose.runtime.getValue
import androidx.compose.runtime.movableContentOf
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.compose.runtime.setValue
import androidx.compose.ui.platform.LocalInspectionMode
import kotlinx.coroutines.delay
import kotlin.time.Duration
import kotlin.time.Duration.Companion.milliseconds
/**
* Displays the content of [block] after a delay of [duration].
*/
@Composable
fun DelayedVisibility(
duration: Duration = 300.milliseconds,
block: @Composable () -> Unit,
) {
// Technically this shouldn't be needed because `LocalInspectionMode` won't change, but let's make the linter happy
val movableBlock = remember { movableContentOf { block() } }
if (LocalInspectionMode.current) {
// Just allow the contents to be displayed in the previews/screenshot tests
movableBlock()
} else {
var shouldDisplay by remember { mutableStateOf(false) }
LaunchedEffect(Unit) {
delay(duration)
shouldDisplay = true
}
AnimatedVisibility(shouldDisplay) {
movableBlock()
}
}
}

View file

@ -9,6 +9,7 @@ package io.element.android.libraries.matrix.api
import io.element.android.libraries.core.data.tryOrNull
import io.element.android.libraries.matrix.api.core.DeviceId
import io.element.android.libraries.matrix.api.core.EventId
import io.element.android.libraries.matrix.api.core.MatrixPatterns
import io.element.android.libraries.matrix.api.core.RoomAlias
import io.element.android.libraries.matrix.api.core.RoomId
@ -34,6 +35,7 @@ import io.element.android.libraries.matrix.api.roomlist.RoomListService
import io.element.android.libraries.matrix.api.spaces.SpaceService
import io.element.android.libraries.matrix.api.sync.SlidingSyncVersion
import io.element.android.libraries.matrix.api.sync.SyncService
import io.element.android.libraries.matrix.api.timeline.Timeline
import io.element.android.libraries.matrix.api.user.MatrixSearchUserResults
import io.element.android.libraries.matrix.api.user.MatrixUser
import io.element.android.libraries.matrix.api.verification.SessionVerificationService
@ -183,6 +185,14 @@ interface MatrixClient {
* Adds an emoji to the list of recent emoji reactions for this account.
*/
suspend fun addRecentEmoji(emoji: String): Result<Unit>
/**
* Marks the room with the provided [roomId] as read, sending a fully read receipt for [eventId].
*
* This method should be used with caution as providing the [eventId] ourselves can result in incorrect read receipts.
* Use [Timeline.markAsRead] instead when possible.
*/
suspend fun markRoomAsFullyRead(roomId: RoomId, eventId: EventId): Result<Unit>
}
/**

View file

@ -16,7 +16,7 @@ sealed class QrLoginException : Exception() {
data object OidcMetadataInvalid : QrLoginException()
data object SlidingSyncNotAvailable : QrLoginException()
data object OtherDeviceNotSignedIn : QrLoginException()
data object Unknown : QrLoginException()
data object CheckCodeAlreadySent : QrLoginException()
data object CheckCodeCannotBeSent : QrLoginException()
data object Unknown : QrLoginException()
}

View file

@ -17,6 +17,7 @@ import io.element.android.libraries.matrix.api.room.powerlevels.RoomPowerLevelsV
import io.element.android.libraries.matrix.api.room.tombstone.PredecessorRoom
import io.element.android.libraries.matrix.api.roomdirectory.RoomVisibility
import io.element.android.libraries.matrix.api.timeline.ReceiptType
import io.element.android.libraries.matrix.api.timeline.Timeline
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.StateFlow
@ -180,6 +181,10 @@ interface BaseRoom : Closeable {
/**
* Mark the room as read by trying to attach an unthreaded read receipt to the latest room event.
*
* Note this will instantiate a new timeline, which is an expensive operation.
* Prefer using [Timeline.markAsRead] instead when possible.
*
* @param receiptType The type of receipt to send.
*/
suspend fun markAsRead(receiptType: ReceiptType): Result<Unit>

View file

@ -55,6 +55,7 @@ interface Timeline : AutoCloseable {
val mode: Mode
val membershipChangeEventReceived: Flow<Unit>
suspend fun sendReadReceipt(eventId: EventId, receiptType: ReceiptType): Result<Unit>
suspend fun markAsRead(receiptType: ReceiptType): Result<Unit>
suspend fun paginate(direction: PaginationDirection): Result<Boolean>
val backwardPaginationStatus: StateFlow<PaginationStatus>
@ -227,4 +228,9 @@ interface Timeline : AutoCloseable {
* pinned
*/
suspend fun unpinEvent(eventId: EventId): Result<Boolean>
/**
* Get the latest event id of the timeline.
*/
suspend fun getLatestEventId(): Result<EventId?>
}

View file

@ -17,6 +17,7 @@ import io.element.android.libraries.core.extensions.runCatchingExceptions
import io.element.android.libraries.featureflag.api.FeatureFlagService
import io.element.android.libraries.matrix.api.MatrixClient
import io.element.android.libraries.matrix.api.core.DeviceId
import io.element.android.libraries.matrix.api.core.EventId
import io.element.android.libraries.matrix.api.core.RoomAlias
import io.element.android.libraries.matrix.api.core.RoomId
import io.element.android.libraries.matrix.api.core.RoomIdOrAlias
@ -713,6 +714,13 @@ class RustMatrixClient(
}
}
override suspend fun markRoomAsFullyRead(roomId: RoomId, eventId: EventId): Result<Unit> = withContext(sessionDispatcher) {
runCatchingExceptions {
val room = innerClient.getRoom(roomId.value) ?: error("Could not fetch associated room")
room.markAsFullyReadUnchecked(eventId.value)
}
}
private suspend fun getCacheSize(
includeCryptoDb: Boolean = false,
): Long = withContext(sessionDispatcher) {

View file

@ -473,6 +473,7 @@ class JoinedRustRoom(
override fun destroy() {
baseRoom.destroy()
liveInnerTimeline.destroy()
Timber.d("Room $roomId destroyed")
}
private fun InnerTimeline.map(

View file

@ -159,6 +159,12 @@ class RustTimeline(
}
}
override suspend fun markAsRead(receiptType: ReceiptType): Result<Unit> = withContext(dispatcher) {
runCatchingExceptions {
inner.markAsRead(receiptType.toRustReceiptType())
}
}
private fun updatePaginationStatus(direction: Timeline.PaginationDirection, update: (Timeline.PaginationStatus) -> Timeline.PaginationStatus) {
when (direction) {
Timeline.PaginationDirection.BACKWARDS -> backwardPaginationStatus.getAndUpdate(update)
@ -586,6 +592,12 @@ class RustTimeline(
}
}
override suspend fun getLatestEventId(): Result<EventId?> = withContext(dispatcher) {
runCatchingExceptions {
inner.latestEventId()?.let(::EventId)
}
}
private suspend fun fetchDetailsForEvent(eventId: EventId): Result<Unit> = withContext(dispatcher) {
runCatchingExceptions {
inner.fetchDetailsForEvent(eventId.value)

View file

@ -9,6 +9,7 @@ package io.element.android.libraries.matrix.test
import io.element.android.libraries.matrix.api.MatrixClient
import io.element.android.libraries.matrix.api.core.DeviceId
import io.element.android.libraries.matrix.api.core.EventId
import io.element.android.libraries.matrix.api.core.RoomAlias
import io.element.android.libraries.matrix.api.core.RoomId
import io.element.android.libraries.matrix.api.core.RoomIdOrAlias
@ -101,6 +102,7 @@ class FakeMatrixClient(
private val getJoinedRoomIdsResult: () -> Result<Set<RoomId>> = { Result.success(emptySet()) },
private val getRecentEmojisLambda: () -> Result<List<String>> = { Result.success(emptyList()) },
private val addRecentEmojiLambda: (String) -> Result<Unit> = { Result.success(Unit) },
private val markRoomAsFullyReadResult: (RoomId, EventId) -> Result<Unit> = { _, _ -> lambdaError() },
) : MatrixClient {
var setDisplayNameCalled: Boolean = false
private set
@ -344,4 +346,8 @@ class FakeMatrixClient(
override suspend fun getRecentEmojis(): Result<List<String>> {
return getRecentEmojisLambda()
}
override suspend fun markRoomAsFullyRead(roomId: RoomId, eventId: EventId): Result<Unit> {
return markRoomAsFullyReadResult(roomId, eventId)
}
}

View file

@ -49,6 +49,14 @@ class FakeTimeline(
override val membershipChangeEventReceived: Flow<Unit> = MutableSharedFlow(),
private val cancelSendResult: (TransactionId) -> Result<Unit> = { lambdaError() },
override val mode: Timeline.Mode = Timeline.Mode.Live,
private val markAsReadResult: (ReceiptType) -> Result<Unit> = { lambdaError() },
private val getLatestEventIdResult: () -> Result<EventId?> = { lambdaError() },
var sendReadReceiptLambda: (
eventId: EventId,
receiptType: ReceiptType,
) -> Result<Unit> = { _, _ ->
lambdaError()
}
) : Timeline {
var sendMessageLambda: (
body: String,
@ -397,18 +405,15 @@ class FakeTimeline(
)
}
var sendReadReceiptLambda: (
eventId: EventId,
receiptType: ReceiptType,
) -> Result<Unit> = { _, _ ->
lambdaError()
}
override suspend fun sendReadReceipt(
eventId: EventId,
receiptType: ReceiptType,
): Result<Unit> = sendReadReceiptLambda(eventId, receiptType)
override suspend fun markAsRead(receiptType: ReceiptType): Result<Unit> {
return markAsReadResult(receiptType)
}
var paginateLambda: (direction: Timeline.PaginationDirection) -> Result<Boolean> = {
Result.success(false)
}
@ -431,6 +436,10 @@ class FakeTimeline(
return unpinEventLambda(eventId)
}
override suspend fun getLatestEventId(): Result<EventId?> {
return getLatestEventIdResult()
}
var closeCounter = 0
private set