timeline : improve jumpTo precision (introducing animateScrollToItemCenter)

2024-10-03 12:49:24 +02:00
parent 75977347e7
commit 8181572dde
14 changed files with 102 additions and 86 deletions
--- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/di/MessagesModule.kt
+++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/di/MessagesModule.kt
@@ -30,5 +30,4 @@ interface MessagesModule {

    @Binds
    fun bindTypingNotificationPresenter(presenter: TypingNotificationPresenter): Presenter<TypingNotificationState>
-
 }
--- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineItemIndexer.kt
+++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineItemIndexer.kt
@@ -8,8 +8,6 @@
 package io.element.android.features.messages.impl.timeline

 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 io.element.android.libraries.matrix.api.core.EventId
 import kotlinx.coroutines.CompletableDeferred
 import kotlinx.coroutines.sync.Mutex
@@ -17,7 +15,6 @@ import kotlinx.coroutines.sync.withLock
 import timber.log.Timber
 import javax.inject.Inject

-@SingleIn(RoomScope::class)
 class TimelineItemIndexer @Inject constructor() {
    // This is a latch to wait for the first process call
    private val firstProcessLatch = CompletableDeferred<Unit>()
--- 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
@@ -55,12 +55,12 @@ import kotlinx.coroutines.flow.launchIn
 import kotlinx.coroutines.flow.onEach
 import kotlinx.coroutines.launch
 import kotlinx.coroutines.withContext
+import timber.log.Timber

 const val FOCUS_ON_PINNED_EVENT_DEBOUNCE_DURATION_IN_MILLIS = 200L

 class TimelinePresenter @AssistedInject constructor(
    timelineItemsFactoryCreator: TimelineItemsFactory.Creator,
-    private val timelineItemIndexer: TimelineItemIndexer,
    private val room: MatrixRoom,
    private val dispatchers: CoroutineDispatchers,
    private val appScope: CoroutineScope,
@@ -70,6 +70,7 @@ class TimelinePresenter @AssistedInject constructor(
    private val endPollAction: EndPollAction,
    private val sessionPreferencesStore: SessionPreferencesStore,
    private val timelineController: TimelineController,
+    private val timelineItemIndexer: TimelineItemIndexer = TimelineItemIndexer(),
    private val resolveVerifiedUserSendFailurePresenter: Presenter<ResolveVerifiedUserSendFailureState>,
    private val typingNotificationPresenter: Presenter<TypingNotificationState>,
 ) : Presenter<TimelineState> {
@@ -89,13 +90,7 @@ class TimelinePresenter @AssistedInject constructor(
    @Composable
    override fun present(): TimelineState {
        val localScope = rememberCoroutineScope()
-        val focusRequestState: MutableState<FocusRequestState> = remember {
-            mutableStateOf(FocusRequestState.None)
-        }
-
-        LaunchedEffect(Unit) {
-            timelineItemsFactory.timelineItems.collect { timelineItems = it }
-        }
+        var focusRequestState: FocusRequestState by remember { mutableStateOf(FocusRequestState.None) }

        val lastReadReceiptId = rememberSaveable { mutableStateOf<EventId?>(null) }

@@ -154,13 +149,13 @@ class TimelinePresenter @AssistedInject constructor(
                    navigator.onEditPollClick(event.pollStartId)
                }
                is TimelineEvents.FocusOnEvent -> {
-                    focusRequestState.value = FocusRequestState.Requested(event.eventId, event.debounce)
+                    focusRequestState = FocusRequestState.Requested(event.eventId, event.debounce)
                }
                is TimelineEvents.OnFocusEventRender -> {
-                    focusRequestState.value = focusRequestState.value.onFocusEventRender()
+                    focusRequestState = focusRequestState.onFocusEventRender()
                }
                is TimelineEvents.ClearFocusRequestState -> {
-                    focusRequestState.value = FocusRequestState.None
+                    focusRequestState = FocusRequestState.None
                }
                is TimelineEvents.JumpToLive -> {
                    timelineController.focusOnLive()
@@ -173,49 +168,14 @@ class TimelinePresenter @AssistedInject constructor(
            }
        }

-        LaunchedEffect(focusRequestState.value) {
-            when (val currentFocusRequestState = focusRequestState.value) {
-                is FocusRequestState.Requested -> {
-                    delay(currentFocusRequestState.debounce)
-                    if (timelineItemIndexer.isKnown(currentFocusRequestState.eventId)) {
-                        val index = timelineItemIndexer.indexOf(currentFocusRequestState.eventId)
-                        focusRequestState.value = FocusRequestState.Success(eventId = currentFocusRequestState.eventId, index = index)
-                    } else {
-                        focusRequestState.value = FocusRequestState.Loading(eventId = currentFocusRequestState.eventId)
-                    }
-                }
-                is FocusRequestState.Loading -> {
-                    val eventId = currentFocusRequestState.eventId
-                    timelineController.focusOnEvent(eventId)
-                        .fold(
-                            onSuccess = {
-                                focusRequestState.value = FocusRequestState.Success(eventId = eventId)
-                            },
-                            onFailure = {
-                                focusRequestState.value = FocusRequestState.Failure(throwable = it)
-                            }
-                        )
-                }
-                else -> Unit
-            }
-        }
-
-        LaunchedEffect(timelineItems.size) {
-            computeNewItemState(timelineItems, prevMostRecentItemId, newEventState)
-        }
-
-        LaunchedEffect(timelineItems.size, focusRequestState.value) {
-            val currentFocusRequestState = focusRequestState.value
-            if (currentFocusRequestState is FocusRequestState.Success && !currentFocusRequestState.isIndexed) {
-                val eventId = currentFocusRequestState.eventId
-                if (timelineItemIndexer.isKnown(eventId)) {
-                    val index = timelineItemIndexer.indexOf(eventId)
-                    focusRequestState.value = FocusRequestState.Success(eventId = eventId, index = index)
-                }
-            }
-        }
-
        LaunchedEffect(Unit) {
+            timelineItemsFactory.timelineItems
+                .onEach { newTimelineItems ->
+                    timelineItemIndexer.process(newTimelineItems)
+                    timelineItems = newTimelineItems
+                }
+                .launchIn(this)
+
            combine(timelineController.timelineItems(), room.membersStateFlow) { items, membersState ->
                timelineItemsFactory.replaceWith(
                    timelineItems = items,
@@ -227,6 +187,47 @@ class TimelinePresenter @AssistedInject constructor(
                .launchIn(this)
        }

+        LaunchedEffect(focusRequestState) {
+            Timber.d("## focusRequestState: $focusRequestState")
+            when (val currentFocusRequestState = focusRequestState) {
+                is FocusRequestState.Requested -> {
+                    delay(currentFocusRequestState.debounce)
+                    if (timelineItemIndexer.isKnown(currentFocusRequestState.eventId)) {
+                        val index = timelineItemIndexer.indexOf(currentFocusRequestState.eventId)
+                        focusRequestState = FocusRequestState.Success(eventId = currentFocusRequestState.eventId, index = index)
+                    } else {
+                        focusRequestState = FocusRequestState.Loading(eventId = currentFocusRequestState.eventId)
+                    }
+                }
+                is FocusRequestState.Loading -> {
+                    val eventId = currentFocusRequestState.eventId
+                    timelineController.focusOnEvent(eventId)
+                        .onSuccess {
+                            focusRequestState = FocusRequestState.Success(eventId = eventId)
+                        }
+                        .onFailure {
+                            focusRequestState = FocusRequestState.Failure(it)
+                        }
+                }
+                else -> Unit
+            }
+        }
+
+        LaunchedEffect(timelineItems.size) {
+            computeNewItemState(timelineItems, prevMostRecentItemId, newEventState)
+        }
+
+        LaunchedEffect(timelineItems.size, focusRequestState) {
+            val currentFocusRequestState = focusRequestState
+            if (currentFocusRequestState is FocusRequestState.Success && !currentFocusRequestState.rendered) {
+                val eventId = currentFocusRequestState.eventId
+                if (timelineItemIndexer.isKnown(eventId)) {
+                    val index = timelineItemIndexer.indexOf(eventId)
+                    focusRequestState = FocusRequestState.Success(eventId = eventId, index = index)
+                }
+            }
+        }
+
        val typingNotificationState = typingNotificationPresenter.present()
        val timelineRoomInfo by remember(typingNotificationState) {
            derivedStateOf {
@@ -247,7 +248,7 @@ class TimelinePresenter @AssistedInject constructor(
            renderReadReceipts = renderReadReceipts,
            newEventState = newEventState.value,
            isLive = isLive,
-            focusRequestState = focusRequestState.value,
+            focusRequestState = focusRequestState,
            messageShield = messageShield.value,
            resolveVerifiedUserSendFailureState = resolveVerifiedUserSendFailureState,
            eventSink = { handleEvents(it) }
--- 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
@@ -31,15 +31,13 @@ data class TimelineState(
    val resolveVerifiedUserSendFailureState: ResolveVerifiedUserSendFailureState,
    val eventSink: (TimelineEvents) -> Unit,
 ) {
-    val lastTimelineEvent = timelineItems.firstOrNull { it is TimelineItem.Event} as? TimelineItem.Event
+    private val lastTimelineEvent = timelineItems.firstOrNull { it is TimelineItem.Event } as? TimelineItem.Event
    val hasAnyEvent = lastTimelineEvent != null
    val focusedEventId = focusRequestState.eventId()

-
    fun isLastOutgoingMessage(uniqueId: UniqueId): Boolean {
-        return lastTimelineEvent != null && lastTimelineEvent.isMine && lastTimelineEvent.id == uniqueId
+        return isLive && lastTimelineEvent != null && lastTimelineEvent.isMine && lastTimelineEvent.id == uniqueId
    }
-
 }

@Immutable
--- 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
@@ -62,12 +62,12 @@ import io.element.android.libraries.designsystem.preview.ElementPreview
 import io.element.android.libraries.designsystem.preview.PreviewsDayNight
 import io.element.android.libraries.designsystem.theme.components.FloatingActionButton
 import io.element.android.libraries.designsystem.theme.components.Icon
+import io.element.android.libraries.designsystem.utils.animateScrollToItemCenter
 import io.element.android.libraries.matrix.api.core.EventId
 import io.element.android.libraries.matrix.api.core.UserId
 import io.element.android.libraries.matrix.api.timeline.item.event.MessageShield
 import io.element.android.libraries.ui.strings.CommonStrings
 import kotlinx.coroutines.launch
-import kotlin.math.abs

@Composable
 fun TimelineView(
@@ -238,12 +238,8 @@ private fun BoxScope.TimelineScrollHelper(

    val latestOnFocusEventRender by rememberUpdatedState(onFocusEventRender)
    LaunchedEffect(focusRequestState) {
-        if (focusRequestState is FocusRequestState.Success && focusRequestState.isIndexed) {
-            if (abs(lazyListState.firstVisibleItemIndex - focusRequestState.index) < 10) {
-                lazyListState.animateScrollToItem(focusRequestState.index)
-            } else {
-                lazyListState.scrollToItem(focusRequestState.index)
-            }
+        if (focusRequestState is FocusRequestState.Success && focusRequestState.isIndexed && !focusRequestState.rendered) {
+            lazyListState.animateScrollToItemCenter(focusRequestState.index)
            latestOnFocusEventRender()
        }
    }
--- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineViewMessageShieldPreview.kt
+++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineViewMessageShieldPreview.kt
@@ -14,7 +14,6 @@ import io.element.android.features.messages.impl.timeline.di.LocalTimelineItemPr
 import io.element.android.features.messages.impl.timeline.di.aFakeTimelineItemPresenterFactories
 import io.element.android.features.messages.impl.timeline.model.TimelineItem
 import io.element.android.features.messages.impl.timeline.model.event.aTimelineItemTextContent
-import io.element.android.features.messages.impl.typing.aTypingNotificationState
 import io.element.android.libraries.designsystem.preview.ElementPreview
 import io.element.android.libraries.designsystem.preview.PreviewsDayNight
 import kotlinx.collections.immutable.toImmutableList
--- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/factories/TimelineItemsFactory.kt
+++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/factories/TimelineItemsFactory.kt
@@ -10,7 +10,6 @@ package io.element.android.features.messages.impl.timeline.factories
 import dagger.assisted.Assisted
 import dagger.assisted.AssistedFactory
 import dagger.assisted.AssistedInject
-import io.element.android.features.messages.impl.timeline.TimelineItemIndexer
 import io.element.android.features.messages.impl.timeline.diff.TimelineItemsCacheInvalidator
 import io.element.android.features.messages.impl.timeline.factories.event.TimelineItemEventFactory
 import io.element.android.features.messages.impl.timeline.factories.virtual.TimelineItemVirtualFactory
@@ -36,7 +35,6 @@ class TimelineItemsFactory @AssistedInject constructor(
    private val dispatchers: CoroutineDispatchers,
    private val virtualItemFactory: TimelineItemVirtualFactory,
    private val timelineItemGrouper: TimelineItemGrouper,
-    private val timelineItemIndexer: TimelineItemIndexer,
 ) {
    @AssistedFactory
    interface Creator {
@@ -96,7 +94,6 @@ class TimelineItemsFactory @AssistedInject constructor(
            }
        }
        val result = timelineItemGrouper.group(newTimelineItemStates).toPersistentList()
-        timelineItemIndexer.process(result)
        this._timelineItems.emit(result)
    }

--- a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/MessagesPresenterTest.kt
+++ b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/MessagesPresenterTest.kt
@@ -40,6 +40,7 @@ import io.element.android.features.messages.impl.timeline.model.event.TimelineIt
 import io.element.android.features.messages.impl.timeline.model.event.TimelineItemVideoContent
 import io.element.android.features.messages.impl.timeline.model.event.aTimelineItemPollContent
 import io.element.android.features.messages.impl.timeline.model.event.aTimelineItemTextContent
+import io.element.android.features.messages.impl.typing.aTypingNotificationState
 import io.element.android.features.messages.impl.utils.FakeTextPillificationHelper
 import io.element.android.features.messages.impl.voicemessages.composer.VoiceMessageComposerPlayer
 import io.element.android.features.messages.impl.voicemessages.composer.VoiceMessageComposerPresenter
@@ -1047,6 +1048,7 @@ class MessagesPresenterTest {
            timelineItemIndexer = TimelineItemIndexer(),
            timelineController = TimelineController(matrixRoom),
            resolveVerifiedUserSendFailurePresenter = { aResolveVerifiedUserSendFailureState() },
+            typingNotificationPresenter = { aTypingNotificationState() },
        )
        val timelinePresenterFactory = object : TimelinePresenter.Factory {
            override fun create(navigator: MessagesNavigator): TimelinePresenter {
--- a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/fixtures/TimelineItemsFactoryFixtures.kt
+++ b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/fixtures/TimelineItemsFactoryFixtures.kt
@@ -7,7 +7,6 @@

 package io.element.android.features.messages.impl.fixtures

-import io.element.android.features.messages.impl.timeline.TimelineItemIndexer
 import io.element.android.features.messages.impl.timeline.factories.TimelineItemsFactory
 import io.element.android.features.messages.impl.timeline.factories.TimelineItemsFactoryConfig
 import io.element.android.features.messages.impl.timeline.factories.event.TimelineItemContentFactory
@@ -40,19 +39,16 @@ import io.element.android.libraries.mediaviewer.api.util.FileExtensionExtractorW
 import io.element.android.tests.testutils.testCoroutineDispatchers
 import kotlinx.coroutines.test.TestScope

-internal fun TestScope.aTimelineItemsFactoryCreator(
-    timelineItemIndexer: TimelineItemIndexer = TimelineItemIndexer(),
-): TimelineItemsFactory.Creator {
+internal fun TestScope.aTimelineItemsFactoryCreator(): TimelineItemsFactory.Creator {
    return object : TimelineItemsFactory.Creator {
        override fun create(config: TimelineItemsFactoryConfig): TimelineItemsFactory {
-            return aTimelineItemsFactory(config, timelineItemIndexer)
+            return aTimelineItemsFactory(config)
        }
    }
 }

 internal fun TestScope.aTimelineItemsFactory(
    config: TimelineItemsFactoryConfig,
-    timelineItemIndexer: TimelineItemIndexer = TimelineItemIndexer(),
 ): TimelineItemsFactory {
    val timelineEventFormatter = aTimelineEventFormatter()
    val matrixClient = FakeMatrixClient()
@@ -96,7 +92,6 @@ internal fun TestScope.aTimelineItemsFactory(
            ),
        ),
        timelineItemGrouper = TimelineItemGrouper(),
-        timelineItemIndexer = timelineItemIndexer,
        config = config
    )
 }
--- 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
@@ -671,7 +671,7 @@ import kotlin.time.Duration.Companion.seconds
        timelineItemIndexer: TimelineItemIndexer = TimelineItemIndexer(),
    ): TimelinePresenter {
        return TimelinePresenter(
-            timelineItemsFactoryCreator = aTimelineItemsFactoryCreator(timelineItemIndexer),
+            timelineItemsFactoryCreator = aTimelineItemsFactoryCreator(),
            room = room,
            dispatchers = testCoroutineDispatchers(),
            appScope = this,
--- a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/timeline/TimelineViewTest.kt
+++ b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/timeline/TimelineViewTest.kt
@@ -17,8 +17,6 @@ import io.element.android.features.messages.impl.timeline.components.aCriticalSh
 import io.element.android.features.messages.impl.timeline.model.TimelineItem
 import io.element.android.features.messages.impl.timeline.model.event.aTimelineItemImageContent
 import io.element.android.features.messages.impl.timeline.model.virtual.TimelineItemLoadingIndicatorModel
-import io.element.android.features.messages.impl.typing.TypingNotificationState
-import io.element.android.features.messages.impl.typing.aTypingNotificationState
 import io.element.android.libraries.matrix.api.core.UniqueId
 import io.element.android.libraries.matrix.api.core.UserId
 import io.element.android.libraries.matrix.api.timeline.Timeline
@@ -139,7 +137,6 @@ class TimelineViewTest {

 private fun <R : TestRule> AndroidComposeTestRule<R, ComponentActivity>.setTimelineView(
    state: TimelineState,
-    typingNotificationState: TypingNotificationState = aTypingNotificationState(),
    onUserDataClick: (UserId) -> Unit = EnsureNeverCalledWithParam(),
    onLinkClick: (String) -> Unit = EnsureNeverCalledWithParam(),
    onMessageClick: (TimelineItem.Event) -> Unit = EnsureNeverCalledWithParam(),
--- a/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/utils/LazyListState.kt
+++ b/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/utils/LazyListState.kt
@@ -7,6 +7,8 @@

 package io.element.android.libraries.designsystem.utils

+import androidx.compose.foundation.gestures.Orientation
+import androidx.compose.foundation.lazy.LazyListLayoutInfo
 import androidx.compose.foundation.lazy.LazyListState
 import androidx.compose.runtime.Composable
 import androidx.compose.runtime.derivedStateOf
@@ -35,3 +37,38 @@ fun LazyListState.isScrollingUp(): Boolean {
        }
    }.value
 }
+
+suspend fun LazyListState.animateScrollToItemCenter(index: Int) {
+    fun LazyListLayoutInfo.containerSize(): Int {
+        return if (orientation == Orientation.Vertical) {
+            viewportSize.height
+        } else {
+            viewportSize.width
+        } - beforeContentPadding - afterContentPadding
+    }
+
+    fun LazyListLayoutInfo.resolveItemOffsetToCenter(index: Int): Int? {
+        val itemInfo = visibleItemsInfo.firstOrNull { it.index == index } ?: return null
+        val containerSize = containerSize()
+        val itemSize = itemInfo.size
+        return if (itemSize > containerSize) {
+            itemSize - containerSize / 2
+        } else {
+            -(containerSize() - itemInfo.size) / 2
+        }
+    }
+
+    // await for the first layout.
+    scroll { }
+    layoutInfo.resolveItemOffsetToCenter(index)?.let { offset ->
+        // Item is already visible, just scroll to center.
+        animateScrollToItem(index, offset)
+        return
+    }
+    // Item is not visible, jump to it...
+    scrollToItem(index)
+    // and then adjust according to the actual item size.
+    layoutInfo.resolveItemOffsetToCenter(index)?.let { offset ->
+        animateScrollToItem(index, offset)
+    }
+}
--- a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/timeline/item/virtual/VirtualTimelineItem.kt
+++ b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/timeline/item/virtual/VirtualTimelineItem.kt
@@ -26,5 +26,4 @@ sealed interface VirtualTimelineItem {
    ) : VirtualTimelineItem

    data object TypingNotification : VirtualTimelineItem
-
 }
--- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/postprocessor/TypingNotificationPostProcessor.kt
+++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/postprocessor/TypingNotificationPostProcessor.kt
@@ -16,7 +16,6 @@ import io.element.android.libraries.matrix.api.timeline.item.virtual.VirtualTime
 * This post processor is responsible for adding a typing notification item to the timeline items when the timeline is in live mode.
 */
 class TypingNotificationPostProcessor(private val mode: Timeline.Mode) {
-
    fun process(items: List<MatrixTimelineItem>): List<MatrixTimelineItem> {
        return if (mode == Timeline.Mode.LIVE) {
            buildList {