Merge pull request #868 from vector-im/feature/fga/better_timeline_scroll

Feature/fga/better timeline scroll

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

@@ -22,21 +22,24 @@ import androidx.compose.runtime.MutableState
 import androidx.compose.runtime.collectAsState
 import androidx.compose.runtime.getValue
 import androidx.compose.runtime.mutableStateOf
+import androidx.compose.runtime.remember
 import androidx.compose.runtime.rememberCoroutineScope
 import androidx.compose.runtime.saveable.rememberSaveable
-import androidx.compose.runtime.setValue
 import io.element.android.features.messages.impl.timeline.factories.TimelineItemsFactory
 import io.element.android.features.messages.impl.timeline.model.TimelineItem
 import io.element.android.libraries.architecture.Presenter
+import io.element.android.libraries.core.coroutine.CoroutineDispatchers
 import io.element.android.libraries.matrix.api.core.EventId
 import io.element.android.libraries.matrix.api.room.MatrixRoom
 import io.element.android.libraries.matrix.api.room.MessageEventType
+import io.element.android.libraries.matrix.api.timeline.item.event.TimelineItemEventOrigin
 import io.element.android.libraries.matrix.ui.room.canSendEventAsState
 import kotlinx.collections.immutable.ImmutableList
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.flow.launchIn
 import kotlinx.coroutines.flow.onEach
 import kotlinx.coroutines.launch
+import kotlinx.coroutines.withContext
 import javax.inject.Inject
 
 private const val backPaginationEventLimit = 20
