Skip to content

Commit 3603ed6

Browse files
authored
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.
1 parent c61ee59 commit 3603ed6

File tree

2 files changed

+17
-7
lines changed

2 files changed

+17
-7
lines changed

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelinePresenter.kt

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import io.element.android.features.messages.impl.timeline.factories.TimelineItem
2828
import io.element.android.features.messages.impl.timeline.factories.TimelineItemsFactoryConfig
2929
import io.element.android.features.messages.impl.timeline.model.NewEventState
3030
import io.element.android.features.messages.impl.timeline.model.TimelineItem
31+
import io.element.android.features.messages.impl.timeline.model.virtual.TimelineItemTypingNotificationModel
3132
import io.element.android.features.messages.impl.typing.TypingNotificationState
3233
import io.element.android.features.messages.impl.voicemessages.timeline.RedactedVoiceMessageManager
3334
import io.element.android.features.poll.api.actions.EndPollAction
@@ -131,7 +132,7 @@ class TimelinePresenter @AssistedInject constructor(
131132
if (event.firstIndex == 0) {
132133
newEventState.value = NewEventState.None
133134
}
134-
println("## sendReadReceiptIfNeeded firstVisibleIndex: ${event.firstIndex}")
135+
Timber.d("## sendReadReceiptIfNeeded firstVisibleIndex: ${event.firstIndex}")
135136
appScope.sendReadReceiptIfNeeded(
136137
firstVisibleIndex = event.firstIndex,
137138
timelineItems = timelineItems,
@@ -278,7 +279,10 @@ class TimelinePresenter @AssistedInject constructor(
278279
if (newEventState.value == NewEventState.FromMe) {
279280
return@withContext
280281
}
281-
val newMostRecentItem = timelineItems.firstOrNull()
282+
val newMostRecentItem = timelineItems.firstOrNull {
283+
// Ignore typing item
284+
(it as? TimelineItem.Virtual)?.model !is TimelineItemTypingNotificationModel
285+
}
282286
val prevMostRecentItemIdValue = prevMostRecentItemId.value
283287
val newMostRecentItemId = newMostRecentItem?.identifier()
284288
val hasNewEvent = prevMostRecentItemIdValue != null &&

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineView.kt

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -286,19 +286,24 @@ private fun BoxScope.TimelineScrollHelper(
286286
}
287287
var jumpToLiveHandled by remember { mutableStateOf(true) }
288288

289-
fun scrollToBottom() {
289+
/**
290+
* @param force If true, scroll to the bottom even if the user is already seeing the most recent item.
291+
* This fixes the issue where the user is seeing typing notification and so the read receipt is not sent
292+
* when a new message comes in.
293+
*/
294+
fun scrollToBottom(force: Boolean) {
290295
coroutineScope.launch {
291296
if (lazyListState.firstVisibleItemIndex > 10) {
292297
lazyListState.scrollToItem(0)
293-
} else if (lazyListState.firstVisibleItemIndex != 0) {
298+
} else if (force || lazyListState.firstVisibleItemIndex != 0) {
294299
lazyListState.animateScrollToItem(0)
295300
}
296301
}
297302
}
298303

299304
fun jumpToBottom() {
300305
if (isLive) {
301-
scrollToBottom()
306+
scrollToBottom(force = false)
302307
} else {
303308
jumpToLiveHandled = false
304309
onJumpToLive()
@@ -321,9 +326,10 @@ private fun BoxScope.TimelineScrollHelper(
321326
}
322327

323328
LaunchedEffect(canAutoScroll, newEventState) {
324-
val shouldScrollToBottom = isScrollFinished && (canAutoScroll || newEventState == NewEventState.FromMe)
329+
val shouldScrollToBottom = isScrollFinished &&
330+
(canAutoScroll && newEventState == NewEventState.FromOther || newEventState == NewEventState.FromMe)
325331
if (shouldScrollToBottom) {
326-
scrollToBottom()
332+
scrollToBottom(force = true)
327333
}
328334
}
329335

0 commit comments

Comments
 (0)