Timeline : Scroll to end of timeline when sending a new message #1877

2023-11-24 11:29:50 +01:00
parent 2af7fe4e9c
commit fe1a0316a8
7 changed files with 107 additions and 38 deletions
--- a/changelog.d/1877.feature
+++ b/changelog.d/1877.feature
@@ -0,0 +1 @@
+Scroll to end of timeline when sending a new message.
--- 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
@@ -30,6 +30,7 @@ import androidx.compose.runtime.saveable.rememberSaveable
 import im.vector.app.features.analytics.plan.PollEnd
 import im.vector.app.features.analytics.plan.PollVote
 import io.element.android.features.messages.impl.timeline.factories.TimelineItemsFactory
+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.session.SessionState
 import io.element.android.features.messages.impl.voicemessages.timeline.RedactedVoiceMessageManager
@@ -89,7 +90,7 @@ class TimelinePresenter @Inject constructor(
        val userHasPermissionToSendMessage by room.canSendMessageAsState(type = MessageEventType.ROOM_MESSAGE, updateKey = syncUpdateFlow.value)

        val prevMostRecentItemId = rememberSaveable { mutableStateOf<String?>(null) }
-        val hasNewItems = remember { mutableStateOf(false) }
+        val newItemState = remember { mutableStateOf(NewEventState.None) }

        val sessionVerifiedStatus by verificationService.sessionVerifiedStatus.collectAsState()
        val keyBackupState by encryptionService.backupStateStateFlow.collectAsState()
@@ -112,7 +113,7 @@ class TimelinePresenter @Inject constructor(
                is TimelineEvents.SetHighlightedEvent -> highlightedEventId.value = event.eventId
                is TimelineEvents.OnScrollFinished -> {
                    if (event.firstIndex == 0) {
-                        hasNewItems.value = false
+                        newItemState.value = NewEventState.None
                    }
                    appScope.sendReadReceiptIfNeeded(
                        firstVisibleIndex = event.firstIndex,
@@ -139,7 +140,7 @@ class TimelinePresenter @Inject constructor(
        }

        LaunchedEffect(timelineItems.size) {
-            computeHasNewItems(timelineItems, prevMostRecentItemId, hasNewItems)
+            computeNewItemState(timelineItems, prevMostRecentItemId, newItemState)
        }

        LaunchedEffect(Unit) {
@@ -171,7 +172,7 @@ class TimelinePresenter @Inject constructor(
            paginationState = paginationState,
            timelineItems = timelineItems,
            showReadReceipts = readReceiptsEnabled,
-            hasNewItems = hasNewItems.value,
+            newEventState = newItemState.value,
            sessionState = sessionState,
            eventSink = ::handleEvents
        )
@@ -180,22 +181,32 @@ class TimelinePresenter @Inject constructor(
    /**
     * 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.
+     * The state never goes back to None from this method, but need to be reset from somewhere else.
     */
-    private suspend fun computeHasNewItems(
+    private suspend fun computeNewItemState(
        timelineItems: ImmutableList<TimelineItem>,
        prevMostRecentItemId: MutableState<String?>,
-        hasNewItemsState: MutableState<Boolean>
+        newEventState: MutableState<NewEventState>
    ) = withContext(dispatchers.computation) {
+        // FromMe is prioritized over FromOther, so skip if we already have a FromMe
+        if (newEventState.value == NewEventState.FromMe) {
+            return@withContext
+        }
        val newMostRecentItem = timelineItems.firstOrNull()
        val prevMostRecentItemIdValue = prevMostRecentItemId.value
        val newMostRecentItemId = newMostRecentItem?.identifier()
-        val hasNewItems = prevMostRecentItemIdValue != null &&
+        val hasNewEvent = prevMostRecentItemIdValue != null &&
            newMostRecentItem is TimelineItem.Event &&
            newMostRecentItem.origin != TimelineItemEventOrigin.PAGINATION &&
            newMostRecentItemId != prevMostRecentItemIdValue
-        if (hasNewItems) {
-            hasNewItemsState.value = true
+        if (hasNewEvent) {
+            val newMostRecentEvent = newMostRecentItem as? TimelineItem.Event
+            val fromMe = newMostRecentEvent?.localSendState != null
+            newEventState.value = if (fromMe) {
+                NewEventState.FromMe
+            } else {
+                NewEventState.FromOther
+            }
        }
        prevMostRecentItemId.value = newMostRecentItemId
    }
--- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineState.kt
+++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineState.kt
@@ -17,6 +17,7 @@
 package io.element.android.features.messages.impl.timeline

 import androidx.compose.runtime.Immutable
+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.session.SessionState
 import io.element.android.libraries.matrix.api.core.EventId
@@ -30,7 +31,7 @@ data class TimelineState(
    val highlightedEventId: EventId?,
    val userHasPermissionToSendMessage: Boolean,
    val paginationState: MatrixTimeline.PaginationState,
-    val hasNewItems: Boolean,
+    val newEventState: NewEventState,
    val sessionState: SessionState,
    val eventSink: (TimelineEvents) -> Unit
 )
--- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineStateProvider.kt
+++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineStateProvider.kt
@@ -17,6 +17,7 @@
 package io.element.android.features.messages.impl.timeline

 import io.element.android.features.messages.impl.timeline.model.InReplyToDetails
+import io.element.android.features.messages.impl.timeline.model.NewEventState
 import io.element.android.features.messages.impl.timeline.model.ReadReceiptData
 import io.element.android.features.messages.impl.timeline.model.TimelineItem
 import io.element.android.features.messages.impl.timeline.model.TimelineItemGroupPosition
@@ -53,7 +54,7 @@ fun aTimelineState(timelineItems: ImmutableList<TimelineItem> = persistentListOf
    ),
    highlightedEventId = null,
    userHasPermissionToSendMessage = true,
-    hasNewItems = false,
+    newEventState = NewEventState.None,
    sessionState = aSessionState(
        isSessionVerified = true,
        isKeyBackupEnabled = true,
--- 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
@@ -64,6 +64,7 @@ import io.element.android.features.messages.impl.timeline.components.virtual.Tim
 import io.element.android.features.messages.impl.timeline.components.virtual.TimelineLoadingMoreIndicator
 import io.element.android.features.messages.impl.timeline.di.LocalTimelineItemPresenterFactories
 import io.element.android.features.messages.impl.timeline.di.aFakeTimelineItemPresenterFactories
+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.event.TimelineItemEventContent
 import io.element.android.features.messages.impl.timeline.model.event.TimelineItemEventContentProvider
@@ -167,7 +168,7 @@ fun TimelineView(
        TimelineScrollHelper(
            isTimelineEmpty = state.timelineItems.isEmpty(),
            lazyListState = lazyListState,
-            hasNewItems = state.hasNewItems,
+            newEventState = state.newEventState,
            onScrollFinishedAt = ::onScrollFinishedAt
        )
    }
@@ -286,22 +287,34 @@ private fun TimelineItemRow(
 private fun BoxScope.TimelineScrollHelper(
    isTimelineEmpty: Boolean,
    lazyListState: LazyListState,
-    hasNewItems: Boolean,
+    newEventState: NewEventState,
    onScrollFinishedAt: (Int) -> Unit,
 ) {
    val coroutineScope = rememberCoroutineScope()
    val isScrollFinished by remember { derivedStateOf { !lazyListState.isScrollInProgress } }
-    val canAutoScroll by remember { derivedStateOf { lazyListState.firstVisibleItemIndex < 3 } }
+    val canAutoScroll by remember {
+        derivedStateOf {
+            lazyListState.firstVisibleItemIndex < 3
+        }
+    }

-    LaunchedEffect(canAutoScroll, hasNewItems) {
-        val shouldAutoScroll = isScrollFinished && canAutoScroll && hasNewItems
-        if (shouldAutoScroll) {
-            coroutineScope.launch {
+    fun scrollToBottom() {
+        coroutineScope.launch {
+            if (lazyListState.firstVisibleItemIndex > 10) {
+                lazyListState.scrollToItem(0)
+            } else {
                lazyListState.animateScrollToItem(0)
            }
        }
    }

+    LaunchedEffect(canAutoScroll, newEventState) {
+        val shouldAutoScroll = isScrollFinished && (canAutoScroll || newEventState == NewEventState.FromMe)
+        if (shouldAutoScroll) {
+            scrollToBottom()
+        }
+    }
+
    LaunchedEffect(isScrollFinished, isTimelineEmpty) {
        if (isScrollFinished && !isTimelineEmpty) {
            // Notify the parent composable about the first visible item index when scrolling finishes
@@ -315,15 +328,7 @@ private fun BoxScope.TimelineScrollHelper(
        modifier = Modifier
            .align(Alignment.BottomEnd)
            .padding(end = 24.dp, bottom = 12.dp),
-        onClick = {
-            coroutineScope.launch {
-                if (lazyListState.firstVisibleItemIndex > 10) {
-                    lazyListState.scrollToItem(0)
-                } else {
-                    lazyListState.animateScrollToItem(0)
-                }
-            }
-        }
+        onClick = ::scrollToBottom,
    )
 }

--- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/NewEventState.kt
+++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/NewEventState.kt
@@ -0,0 +1,25 @@
+/*
+ * 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.features.messages.impl.timeline.model
+
+/**
+ * Model if there is a new event in the timeline and if it is from me or from other.
+ * This can be used to scroll to the bottom of the list when a new event is added.
+ */
+enum class NewEventState {
+    None, FromMe, FromOther
+}
--- a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/timeline/TimelinePresenterTest.kt
+++ b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/timeline/TimelinePresenterTest.kt
@@ -25,6 +25,7 @@ import im.vector.app.features.analytics.plan.PollVote
 import io.element.android.features.messages.impl.fixtures.aMessageEvent
 import io.element.android.features.messages.impl.fixtures.aTimelineItemsFactory
 import io.element.android.features.messages.impl.timeline.factories.TimelineItemsFactory
+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.session.SessionState
 import io.element.android.features.messages.impl.voicemessages.timeline.FakeRedactedVoiceMessageManager
@@ -35,6 +36,7 @@ import io.element.android.libraries.matrix.api.room.MatrixRoom
 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.event.EventReaction
+import io.element.android.libraries.matrix.api.timeline.item.event.LocalEventSendState
 import io.element.android.libraries.matrix.api.timeline.item.event.ReactionSender
 import io.element.android.libraries.matrix.api.timeline.item.virtual.VirtualTimelineItem
 import io.element.android.libraries.matrix.test.AN_EVENT_ID
@@ -47,7 +49,9 @@ import io.element.android.libraries.matrix.test.verification.FakeSessionVerifica
 import io.element.android.libraries.matrix.ui.components.aMatrixUserList
 import io.element.android.services.analytics.test.FakeAnalyticsService
 import io.element.android.tests.testutils.WarmUpRule
+import io.element.android.tests.testutils.awaitLastSequentialItem
 import io.element.android.tests.testutils.awaitWithLatch
+import io.element.android.tests.testutils.consumeItemsUntilPredicate
 import io.element.android.tests.testutils.testCoroutineDispatchers
 import io.element.android.tests.testutils.waitForPredicate
 import kotlinx.coroutines.delay
@@ -186,28 +190,49 @@ class TimelinePresenterTest {
    }

    @Test
-    fun `present - covers hasNewItems scenarios`() = runTest {
+    fun `present - covers newEventState scenarios`() = runTest {
        val timeline = FakeMatrixTimeline()
        val presenter = createTimelinePresenter(timeline)
        moleculeFlow(RecompositionMode.Immediate) {
            presenter.present()
        }.test {
            val initialState = awaitItem()
-            assertThat(initialState.hasNewItems).isFalse()
+            assertThat(initialState.newEventState).isEqualTo(NewEventState.None)
            assertThat(initialState.timelineItems.size).isEqualTo(0)
            timeline.updateTimelineItems {
                listOf(MatrixTimelineItem.Event(0, anEventTimelineItem(content = aMessageContent())))
            }
-            skipItems(1)
-            assertThat(awaitItem().timelineItems.size).isEqualTo(1)
+            consumeItemsUntilPredicate { it.timelineItems.size == 1 }
+            // Mimics sending a message, and assert newEventState is FromMe
            timeline.updateTimelineItems { items ->
-                items + listOf(MatrixTimelineItem.Event(1, anEventTimelineItem(content = aMessageContent())))
+                val event = anEventTimelineItem(content = aMessageContent(), localSendState = LocalEventSendState.Sent(AN_EVENT_ID))
+                items + listOf(MatrixTimelineItem.Event(1, event))
            }
-            skipItems(1)
-            assertThat(awaitItem().timelineItems.size).isEqualTo(2)
-            assertThat(awaitItem().hasNewItems).isTrue()
+            consumeItemsUntilPredicate { it.timelineItems.size == 2 }
+            awaitLastSequentialItem().also { state ->
+                assertThat(state.newEventState).isEqualTo(NewEventState.FromMe)
+            }
+            // Mimics receiving a message without clearing the previous FromMe
+            timeline.updateTimelineItems { items ->
+                val event = anEventTimelineItem(content = aMessageContent())
+                items + listOf(MatrixTimelineItem.Event(2, event))
+            }
+            consumeItemsUntilPredicate { it.timelineItems.size == 3 }
+
+            // Scroll to bottom to clear previous FromMe
            initialState.eventSink.invoke(TimelineEvents.OnScrollFinished(0))
-            assertThat(awaitItem().hasNewItems).isFalse()
+            awaitLastSequentialItem().also { state ->
+                assertThat(state.newEventState).isEqualTo(NewEventState.None)
+            }
+            // Mimics receiving a message and assert newEventState is FromOther
+            timeline.updateTimelineItems { items ->
+                val event = anEventTimelineItem(content = aMessageContent())
+                items + listOf(MatrixTimelineItem.Event(3, event))
+            }
+            consumeItemsUntilPredicate { it.timelineItems.size == 4 }
+            awaitLastSequentialItem().also { state ->
+                assertThat(state.newEventState).isEqualTo(NewEventState.FromOther)
+            }
            cancelAndIgnoreRemainingEvents()
        }
    }
@@ -220,7 +245,7 @@ class TimelinePresenterTest {
            presenter.present()
        }.test {
            val initialState = awaitItem()
-            assertThat(initialState.hasNewItems).isFalse()
+            assertThat(initialState.newEventState).isEqualTo(NewEventState.None)
            assertThat(initialState.timelineItems.size).isEqualTo(0)
            val now = Date().time
            val minuteInMilis = 60 * 1000
				`@@ -0,0 +1 @@`
				`Scroll to end of timeline when sending a new message.`