From 32916444e5ea9e69e32b688c13bbc2d59b51e689 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 6 May 2025 14:41:34 +0200 Subject: [PATCH] Fix read receipt behavior when the timeline is opened. (#4679) * Use Timber instead of println * Fix ReadReceipt not sent when new event is received: - filter Virtual item regarding typing to compute NewEventState - always scroll, since the latest event is always the typing notification (but with 0 height). This triggers the sending of RR. * Improve the fix to fix regression detected by unit tests. * Remove unnecessary parenthesis * Document parameter. --- .../messages/impl/timeline/TimelinePresenter.kt | 8 ++++++-- .../messages/impl/timeline/TimelineView.kt | 16 +++++++++++----- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelinePresenter.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelinePresenter.kt index cc6c9c5860..3be27fe7d0 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelinePresenter.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelinePresenter.kt @@ -28,6 +28,7 @@ import io.element.android.features.messages.impl.timeline.factories.TimelineItem import io.element.android.features.messages.impl.timeline.factories.TimelineItemsFactoryConfig import io.element.android.features.messages.impl.timeline.model.NewEventState import io.element.android.features.messages.impl.timeline.model.TimelineItem +import io.element.android.features.messages.impl.timeline.model.virtual.TimelineItemTypingNotificationModel import io.element.android.features.messages.impl.typing.TypingNotificationState import io.element.android.features.messages.impl.voicemessages.timeline.RedactedVoiceMessageManager import io.element.android.features.poll.api.actions.EndPollAction @@ -131,7 +132,7 @@ class TimelinePresenter @AssistedInject constructor( if (event.firstIndex == 0) { newEventState.value = NewEventState.None } - println("## sendReadReceiptIfNeeded firstVisibleIndex: ${event.firstIndex}") + Timber.d("## sendReadReceiptIfNeeded firstVisibleIndex: ${event.firstIndex}") appScope.sendReadReceiptIfNeeded( firstVisibleIndex = event.firstIndex, timelineItems = timelineItems, @@ -278,7 +279,10 @@ class TimelinePresenter @AssistedInject constructor( if (newEventState.value == NewEventState.FromMe) { return@withContext } - val newMostRecentItem = timelineItems.firstOrNull() + val newMostRecentItem = timelineItems.firstOrNull { + // Ignore typing item + (it as? TimelineItem.Virtual)?.model !is TimelineItemTypingNotificationModel + } val prevMostRecentItemIdValue = prevMostRecentItemId.value val newMostRecentItemId = newMostRecentItem?.identifier() val hasNewEvent = prevMostRecentItemIdValue != null && diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineView.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineView.kt index 6e677e5984..c64eec9bee 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineView.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineView.kt @@ -286,11 +286,16 @@ private fun BoxScope.TimelineScrollHelper( } var jumpToLiveHandled by remember { mutableStateOf(true) } - fun scrollToBottom() { + /** + * @param force If true, scroll to the bottom even if the user is already seeing the most recent item. + * This fixes the issue where the user is seeing typing notification and so the read receipt is not sent + * when a new message comes in. + */ + fun scrollToBottom(force: Boolean) { coroutineScope.launch { if (lazyListState.firstVisibleItemIndex > 10) { lazyListState.scrollToItem(0) - } else if (lazyListState.firstVisibleItemIndex != 0) { + } else if (force || lazyListState.firstVisibleItemIndex != 0) { lazyListState.animateScrollToItem(0) } } @@ -298,7 +303,7 @@ private fun BoxScope.TimelineScrollHelper( fun jumpToBottom() { if (isLive) { - scrollToBottom() + scrollToBottom(force = false) } else { jumpToLiveHandled = false onJumpToLive() @@ -321,9 +326,10 @@ private fun BoxScope.TimelineScrollHelper( } LaunchedEffect(canAutoScroll, newEventState) { - val shouldScrollToBottom = isScrollFinished && (canAutoScroll || newEventState == NewEventState.FromMe) + val shouldScrollToBottom = isScrollFinished && + (canAutoScroll && newEventState == NewEventState.FromOther || newEventState == NewEventState.FromMe) if (shouldScrollToBottom) { - scrollToBottom() + scrollToBottom(force = true) } }