Timeline : improve auto-scroll

2023-07-13 13:08:25 +02:00
parent d7aef3818a
commit 8f2afbd797
4 changed files with 87 additions and 47 deletions
--- 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
@@ -22,21 +22,25 @@ 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 +49,57 @@ 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) }
+        var lastReadReceiptIndex by rememberSaveable { mutableStateOf(Int.MAX_VALUE) }
+        var lastReadReceiptId by 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 CoroutineScope.sendReadReceiptIfNeeded(firstVisibleIndex: Int) = 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 && eventId != lastReadReceiptId) {
+                lastReadReceiptIndex = firstVisibleIndex
+                lastReadReceiptId = eventId
+                timeline.sendReadReceipt(eventId)
+            }
+        }
+
        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(event.firstIndex)
                }
            }
        }

+        LaunchedEffect(timelineItems.size) {
+            computeHasNewItems(timelineItems, prevMostRecentItemId, hasNewItems)
+        }
+
        LaunchedEffect(Unit) {
            timeline
                .timelineItems
@@ -98,10 +117,26 @@ class TimelinePresenter @Inject constructor(
            canReply = userHasPermissionToSendMessage,
            paginationState = paginationState,
            timelineItems = timelineItems,
+            hasNewItems = hasNewItems.value,
            eventSink = ::handleEvents
        )
    }

+    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()
+        hasNewItemsState.value = prevMostRecentItemIdValue != null &&
+            newMostRecentItem is TimelineItem.Event &&
+            newMostRecentItem.origin != TimelineItemEventOrigin.PAGINATION &&
+            newMostRecentItemId != prevMostRecentItemIdValue
+        prevMostRecentItemId.value = newMostRecentItemId
+    }
+
    private fun getLastEventIdBeforeOrAt(index: Int, items: ImmutableList<TimelineItem>): EventId? {
        for (item in items.subList(index, items.count())) {
            if (item is TimelineItem.Event) {
@@ -114,8 +149,4 @@ class TimelinePresenter @Inject constructor(
    private fun CoroutineScope.paginateBackwards() = launch {
        timeline.paginateBackwards(backPaginationEventLimit, backPaginationPageSize)
    }
-
-    private fun CoroutineScope.sendReadReceipt(eventId: EventId) = launch {
-        timeline.sendReadReceipt(eventId)
-    }
 }
--- 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
@@ -28,5 +28,6 @@ data class TimelineState(
    val highlightedEventId: EventId?,
    val canReply: Boolean,
    val paginationState: MatrixTimeline.PaginationState,
+    val hasNewItems: Boolean,
    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
@@ -44,7 +44,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> {
--- 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
@@ -49,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
@@ -72,7 +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.coroutines.launch

@Composable
@@ -141,8 +139,8 @@ fun TimelineView(

        TimelineScrollHelper(
            lazyListState = lazyListState,
-            timelineItems = state.timelineItems,
-            onScrollFinishedAt = ::onScrollFinishedAt,
+            hasNewItems = state.hasNewItems,
+            onScrollFinishedAt = ::onScrollFinishedAt
        )
    }
 }
@@ -238,53 +236,62 @@ 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(
+        isVisible = !canAutoScroll,
        modifier = Modifier
            .align(Alignment.BottomEnd)
            .padding(end = 24.dp, bottom = 12.dp),
-        visible = showScrollToBottomButton || LocalInspectionMode.current,
+        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.size(36.dp),