@@ -45,42 +48,52 @@ private const val backPaginationPageSize = 50
 class TimelinePresenter @Inject constructor(
     private val timelineItemsFactory: TimelineItemsFactory,
     private val room: MatrixRoom,
+    private val dispatchers: CoroutineDispatchers,
+    private val appScope: CoroutineScope,
 ) : Presenter<TimelineState> {
 
     private val timeline = room.timeline
 
     @Composable
     override fun present(): TimelineState {
-        val localCoroutineScope = rememberCoroutineScope()
+        val localScope = rememberCoroutineScope()
         val highlightedEventId: MutableState<EventId?> = rememberSaveable {
             mutableStateOf(null)
         }
 
-        var lastReadMarkerIndex by rememberSaveable { mutableStateOf(Int.MAX_VALUE) }
-        var lastReadMarkerId by rememberSaveable { mutableStateOf<EventId?>(null) }
+        val lastReadReceiptIndex = rememberSaveable { mutableStateOf(Int.MAX_VALUE) }
+        val lastReadReceiptId = rememberSaveable { mutableStateOf<EventId?>(null) }
 
         val timelineItems by timelineItemsFactory.collectItemsAsState()
         val paginationState by timeline.paginationState.collectAsState()
-
         val syncUpdateFlow = room.syncUpdateFlow.collectAsState()
         val userHasPermissionToSendMessage by room.canSendEventAsState(type = MessageEventType.ROOM_MESSAGE, updateKey = syncUpdateFlow.value)
 
+        val prevMostRecentItemId = rememberSaveable { mutableStateOf<String?>(null) }
+        val hasNewItems = remember { mutableStateOf(false) }
+
         fun handleEvents(event: TimelineEvents) {
             when (event) {
-                TimelineEvents.LoadMore -> localCoroutineScope.paginateBackwards()
+                TimelineEvents.LoadMore -> localScope.paginateBackwards()
                 is TimelineEvents.SetHighlightedEvent -> highlightedEventId.value = event.eventId
                 is TimelineEvents.OnScrollFinished -> {
-                    // Get last valid EventId seen by the user, as the first index might refer to a Virtual item
-                    val eventId = getLastEventIdBeforeOrAt(event.firstIndex, timelineItems) ?: return
-                    if (event.firstIndex <= lastReadMarkerIndex && eventId != lastReadMarkerId) {
-                        lastReadMarkerIndex = event.firstIndex
-                        lastReadMarkerId = eventId
-                        localCoroutineScope.sendReadReceipt(eventId)
+                    if (event.firstIndex == 0) {
+                        hasNewItems.value = false
                     }
+                    appScope.sendReadReceiptIfNeeded(
+                        firstVisibleIndex = event.firstIndex,
+                        timelineItems = timelineItems,
+                        lastReadReceiptIndex = lastReadReceiptIndex,
+                        lastReadReceiptId = lastReadReceiptId
+                    )
                 }
             }
         }
 
+        LaunchedEffect(timelineItems.size) {
+            computeHasNewItems(timelineItems, prevMostRecentItemId, hasNewItems)
+        }
+
         LaunchedEffect(Unit) {
             timeline
                 .timelineItems
@@ -98,10 +111,49 @@ class TimelinePresenter @Inject constructor(
             canReply = userHasPermissionToSendMessage,
             paginationState = paginationState,
             timelineItems = timelineItems,
+            hasNewItems = hasNewItems.value,
             eventSink = ::handleEvents
         )
     }
 
+    /**
+     * This method compute the hasNewItem state passed as a [MutableState] each time the timeline items size changes.
+     * Basically, if we got new timeline event from sync or local, either from us or another user, we update the state so we tell we have new items.
+     * The state never goes back to false from this method, but need to be reset from somewhere else.
+     */
+    private suspend fun computeHasNewItems(
+        timelineItems: ImmutableList<TimelineItem>,
+        prevMostRecentItemId: MutableState<String?>,
+        hasNewItemsState: MutableState<Boolean>
+    ) = withContext(dispatchers.computation) {
+        val newMostRecentItem = timelineItems.firstOrNull()
+        val prevMostRecentItemIdValue = prevMostRecentItemId.value
+        val newMostRecentItemId = newMostRecentItem?.identifier()
+        val hasNewItems = prevMostRecentItemIdValue != null &&
+            newMostRecentItem is TimelineItem.Event &&
+            newMostRecentItem.origin != TimelineItemEventOrigin.PAGINATION &&
+            newMostRecentItemId != prevMostRecentItemIdValue
+        if (hasNewItems) {
+            hasNewItemsState.value = true
+        }
+        prevMostRecentItemId.value = newMostRecentItemId
+    }
+
+    private fun CoroutineScope.sendReadReceiptIfNeeded(
+        firstVisibleIndex: Int,
+        timelineItems: ImmutableList<TimelineItem>,
+        lastReadReceiptIndex: MutableState<Int>,
+        lastReadReceiptId: MutableState<EventId?>,
+    ) = launch(dispatchers.computation) {
+        // Get last valid EventId seen by the user, as the first index might refer to a Virtual item
+        val eventId = getLastEventIdBeforeOrAt(firstVisibleIndex, timelineItems)
+        if (eventId != null && firstVisibleIndex <= lastReadReceiptIndex.value && eventId != lastReadReceiptId.value) {
+            lastReadReceiptIndex.value = firstVisibleIndex
+            lastReadReceiptId.value = eventId
+            timeline.sendReadReceipt(eventId)
+        }
+    }
+
     private fun getLastEventIdBeforeOrAt(index: Int, items: ImmutableList<TimelineItem>): EventId? {
         for (item in items.subList(index, items.count())) {
             if (item is TimelineItem.Event) {
@@ -114,8 +166,4 @@ class TimelinePresenter @Inject constructor(
     private fun CoroutineScope.paginateBackwards() = launch {
         timeline.paginateBackwards(backPaginationEventLimit, backPaginationPageSize)
     }
-
-    private fun CoroutineScope.sendReadReceipt(eventId: EventId) = launch {
-        timeline.sendReadReceipt(eventId)
-    }
 }

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

@@ -28,5 +28,6 @@ data class TimelineState(
     val highlightedEventId: EventId?,
     val canReply: Boolean,
     val paginationState: MatrixTimeline.PaginationState,
+    val hasNewItems: Boolean,
     val eventSink: (TimelineEvents) -> Unit
 )

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

@@ -31,8 +31,8 @@ import io.element.android.libraries.matrix.api.core.TransactionId
 import io.element.android.libraries.matrix.api.core.UserId
 import io.element.android.libraries.matrix.api.timeline.MatrixTimeline
 import io.element.android.libraries.matrix.api.timeline.item.TimelineItemDebugInfo
-import io.element.android.libraries.matrix.api.timeline.item.event.LocalEventSendState
 import io.element.android.libraries.matrix.api.timeline.item.event.InReplyTo
+import io.element.android.libraries.matrix.api.timeline.item.event.LocalEventSendState
 import kotlinx.collections.immutable.ImmutableList
 import kotlinx.collections.immutable.persistentListOf
 import kotlinx.collections.immutable.toImmutableList
@@ -45,7 +45,8 @@ fun aTimelineState(timelineItems: ImmutableList<TimelineItem> = persistentListOf
     paginationState = MatrixTimeline.PaginationState(isBackPaginating = false, hasMoreToLoadBackwards = true),
     highlightedEventId = null,
     canReply = true,
-    eventSink = {}
+    hasNewItems = false,
+    eventSink = {},
 )
 
 internal fun aTimelineItemList(content: TimelineItemEventContent): ImmutableList<TimelineItem> {
@@ -127,6 +128,7 @@ internal fun aTimelineItemEvent(
         localSendState = sendState,
         inReplyTo = inReplyTo,
         debugInfo = debugInfo,
+        origin = null
     )
 }
 
@@ -153,13 +155,14 @@ internal fun aTimelineItemDebugInfo(
     model, originalJson, latestEditedJson
 )
 
-fun aGroupedEvents(): TimelineItem.GroupedEvents {
+fun aGroupedEvents(id: Long = 0): TimelineItem.GroupedEvents {
     val event = aTimelineItemEvent(
         isMine = true,
         content = aTimelineItemStateEventContent(),
         groupPosition = TimelineItemGroupPosition.None
     )
     return TimelineItem.GroupedEvents(
+        id = id.toString(),
         events = listOf(
             event,
             event,

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

@@ -21,6 +21,7 @@ package io.element.android.features.messages.impl.timeline
 import androidx.compose.animation.AnimatedVisibility
 import androidx.compose.animation.ExperimentalAnimationApi
 import androidx.compose.animation.animateContentSize
+import androidx.compose.animation.core.tween
 import androidx.compose.animation.scaleIn
 import androidx.compose.animation.scaleOut
 import androidx.compose.foundation.layout.Box
@@ -48,7 +49,6 @@ import androidx.compose.runtime.rememberCoroutineScope
 import androidx.compose.runtime.saveable.rememberSaveable
 import androidx.compose.ui.Alignment
 import androidx.compose.ui.Modifier
-import androidx.compose.ui.draw.shadow
 import androidx.compose.ui.platform.LocalInspectionMode
 import androidx.compose.ui.res.pluralStringResource
 import androidx.compose.ui.tooling.preview.Preview
@@ -64,7 +64,6 @@ import io.element.android.features.messages.impl.timeline.model.TimelineItem
 import io.element.android.features.messages.impl.timeline.model.event.TimelineItemEventContent
 import io.element.android.features.messages.impl.timeline.model.event.TimelineItemEventContentProvider
 import io.element.android.features.messages.impl.timeline.model.event.TimelineItemStateContent
-import io.element.android.features.messages.impl.timeline.model.virtual.TimelineItemEncryptedHistoryBannerVirtualModel
 import io.element.android.libraries.designsystem.preview.ElementPreviewDark
 import io.element.android.libraries.designsystem.preview.ElementPreviewLight
 import io.element.android.libraries.designsystem.theme.components.FloatingActionButton
@@ -72,9 +71,6 @@ import io.element.android.libraries.designsystem.theme.components.Icon
 import io.element.android.libraries.matrix.api.core.EventId
 import io.element.android.libraries.matrix.api.core.UserId
 import io.element.android.libraries.theme.ElementTheme
-import kotlinx.collections.immutable.ImmutableList
-import kotlinx.collections.immutable.persistentListOf
-import kotlinx.collections.immutable.toPersistentList
 import kotlinx.coroutines.launch
 
 @Composable
@@ -103,13 +99,6 @@ fun TimelineView(
         // TODO implement this logic once we have support to 'jump to event X' in sliding sync
     }
 
-    // Send an event to the presenter when the scrolling is finished, with the first visible index at the bottom.
-    val firstVisibleIndex by remember { derivedStateOf { lazyListState.firstVisibleItemIndex } }
-    val isScrollFinished by remember { derivedStateOf { !lazyListState.isScrollInProgress } }
-    LaunchedEffect(firstVisibleIndex, isScrollFinished) {
-        if (!isScrollFinished) return@LaunchedEffect
-        state.eventSink(TimelineEvents.OnScrollFinished(firstVisibleIndex))
-    }
 
     Box(modifier = modifier) {
         LazyColumn(
@@ -150,8 +139,8 @@ fun TimelineView(
 
         TimelineScrollHelper(
             lazyListState = lazyListState,
-            timelineItems = state.timelineItems,
-            onScrollFinishedAt = ::onScrollFinishedAt,
+            hasNewItems = state.hasNewItems,
+            onScrollFinishedAt = ::onScrollFinishedAt
         )
     }
 }
@@ -247,63 +236,66 @@ fun TimelineItemRow(
 }
 
 @Composable
-internal fun BoxScope.TimelineScrollHelper(
+private fun BoxScope.TimelineScrollHelper(
     lazyListState: LazyListState,
-    timelineItems: ImmutableList<TimelineItem>,
-    onScrollFinishedAt: (Int) -> Unit = {},
+    hasNewItems: Boolean,
+    onScrollFinishedAt: (Int) -> Unit,
 ) {
     val coroutineScope = rememberCoroutineScope()
-    val firstVisibleItemIndex by remember { derivedStateOf { lazyListState.firstVisibleItemIndex } }
     val isScrollFinished by remember { derivedStateOf { !lazyListState.isScrollInProgress } }
-    val shouldAutoScrollToBottom by remember { derivedStateOf { lazyListState.firstVisibleItemIndex < 2 } }
-    val showScrollToBottomButton by remember { derivedStateOf { lazyListState.firstVisibleItemIndex > 0 } }
+    val canAutoScroll by remember { derivedStateOf { lazyListState.firstVisibleItemIndex < 3 } }
 
-    LaunchedEffect(timelineItems, firstVisibleItemIndex) {
-        if (!isScrollFinished) return@LaunchedEffect
-
-        // Auto-scroll when new timeline items appear
-        if (shouldAutoScrollToBottom) {
+    LaunchedEffect(canAutoScroll, hasNewItems) {
+        val shouldAutoScroll = isScrollFinished && canAutoScroll && hasNewItems
+        if (shouldAutoScroll) {
             coroutineScope.launch {
                 lazyListState.animateScrollToItem(0)
             }
         }
     }
-    LaunchedEffect(isScrollFinished) {
-        if (!isScrollFinished) return@LaunchedEffect
 
-        // Notify the parent composable about the first visible item index when scrolling finishes
-        onScrollFinishedAt(firstVisibleItemIndex)
+    LaunchedEffect(isScrollFinished) {
+        if (isScrollFinished) {
+            // Notify the parent composable about the first visible item index when scrolling finishes
+            onScrollFinishedAt(lazyListState.firstVisibleItemIndex)
+        }
     }
 
-    // Jump to bottom button (display also in previews)
-    AnimatedVisibility(
+    JumpToBottomButton(
+        // Use inverse of canAutoScroll otherwise we might briefly see the before the scroll animation is triggered
+        isVisible = !canAutoScroll,
         modifier = Modifier
             .align(Alignment.BottomEnd)
             .padding(end = 24.dp, bottom = 12.dp),
-        visible = showScrollToBottomButton || LocalInspectionMode.current,
-        enter = scaleIn(),
-        exit = scaleOut(),
+        onClick = {
+            coroutineScope.launch {
+                if (lazyListState.firstVisibleItemIndex > 10) {
+                    lazyListState.scrollToItem(0)
+                } else {
+                    lazyListState.animateScrollToItem(0)
+                }
+            }
+        }
+    )
+}
+
+@Composable
+private fun JumpToBottomButton(
+    isVisible: Boolean,
+    onClick: () -> Unit,
+    modifier: Modifier = Modifier,
+) {
+    AnimatedVisibility(
+        modifier = modifier,
+        visible = isVisible || LocalInspectionMode.current,
+        enter = scaleIn(animationSpec = tween(100)),
+        exit = scaleOut(animationSpec = tween(100)),
     ) {
         FloatingActionButton(
-            onClick = {
-                coroutineScope.launch {
-                    if (firstVisibleItemIndex > 10) {
-                        lazyListState.scrollToItem(0)
-                    } else {
-                        lazyListState.animateScrollToItem(0)
-                    }
-                }
-            },
+            onClick = onClick,
             elevation = FloatingActionButtonDefaults.elevation(4.dp, 4.dp, 4.dp, 4.dp),
             shape = CircleShape,
-            modifier = Modifier
-                .shadow(
-                    elevation = 4.dp,
-                    shape = CircleShape,
-                    ambientColor = ElementTheme.materialColors.primary,
-                    spotColor = ElementTheme.materialColors.primary,
-                )
-                .size(36.dp),
+            modifier = Modifier.size(36.dp),
             containerColor = ElementTheme.colors.bgSubtleSecondary,
             contentColor = ElementTheme.colors.iconSecondary
         ) {

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/factories/event/TimelineItemEventFactory.kt

@@ -85,6 +85,7 @@ class TimelineItemEventFactory @Inject constructor(
             localSendState = currentTimelineItem.event.localSendState,
             inReplyTo = currentTimelineItem.event.inReplyTo(),
             debugInfo = currentTimelineItem.event.debugInfo,
+            origin = currentTimelineItem.event.origin,
         )
     }
 

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

@@ -16,11 +16,22 @@
 
 package io.element.android.features.messages.impl.timeline.groups
 
+import androidx.annotation.VisibleForTesting
 import io.element.android.features.messages.impl.timeline.model.TimelineItem
+import io.element.android.libraries.di.RoomScope
+import io.element.android.libraries.di.SingleIn
 import kotlinx.collections.immutable.toImmutableList
 import javax.inject.Inject
 
+@SingleIn(RoomScope::class)
 class TimelineItemGrouper @Inject constructor() {
+
+    /**
+     * Keys are identifier of items in a group, only one by group will be kept.
+     * Values are the actual groupIds.
+     */
+    private val groupIds = HashMap<String, String>()
+
     /**
      * Create a new list of [TimelineItem] by grouping some of them into [TimelineItem.GroupedEvents].
      */
@@ -34,14 +45,14 @@ class TimelineItemGrouper @Inject constructor() {
                 // timelineItem cannot be grouped
                 if (currentGroup.isNotEmpty()) {
                     // There is a pending group, create a TimelineItem.GroupedEvents if there is more than 1 Event in the pending group.
-                    result.addGroup(currentGroup)
+                    result.addGroup(groupIds, currentGroup)
                     currentGroup.clear()
                 }
                 result.add(timelineItem)
             }
         }
         if (currentGroup.isNotEmpty()) {
-            result.addGroup(currentGroup)
+            result.addGroup(groupIds, currentGroup)
         }
         return result
     }
@@ -51,16 +62,36 @@ class TimelineItemGrouper @Inject constructor() {
  * Will add a group if there is more than 1 item, else add the item to the list.
  */
 private fun MutableList<TimelineItem>.addGroup(
-    group: MutableList<TimelineItem.Event>
+    groupIds: MutableMap<String, String>,
+    groupOfItems: MutableList<TimelineItem.Event>
 ) {
-    if (group.size == 1) {
+    if (groupOfItems.size == 1) {
         // Do not create a group with just 1 item, just add the item to the result
-        add(group.first())
+        add(groupOfItems.first())
     } else {
+        val groupId = groupIds.getOrPutGroupId(groupOfItems)
         add(
             TimelineItem.GroupedEvents(
-                events = group.toImmutableList()
+                id = groupId,
+                events = groupOfItems.toImmutableList()
             )
         )
     }
 }
+
+private fun MutableMap<String, String>.getOrPutGroupId(timelineItems: List<TimelineItem>): String {
+    assert(timelineItems.isNotEmpty())
+    for (item in timelineItems) {
+        val itemIdentifier = item.identifier()
+        if (this.contains(itemIdentifier)) {
+            return this[itemIdentifier]!!
+        }
+    }
+    val timelineItem = timelineItems.first()
+    return computeGroupIdWith(timelineItem).also { groupId ->
+        this[timelineItem.identifier()] = groupId
+    }
+}
+
+@VisibleForTesting
+internal fun computeGroupIdWith(timelineItem: TimelineItem): String = "${timelineItem.identifier()}_group"

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

@@ -27,6 +27,7 @@ import io.element.android.libraries.matrix.api.core.UserId
 import io.element.android.libraries.matrix.api.timeline.item.TimelineItemDebugInfo
 import io.element.android.libraries.matrix.api.timeline.item.event.InReplyTo
 import io.element.android.libraries.matrix.api.timeline.item.event.LocalEventSendState
+import io.element.android.libraries.matrix.api.timeline.item.event.TimelineItemEventOrigin
 import kotlinx.collections.immutable.ImmutableList
 
 @Immutable
@@ -66,6 +67,7 @@ sealed interface TimelineItem {
         val localSendState: LocalEventSendState?,
         val inReplyTo: InReplyTo?,
         val debugInfo: TimelineItemDebugInfo,
+        val origin: TimelineItemEventOrigin?,
     ) : TimelineItem {
 
         val showSenderInformation = groupPosition.isNew() && !isMine
@@ -81,9 +83,8 @@ sealed interface TimelineItem {
 
     @Immutable
     data class GroupedEvents(
+        val id: String,
         val events: ImmutableList<Event>,
-    ) : TimelineItem {
-        // use last id with a suffix. Last will not change in cas of new event from backpagination.
-        val id = "${events.last().id}_group"
-    }
+    ) : TimelineItem
+
 }

features/messages/impl/src/test/kotlin/io/element/android/features/messages/MessagesPresenterTest.kt

@@ -574,6 +574,8 @@ class MessagesPresenterTest {
         val timelinePresenter = TimelinePresenter(
             timelineItemsFactory = aTimelineItemsFactory(),
             room = matrixRoom,
+            dispatchers = coroutineDispatchers,
+            appScope = this
         )
         val buildMeta = aBuildMeta()
         val actionListPresenter = ActionListPresenter(buildMeta = buildMeta)

features/messages/impl/src/test/kotlin/io/element/android/features/messages/fixtures/aMessageEvent.kt

@@ -52,4 +52,5 @@ internal fun aMessageEvent(
     localSendState = sendState,
     inReplyTo = inReplyTo,
     debugInfo = debugInfo,
+    origin = null
 )

features/messages/impl/src/test/kotlin/io/element/android/features/messages/timeline/TimelinePresenterTest.kt

@@ -23,22 +23,25 @@ import com.google.common.truth.Truth.assertThat
 import io.element.android.features.messages.fixtures.aTimelineItemsFactory
 import io.element.android.features.messages.impl.timeline.TimelineEvents
 import io.element.android.features.messages.impl.timeline.TimelinePresenter
+import io.element.android.features.messages.impl.timeline.factories.TimelineItemsFactory
+import io.element.android.libraries.matrix.api.timeline.MatrixTimeline
 import io.element.android.libraries.matrix.api.timeline.MatrixTimelineItem
 import io.element.android.libraries.matrix.api.timeline.item.virtual.VirtualTimelineItem
 import io.element.android.libraries.matrix.test.AN_EVENT_ID
 import io.element.android.libraries.matrix.test.room.FakeMatrixRoom
+import io.element.android.libraries.matrix.test.room.aMessageContent
 import io.element.android.libraries.matrix.test.room.anEventTimelineItem
 import io.element.android.libraries.matrix.test.timeline.FakeMatrixTimeline
+import io.element.android.tests.testutils.awaitWithLatch
+import io.element.android.tests.testutils.testCoroutineDispatchers
+import kotlinx.coroutines.test.TestScope
 import kotlinx.coroutines.test.runTest
 import org.junit.Test
 
 class TimelinePresenterTest {
     @Test
     fun `present - initial state`() = runTest {
-        val presenter = TimelinePresenter(
-            timelineItemsFactory = aTimelineItemsFactory(),
-            room = FakeMatrixRoom(),
-        )
+        val presenter = createTimelinePresenter()
         moleculeFlow(RecompositionClock.Immediate) {
             presenter.present()
         }.test {
@@ -51,10 +54,7 @@ class TimelinePresenterTest {
 
     @Test
     fun `present - load more`() = runTest {
-        val presenter = TimelinePresenter(
-            timelineItemsFactory = aTimelineItemsFactory(),
-            room = FakeMatrixRoom(),
-        )
+        val presenter = createTimelinePresenter()
         moleculeFlow(RecompositionClock.Immediate) {
             presenter.present()
         }.test {
@@ -73,10 +73,7 @@ class TimelinePresenterTest {
 
     @Test
     fun `present - set highlighted event`() = runTest {
-        val presenter = TimelinePresenter(
-            timelineItemsFactory = aTimelineItemsFactory(),
-            room = FakeMatrixRoom(),
-        )
+        val presenter = createTimelinePresenter()
         moleculeFlow(RecompositionClock.Immediate) {
             presenter.present()
         }.test {
@@ -94,70 +91,112 @@ class TimelinePresenterTest {
 
     @Test
     fun `present - on scroll finished send read receipt if an event is before the index`() = runTest {
-        val timeline = FakeMatrixTimeline()
-        val timelineItemsFactory = aTimelineItemsFactory().apply {
-            replaceWith(listOf(MatrixTimelineItem.Event(0, anEventTimelineItem())))
-        }
-        val room = FakeMatrixRoom(matrixTimeline = timeline)
-        val presenter = TimelinePresenter(
-            timelineItemsFactory = timelineItemsFactory,
-            room = room,
+        val timeline = FakeMatrixTimeline(
+            initialTimelineItems = listOf(
+                MatrixTimelineItem.Event(0, anEventTimelineItem())
+            )
         )
+        val presenter = createTimelinePresenter(timeline)
         moleculeFlow(RecompositionClock.Immediate) {
             presenter.present()
         }.test {
             assertThat(timeline.sendReadReceiptCount).isEqualTo(0)
             val initialState = awaitItem()
-
-            initialState.eventSink.invoke(TimelineEvents.OnScrollFinished(0))
-
+            // Wait for timeline items to be populated
+            skipItems(1)
+            awaitWithLatch { latch ->
+                timeline.sendReadReceiptLatch = latch
+                initialState.eventSink.invoke(TimelineEvents.OnScrollFinished(0))
+            }
             assertThat(timeline.sendReadReceiptCount).isEqualTo(1)
+            cancelAndIgnoreRemainingEvents()
         }
     }
 
     @Test
     fun `present - on scroll finished will not send read receipt no event is before the index`() = runTest {
-        val timeline = FakeMatrixTimeline()
-        val timelineItemsFactory = aTimelineItemsFactory().apply {
-            replaceWith(listOf(MatrixTimelineItem.Event(0, anEventTimelineItem())))
-        }
-        val room = FakeMatrixRoom(matrixTimeline = timeline)
-        val presenter = TimelinePresenter(
-            timelineItemsFactory = timelineItemsFactory,
-            room = room,
+        val timeline = FakeMatrixTimeline(
+            initialTimelineItems = listOf(
+                MatrixTimelineItem.Event(0, anEventTimelineItem())
+            )
         )
+        val presenter = createTimelinePresenter(timeline)
         moleculeFlow(RecompositionClock.Immediate) {
             presenter.present()
         }.test {
             assertThat(timeline.sendReadReceiptCount).isEqualTo(0)
             val initialState = awaitItem()
-
-            initialState.eventSink.invoke(TimelineEvents.OnScrollFinished(1))
-
+            // Wait for timeline items to be populated
+            skipItems(1)
+            awaitWithLatch { latch ->
+                timeline.sendReadReceiptLatch = latch
+                initialState.eventSink.invoke(TimelineEvents.OnScrollFinished(1))
+            }
             assertThat(timeline.sendReadReceiptCount).isEqualTo(0)
+            cancelAndIgnoreRemainingEvents()
         }
     }
 
     @Test
     fun `present - on scroll finished will not send read receipt only virtual events exist before the index`() = runTest {
-        val timeline = FakeMatrixTimeline()
-        val timelineItemsFactory = aTimelineItemsFactory().apply {
-            replaceWith(listOf(MatrixTimelineItem.Virtual(0, VirtualTimelineItem.ReadMarker)))
-        }
-        val room = FakeMatrixRoom(matrixTimeline = timeline)
-        val presenter = TimelinePresenter(
-            timelineItemsFactory = timelineItemsFactory,
-            room = room,
+        val timeline = FakeMatrixTimeline(
+            initialTimelineItems = listOf(
+                MatrixTimelineItem.Virtual(0, VirtualTimelineItem.ReadMarker)
+            )
         )
+        val presenter = createTimelinePresenter(timeline)
         moleculeFlow(RecompositionClock.Immediate) {
             presenter.present()
         }.test {
             assertThat(timeline.sendReadReceiptCount).isEqualTo(0)
             val initialState = awaitItem()
-
-            initialState.eventSink.invoke(TimelineEvents.OnScrollFinished(1))
-
+            // Wait for timeline items to be populated
+            skipItems(1)
+            awaitWithLatch { latch ->
+                timeline.sendReadReceiptLatch = latch
+                initialState.eventSink.invoke(TimelineEvents.OnScrollFinished(0))
+            }
             assertThat(timeline.sendReadReceiptCount).isEqualTo(0)
+            cancelAndIgnoreRemainingEvents()
         }
     }
+
+    @Test
+    fun `present - covers hasNewItems scenarios`() = runTest {
+        val timeline = FakeMatrixTimeline()
+        val presenter = createTimelinePresenter(timeline)
+        moleculeFlow(RecompositionClock.Immediate) {
+            presenter.present()
+        }.test {
+            val initialState = awaitItem()
+            assertThat(initialState.hasNewItems).isFalse()
+            assertThat(initialState.timelineItems.size).isEqualTo(0)
+            timeline.updateTimelineItems {
+                listOf(MatrixTimelineItem.Event(0, anEventTimelineItem(content = aMessageContent())))
+            }
+            skipItems(1)
+            assertThat(awaitItem().timelineItems.size).isEqualTo(1)
+            timeline.updateTimelineItems { items ->
+                items + listOf(MatrixTimelineItem.Event(1, anEventTimelineItem(content = aMessageContent())))
+            }
+            skipItems(1)
+            assertThat(awaitItem().timelineItems.size).isEqualTo(2)
+            assertThat(awaitItem().hasNewItems).isTrue()
+            initialState.eventSink.invoke(TimelineEvents.OnScrollFinished(0))
+            assertThat(awaitItem().hasNewItems).isFalse()
+            cancelAndIgnoreRemainingEvents()
+        }
+    }
+
+    private fun TestScope.createTimelinePresenter(
+        timeline: MatrixTimeline = FakeMatrixTimeline(),
+        timelineItemsFactory: TimelineItemsFactory = aTimelineItemsFactory()
+    ): TimelinePresenter {
+        return TimelinePresenter(
+            timelineItemsFactory = timelineItemsFactory,
+            room = FakeMatrixRoom(matrixTimeline = timeline),
+            dispatchers = testCoroutineDispatchers(),
+            appScope = this
+        )
+    }
 }

features/messages/impl/src/test/kotlin/io/element/android/features/messages/timeline/groups/TimelineItemGrouperTest.kt

@@ -20,13 +20,13 @@ import com.google.common.truth.Truth.assertThat
 import io.element.android.features.messages.fixtures.aMessageEvent
 import io.element.android.features.messages.impl.timeline.aTimelineItemReactions
 import io.element.android.features.messages.impl.timeline.groups.TimelineItemGrouper
+import io.element.android.features.messages.impl.timeline.groups.computeGroupIdWith
 import io.element.android.features.messages.impl.timeline.model.TimelineItem
 import io.element.android.features.messages.impl.timeline.model.event.TimelineItemStateEventContent
 import io.element.android.features.messages.impl.timeline.model.virtual.aTimelineItemDaySeparatorModel
 import io.element.android.libraries.designsystem.components.avatar.anAvatarData
 import io.element.android.libraries.matrix.api.timeline.item.event.LocalEventSendState
 import io.element.android.libraries.matrix.test.AN_EVENT_ID
-import io.element.android.libraries.matrix.test.AN_EVENT_ID_2
 import io.element.android.libraries.matrix.test.A_USER_ID
 import io.element.android.libraries.matrix.test.room.aTimelineItemDebugInfo
 import kotlinx.collections.immutable.toImmutableList
@@ -36,7 +36,7 @@ class TimelineItemGrouperTest {
     private val sut = TimelineItemGrouper()
 
     private val aGroupableItem = TimelineItem.Event(
-        id = AN_EVENT_ID.value,
+        id = "0",
         senderId = A_USER_ID,
         senderAvatar = anAvatarData(),
         senderDisplayName = "",
@@ -45,6 +45,7 @@ class TimelineItemGrouperTest {
         localSendState = LocalEventSendState.Sent(AN_EVENT_ID),
         inReplyTo = null,
         debugInfo = aTimelineItemDebugInfo(),
+        origin = null
     )
     private val aNonGroupableItem = aMessageEvent()
     private val aNonGroupableItemNoEvent = TimelineItem.Virtual("virtual", aTimelineItemDaySeparatorModel("Today"))
@@ -75,16 +76,17 @@ class TimelineItemGrouperTest {
     fun `test groupables and ensure reordering`() {
         val result = sut.group(
             listOf(
-                aGroupableItem.copy(id = AN_EVENT_ID_2.value),
-                aGroupableItem,
+                aGroupableItem.copy(id = "1"),
+                aGroupableItem.copy(id = "0"),
             ),
         )
         assertThat(result).isEqualTo(
             listOf(
                 TimelineItem.GroupedEvents(
+                    computeGroupIdWith(aGroupableItem),
                     events = listOf(
-                        aGroupableItem,
-                        aGroupableItem.copy(id = AN_EVENT_ID_2.value),
+                        aGroupableItem.copy("0"),
+                        aGroupableItem.copy(id = "1"),
                     ).toImmutableList()
                 ),
             )
@@ -127,6 +129,7 @@ class TimelineItemGrouperTest {
         assertThat(result).isEqualTo(
             listOf(
                 TimelineItem.GroupedEvents(
+                    computeGroupIdWith(aGroupableItem),
                     events = listOf(
                         aGroupableItem,
                         aGroupableItem,
@@ -134,6 +137,7 @@ class TimelineItemGrouperTest {
                 ),
                 aNonGroupableItem,
                 TimelineItem.GroupedEvents(
+                    computeGroupIdWith(aGroupableItem),
                     events = listOf(
                         aGroupableItem,
                         aGroupableItem,
@@ -143,4 +147,20 @@ class TimelineItemGrouperTest {
             )
         )
     }
+
+    @Test
+    fun `when calling multiple time the method group over a growing list of groupable items, then groupId is stable`() {
+        // When
+        val groupableItems = mutableListOf(
+            aGroupableItem.copy(id = "1"),
+            aGroupableItem.copy(id = "2")
+        )
+        val expectedGroupId = sut.group(groupableItems).first().identifier()
+        groupableItems.add(0, aGroupableItem.copy("3"))
+        groupableItems.add(2, aGroupableItem.copy("4"))
+        groupableItems.add(aGroupableItem.copy("5"))
+        val actualGroupId = sut.group(groupableItems).first().identifier()
+        // Then
+        assertThat(actualGroupId).isEqualTo(expectedGroupId)
+    }
 }

libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/timeline/item/event/EventTimelineItem.kt

@@ -35,6 +35,7 @@ data class EventTimelineItem(
     val timestamp: Long,
     val content: EventContent,
     val debugInfo: TimelineItemDebugInfo,
+    val origin: TimelineItemEventOrigin?,
 ) {
     fun inReplyTo(): InReplyTo? {
         return (content as? MessageContent)?.inReplyTo

libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/timeline/item/event/TimelineItemEventOrigin.kt

@@ -0,0 +1,21 @@
+/*
+ * Copyright (c) 2023 New Vector Ltd
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package io.element.android.libraries.matrix.api.timeline.item.event
+
+enum class TimelineItemEventOrigin {
+    LOCAL, SYNC, PAGINATION;
+}

libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/item/event/EventTimelineItemMapper.kt

@@ -20,11 +20,13 @@ import io.element.android.libraries.matrix.api.core.EventId
 import io.element.android.libraries.matrix.api.core.TransactionId
 import io.element.android.libraries.matrix.api.core.UserId
 import io.element.android.libraries.matrix.api.timeline.item.TimelineItemDebugInfo
+import io.element.android.libraries.matrix.api.timeline.item.event.TimelineItemEventOrigin
 import io.element.android.libraries.matrix.api.timeline.item.event.EventReaction
 import io.element.android.libraries.matrix.api.timeline.item.event.EventTimelineItem
 import io.element.android.libraries.matrix.api.timeline.item.event.LocalEventSendState
 import io.element.android.libraries.matrix.api.timeline.item.event.ProfileTimelineDetails
 import org.matrix.rustcomponents.sdk.Reaction
+import org.matrix.rustcomponents.sdk.EventItemOrigin as RustEventItemOrigin
 import org.matrix.rustcomponents.sdk.EventSendState as RustEventSendState
 import org.matrix.rustcomponents.sdk.EventTimelineItem as RustEventTimelineItem
 import org.matrix.rustcomponents.sdk.EventTimelineItemDebugInfo as RustEventTimelineItemDebugInfo
@@ -47,6 +49,7 @@ class EventTimelineItemMapper(private val contentMapper: TimelineEventContentMap
             timestamp = it.timestamp().toLong(),
             content = contentMapper.map(it.content()),
             debugInfo = it.debugInfo().map(),
+            origin = it.origin()?.map()
         )
     }
 }
@@ -91,3 +94,11 @@ private fun RustEventTimelineItemDebugInfo.map(): TimelineItemDebugInfo {
         latestEditedJson = latestEditJson,
     )
 }
+
+private fun RustEventItemOrigin.map(): TimelineItemEventOrigin {
+    return when (this) {
+        RustEventItemOrigin.LOCAL -> TimelineItemEventOrigin.LOCAL
+        RustEventItemOrigin.SYNC -> TimelineItemEventOrigin.SYNC
+        RustEventItemOrigin.PAGINATION -> TimelineItemEventOrigin.PAGINATION
+    }
+}

libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/room/RoomSummaryFixture.kt

@@ -26,14 +26,17 @@ import io.element.android.libraries.matrix.api.room.message.RoomMessage
 import io.element.android.libraries.matrix.api.timeline.item.TimelineItemDebugInfo
 import io.element.android.libraries.matrix.api.timeline.item.event.EventContent
 import io.element.android.libraries.matrix.api.timeline.item.event.EventReaction
-import io.element.android.libraries.matrix.api.timeline.item.event.LocalEventSendState
 import io.element.android.libraries.matrix.api.timeline.item.event.EventTimelineItem
+import io.element.android.libraries.matrix.api.timeline.item.event.InReplyTo
+import io.element.android.libraries.matrix.api.timeline.item.event.LocalEventSendState
+import io.element.android.libraries.matrix.api.timeline.item.event.MessageContent
+import io.element.android.libraries.matrix.api.timeline.item.event.MessageType
 import io.element.android.libraries.matrix.api.timeline.item.event.ProfileChangeContent
 import io.element.android.libraries.matrix.api.timeline.item.event.ProfileTimelineDetails
+import io.element.android.libraries.matrix.api.timeline.item.event.TextMessageType
 import io.element.android.libraries.matrix.test.AN_EVENT_ID
 import io.element.android.libraries.matrix.test.A_ROOM_ID
 import io.element.android.libraries.matrix.test.A_ROOM_NAME
-import io.element.android.libraries.matrix.test.A_UNIQUE_ID
 import io.element.android.libraries.matrix.test.A_USER_ID
 import io.element.android.libraries.matrix.test.A_USER_NAME
 
@@ -115,6 +118,7 @@ fun anEventTimelineItem(
     timestamp = timestamp,
     content = content,
     debugInfo = debugInfo,
+    origin = null,
 )
 
 fun aProfileTimelineDetails(
@@ -139,6 +143,21 @@ fun aProfileChangeMessageContent(
     prevAvatarUrl = prevAvatarUrl,
 )
 
+fun aMessageContent(
+    body: String = "body",
+    inReplyTo: InReplyTo? = null,
+    isEdited: Boolean = false,
+    messageType: MessageType = TextMessageType(
+        body = body,
+        formatted = null
+    )
+) = MessageContent(
+    body = body,
+    inReplyTo = inReplyTo,
+    isEdited = isEdited,
+    type = messageType
+)
+
 fun aTimelineItemDebugInfo(
     model: String = "Rust(Model())",
     originalJson: String? = null,
@@ -146,3 +165,4 @@ fun aTimelineItemDebugInfo(
 ) = TimelineItemDebugInfo(
     model, originalJson, latestEditedJson
 )
+

libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/timeline/FakeMatrixTimeline.kt

@@ -19,6 +19,8 @@ package io.element.android.libraries.matrix.test.timeline
 import io.element.android.libraries.matrix.api.core.EventId
 import io.element.android.libraries.matrix.api.timeline.MatrixTimeline
 import io.element.android.libraries.matrix.api.timeline.MatrixTimelineItem
+import io.element.android.tests.testutils.simulateLongTask
+import kotlinx.coroutines.CompletableDeferred
 import kotlinx.coroutines.delay
 import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.MutableStateFlow
@@ -36,6 +38,8 @@ class FakeMatrixTimeline(
     var sendReadReceiptCount = 0
         private set
 
+    var sendReadReceiptLatch: CompletableDeferred<Unit>? = null
+
     fun updatePaginationState(update: (MatrixTimeline.PaginationState.() -> MatrixTimeline.PaginationState)) {
         _paginationState.getAndUpdate(update)
     }
@@ -62,13 +66,13 @@ class FakeMatrixTimeline(
         return Result.success(Unit)
     }
 
-
-    override suspend fun fetchDetailsForEvent(eventId: EventId): Result<Unit> {
-        return Result.success(Unit)
+    override suspend fun fetchDetailsForEvent(eventId: EventId): Result<Unit> = simulateLongTask {
+        Result.success(Unit)
     }
 
-    override suspend fun sendReadReceipt(eventId: EventId): Result<Unit> {
+    override suspend fun sendReadReceipt(eventId: EventId): Result<Unit> = simulateLongTask {
         sendReadReceiptCount++
-        return Result.success(Unit)
+        sendReadReceiptLatch?.complete(Unit)
+        Result.success(Unit)
     }
 }

tests/testutils/src/main/kotlin/io/element/android/tests/testutils/LongTask.kt

@@ -16,7 +16,12 @@
 
 package io.element.android.tests.testutils
 
+import kotlinx.coroutines.CompletableDeferred
+import kotlinx.coroutines.TimeoutCancellationException
 import kotlinx.coroutines.delay
+import kotlinx.coroutines.withTimeout
+import kotlin.time.Duration
+import kotlin.time.Duration.Companion.milliseconds
 
 /**
  * Workaround for https://github.com/cashapp/molecule/issues/249.
@@ -26,3 +31,18 @@ suspend inline fun <T> simulateLongTask(lambda: () -> T): T {
     delay(1)
     return lambda()
 }
+
+/**
+ * Can be used for testing events in Presenter, where the event does not emit new state.
+ * If the (virtual) timeout is passed, we release the latch manually.
+ */
+suspend fun awaitWithLatch(timeout: Duration = 300.milliseconds, block: (CompletableDeferred<Unit>) -> Unit) {
+    val latch = CompletableDeferred<Unit>()
+    try {
+        withTimeout(timeout) {
+            latch.also(block).await()
+        }
+    } catch (exception: TimeoutCancellationException) {
+        latch.complete(Unit)
+    }
+}