timeline : improve jumpTo precision (introducing animateScrollToItemCenter)

ganfra · ganfra · commit adc03c96761e · 2024-10-03T15:33:03.000+02:00
diff --git 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>
-
 }
diff --git 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,16 +8,13 @@
 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
 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>()
diff --git 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,28 +168,46 @@ class TimelinePresenter @AssistedInject constructor(
             }
         }
 
-        LaunchedEffect(focusRequestState.value) {
-            when (val currentFocusRequestState = focusRequestState.value) {
+        LaunchedEffect(Unit) {
+            timelineItemsFactory.timelineItems
+                .onEach { newTimelineItems ->
+                    timelineItemIndexer.process(newTimelineItems)
+                    timelineItems = newTimelineItems
+                }
+                .launchIn(this)
+
+            combine(timelineController.timelineItems(), room.membersStateFlow) { items, membersState ->
+                timelineItemsFactory.replaceWith(
+                    timelineItems = items,
+                    roomMembers = membersState.roomMembers().orEmpty()
+                )
+                items
+            }
+                .onEach(redactedVoiceMessageManager::onEachMatrixTimelineItem)
+                .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.value = FocusRequestState.Success(eventId = currentFocusRequestState.eventId, index = index)
+                        focusRequestState = FocusRequestState.Success(eventId = currentFocusRequestState.eventId, index = index)
                     } else {
-                        focusRequestState.value = FocusRequestState.Loading(eventId = currentFocusRequestState.eventId)
+                        focusRequestState = 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)
-                            }
-                        )
+                        .onSuccess {
+                            focusRequestState = FocusRequestState.Success(eventId = eventId)
+                        }
+                        .onFailure {
+                            focusRequestState = FocusRequestState.Failure(it)
+                        }
                 }
                 else -> Unit
             }
@@ -204,29 +217,17 @@ class TimelinePresenter @AssistedInject constructor(
             computeNewItemState(timelineItems, prevMostRecentItemId, newEventState)
         }
 
-        LaunchedEffect(timelineItems.size, focusRequestState.value) {
-            val currentFocusRequestState = focusRequestState.value
-            if (currentFocusRequestState is FocusRequestState.Success && !currentFocusRequestState.isIndexed) {
+        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.value = FocusRequestState.Success(eventId = eventId, index = index)
+                    focusRequestState = FocusRequestState.Success(eventId = eventId, index = index)
                 }
             }
         }
 
-        LaunchedEffect(Unit) {
-            combine(timelineController.timelineItems(), room.membersStateFlow) { items, membersState ->
-                timelineItemsFactory.replaceWith(
-                    timelineItems = items,
-                    roomMembers = membersState.roomMembers().orEmpty()
-                )
-                items
-            }
-                .onEach(redactedVoiceMessageManager::onEachMatrixTimelineItem)
-                .launchIn(this)
-        }
-
         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) }
diff --git 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
diff --git 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()
         }
     }
diff --git 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
diff --git 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)
     }
 
diff --git 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 {
diff --git 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
     )
 }
diff --git 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,
diff --git 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(),
diff --git 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)
+    }
+}
diff --git 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
-
 }
diff --git 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 {

Original file line number	Diff line number	Diff line change
`@@ -30,5 +30,4 @@ interface MessagesModule {`
`30`	`30`
`31`	`31`	`@Binds`
`32`	`32`	`fun bindTypingNotificationPresenter(presenter: TypingNotificationPresenter): Presenter<TypingNotificationState>`
`33`		`-`
`34`	`33`	`}`