From 7b4fa146e5a7920f364d89bcbfa89f4cce198c60 Mon Sep 17 00:00:00 2001 From: ganfra Date: Thu, 25 Apr 2024 23:34:04 +0200 Subject: [PATCH] Timeline : try to get better forward pagination. --- .../components/TimelineItemVirtualRow.kt | 8 +- .../virtual/TimelineLoadingMoreIndicator.kt | 39 ++++++--- .../virtual/TimelineItemVirtualFactory.kt | 4 +- ... TimelineItemLastForwardIndicatorModel.kt} | 4 +- .../item/virtual/VirtualTimelineItem.kt | 2 +- .../matrix/impl/timeline/RustTimeline.kt | 8 +- .../InvisibleIndicatorPostProcessor.kt | 79 ------------------- .../LastForwardIndicatorsPostProcessor.kt | 76 ++++++++++++++++++ 8 files changed, 119 insertions(+), 101 deletions(-) rename features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/virtual/{TimelineItemInvisibleIndicatorModel.kt => TimelineItemLastForwardIndicatorModel.kt} (81%) delete mode 100644 libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/postprocessor/InvisibleIndicatorPostProcessor.kt create mode 100644 libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/postprocessor/LastForwardIndicatorsPostProcessor.kt diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/TimelineItemVirtualRow.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/TimelineItemVirtualRow.kt index df9d355dbf..635cda526e 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/TimelineItemVirtualRow.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/TimelineItemVirtualRow.kt @@ -31,7 +31,7 @@ import io.element.android.features.messages.impl.timeline.components.virtual.Tim import io.element.android.features.messages.impl.timeline.model.TimelineItem import io.element.android.features.messages.impl.timeline.model.virtual.TimelineItemDaySeparatorModel import io.element.android.features.messages.impl.timeline.model.virtual.TimelineItemEncryptedHistoryBannerVirtualModel -import io.element.android.features.messages.impl.timeline.model.virtual.TimelineItemInvisibleIndicatorModel +import io.element.android.features.messages.impl.timeline.model.virtual.TimelineItemLastForwardIndicatorModel import io.element.android.features.messages.impl.timeline.model.virtual.TimelineItemLoadingIndicatorModel import io.element.android.features.messages.impl.timeline.model.virtual.TimelineItemReadMarkerModel import io.element.android.features.messages.impl.timeline.model.virtual.TimelineItemRoomBeginningModel @@ -50,12 +50,14 @@ fun TimelineItemVirtualRow( is TimelineItemEncryptedHistoryBannerVirtualModel -> TimelineEncryptedHistoryBannerView() TimelineItemRoomBeginningModel -> TimelineItemRoomBeginningView(roomName = timelineRoomInfo.name) is TimelineItemLoadingIndicatorModel -> { - TimelineLoadingMoreIndicator() + TimelineLoadingMoreIndicator(virtual.model.direction) LaunchedEffect(virtual.model.timestamp) { eventSink(TimelineEvents.LoadMore(virtual.model.direction)) } } - TimelineItemInvisibleIndicatorModel -> Spacer(Modifier) + is TimelineItemLastForwardIndicatorModel -> { + Spacer(modifier = Modifier) + } } } } diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/virtual/TimelineLoadingMoreIndicator.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/virtual/TimelineLoadingMoreIndicator.kt index cf5c54f3d4..90662957be 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/virtual/TimelineLoadingMoreIndicator.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/virtual/TimelineLoadingMoreIndicator.kt @@ -17,35 +17,54 @@ package io.element.android.features.messages.impl.timeline.components.virtual import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.height import androidx.compose.foundation.layout.padding -import androidx.compose.foundation.layout.wrapContentHeight import androidx.compose.runtime.Composable import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.unit.dp import io.element.android.libraries.designsystem.preview.ElementPreview import io.element.android.libraries.designsystem.preview.PreviewsDayNight +import io.element.android.libraries.designsystem.theme.components.CircularProgressIndicator import io.element.android.libraries.designsystem.theme.components.LinearProgressIndicator +import io.element.android.libraries.matrix.api.timeline.Timeline @Composable -internal fun TimelineLoadingMoreIndicator(modifier: Modifier = Modifier) { +internal fun TimelineLoadingMoreIndicator( + direction: Timeline.PaginationDirection, + modifier: Modifier = Modifier +) { Box( - modifier = modifier - .fillMaxWidth() - .padding(2.dp), + modifier = modifier.fillMaxWidth(), contentAlignment = Alignment.Center, ) { - LinearProgressIndicator(modifier = Modifier - .height(1.dp) - .fillMaxWidth() - ) + when (direction) { + Timeline.PaginationDirection.FORWARDS -> { + LinearProgressIndicator( + modifier = Modifier + .fillMaxWidth() + .padding(top = 2.dp) + .height(1.dp) + ) + } + Timeline.PaginationDirection.BACKWARDS -> { + CircularProgressIndicator( + strokeWidth = 2.dp, + modifier = Modifier.padding(vertical = 8.dp) + ) + } + } + } } @PreviewsDayNight @Composable internal fun TimelineLoadingMoreIndicatorPreview() = ElementPreview { - TimelineLoadingMoreIndicator() + Column { + TimelineLoadingMoreIndicator(Timeline.PaginationDirection.FORWARDS) + TimelineLoadingMoreIndicator(Timeline.PaginationDirection.BACKWARDS) + } } diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/factories/virtual/TimelineItemVirtualFactory.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/factories/virtual/TimelineItemVirtualFactory.kt index e6df9e889d..2fe782b960 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/factories/virtual/TimelineItemVirtualFactory.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/factories/virtual/TimelineItemVirtualFactory.kt @@ -18,7 +18,7 @@ package io.element.android.features.messages.impl.timeline.factories.virtual import io.element.android.features.messages.impl.timeline.model.TimelineItem import io.element.android.features.messages.impl.timeline.model.virtual.TimelineItemEncryptedHistoryBannerVirtualModel -import io.element.android.features.messages.impl.timeline.model.virtual.TimelineItemInvisibleIndicatorModel +import io.element.android.features.messages.impl.timeline.model.virtual.TimelineItemLastForwardIndicatorModel import io.element.android.features.messages.impl.timeline.model.virtual.TimelineItemLoadingIndicatorModel import io.element.android.features.messages.impl.timeline.model.virtual.TimelineItemReadMarkerModel import io.element.android.features.messages.impl.timeline.model.virtual.TimelineItemRoomBeginningModel @@ -49,7 +49,7 @@ class TimelineItemVirtualFactory @Inject constructor( direction = inner.direction, timestamp = inner.timestamp ) - VirtualTimelineItem.LatestKnownEventIndicator -> TimelineItemInvisibleIndicatorModel + is VirtualTimelineItem.LastForwardIndicator -> TimelineItemLastForwardIndicatorModel } } } diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/virtual/TimelineItemInvisibleIndicatorModel.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/virtual/TimelineItemLastForwardIndicatorModel.kt similarity index 81% rename from features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/virtual/TimelineItemInvisibleIndicatorModel.kt rename to features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/virtual/TimelineItemLastForwardIndicatorModel.kt index 2f4a118746..c532538722 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/virtual/TimelineItemInvisibleIndicatorModel.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/virtual/TimelineItemLastForwardIndicatorModel.kt @@ -16,6 +16,6 @@ package io.element.android.features.messages.impl.timeline.model.virtual -data object TimelineItemInvisibleIndicatorModel : TimelineItemVirtualModel { - override val type: String = "TimelineItemInvisibleIndicatorModel" +data object TimelineItemLastForwardIndicatorModel: TimelineItemVirtualModel { + override val type: String = "TimelineItemLastForwardIndicatorModel" } 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 index 2879225553..5963f2c2d0 100644 --- 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 @@ -29,7 +29,7 @@ sealed interface VirtualTimelineItem { data object RoomBeginning: VirtualTimelineItem - data object LatestKnownEventIndicator: VirtualTimelineItem + data object LastForwardIndicator: VirtualTimelineItem data class LoadingIndicator( val direction: Timeline.PaginationDirection, diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/RustTimeline.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/RustTimeline.kt index 1e572d925d..71c331021b 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/RustTimeline.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/RustTimeline.kt @@ -45,12 +45,11 @@ import io.element.android.libraries.matrix.impl.timeline.item.event.EventMessage import io.element.android.libraries.matrix.impl.timeline.item.event.EventTimelineItemMapper import io.element.android.libraries.matrix.impl.timeline.item.event.TimelineEventContentMapper import io.element.android.libraries.matrix.impl.timeline.item.virtual.VirtualTimelineItemMapper -import io.element.android.libraries.matrix.impl.timeline.postprocessor.InvisibleIndicatorPostProcessor +import io.element.android.libraries.matrix.impl.timeline.postprocessor.LastForwardIndicatorsPostProcessor import io.element.android.libraries.matrix.impl.timeline.postprocessor.LoadingIndicatorsPostProcessor import io.element.android.libraries.matrix.impl.timeline.postprocessor.RoomBeginningPostProcessor import io.element.android.libraries.matrix.impl.timeline.postprocessor.TimelineEncryptedHistoryPostProcessor import io.element.android.services.toolbox.api.systemclock.SystemClock -import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.CoroutineScope @@ -116,7 +115,7 @@ class RustTimeline( private val roomBeginningPostProcessor = RoomBeginningPostProcessor() private val loadingIndicatorsPostProcessor = LoadingIndicatorsPostProcessor(systemClock) - private val invisibleIndicatorPostProcessor = InvisibleIndicatorPostProcessor(isLive) + private val lastForwardIndicatorsPostProcessor = LastForwardIndicatorsPostProcessor(isLive) private val timelineItemFactory = MatrixTimelineItemMapper( fetchDetailsForEvent = this::fetchDetailsForEvent, @@ -225,7 +224,8 @@ class RustTimeline( hasMoreToLoadBackwards = hasMoreToLoadBackward ) }.let { items -> loadingIndicatorsPostProcessor.process(items, hasMoreToLoadBackward, hasMoreToLoadForward) } - .let { items -> invisibleIndicatorPostProcessor.process(items) } + // Keep lastForwardIndicatorsPostProcessor last + .let { items -> lastForwardIndicatorsPostProcessor.process(items) } } diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/postprocessor/InvisibleIndicatorPostProcessor.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/postprocessor/InvisibleIndicatorPostProcessor.kt deleted file mode 100644 index 7c7f819ed7..0000000000 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/postprocessor/InvisibleIndicatorPostProcessor.kt +++ /dev/null @@ -1,79 +0,0 @@ -/* - * Copyright (c) 2024 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.impl.timeline.postprocessor - -import io.element.android.libraries.matrix.api.timeline.MatrixTimelineItem -import io.element.android.libraries.matrix.api.timeline.item.virtual.VirtualTimelineItem - -class InvisibleIndicatorPostProcessor( - private val isLive: Boolean, -) { - private val latestEventIdentifiers: MutableSet = HashSet() - - fun process( - items: List, - ): List { - if (isLive) { - return items - } else { - return buildList { - items.forEach { item -> - add(item) - if (item is MatrixTimelineItem.Event) { - if (latestEventIdentifiers.contains(item.uniqueId)) { - add(createLatestKnownEventIndicator(item.uniqueId)) - } - } - } - items.latestEventIdentifier()?.let { latestEventIdentifier -> - if (latestEventIdentifiers.add(latestEventIdentifier)) { - add(createLatestKnownEventIndicator(latestEventIdentifier)) - } - } - } - } - } - - private fun createLatestKnownEventIndicator(identifier: String): MatrixTimelineItem { - return MatrixTimelineItem.Virtual( - uniqueId = "latest_known_event_$identifier", - virtual = VirtualTimelineItem.LatestKnownEventIndicator - ) - } - - private fun List.latestEventIdentifier(): String? { - return findLast { - when (it) { - is MatrixTimelineItem.Event -> true - else -> false - } - }?.let { - (it as MatrixTimelineItem.Event).uniqueId - } - } - - private fun List.indexOf(identifier: String): Int { - return indexOfLast { - when (it) { - is MatrixTimelineItem.Event -> { - it.uniqueId == identifier - } - else -> false - } - } - } -} diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/postprocessor/LastForwardIndicatorsPostProcessor.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/postprocessor/LastForwardIndicatorsPostProcessor.kt new file mode 100644 index 0000000000..ea7337bd9e --- /dev/null +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/postprocessor/LastForwardIndicatorsPostProcessor.kt @@ -0,0 +1,76 @@ +/* + * Copyright (c) 2024 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.impl.timeline.postprocessor + +import io.element.android.libraries.matrix.api.timeline.MatrixTimelineItem +import io.element.android.libraries.matrix.api.timeline.item.virtual.VirtualTimelineItem + +/** + * This post processor is responsible for adding virtual items to indicate all the previous last forward item. + */ +class LastForwardIndicatorsPostProcessor( + private val isTimelineLive: Boolean, +) { + + private val lastForwardIdentifiers = LinkedHashSet() + + fun process( + items: List, + ): List { + // If the timeline is live, we don't have any last forward indicator to display + if (isTimelineLive) { + return items + } else { + return buildList { + val latestEventIdentifier = items.latestEventIdentifier() + // Remove if it always exists (this should happen only when no new events are added) + lastForwardIdentifiers.remove(latestEventIdentifier) + + items.forEach { item -> + add(item) + + if (item is MatrixTimelineItem.Event) { + if (lastForwardIdentifiers.contains(item.uniqueId)) { + add(createLastForwardIndicator(item.uniqueId)) + } + } + } + // This is important to always add this one at the end of the list so it's used to keep the scroll position. + add(createLastForwardIndicator(latestEventIdentifier)) + lastForwardIdentifiers.add(latestEventIdentifier) + } + } + } +} + +private fun createLastForwardIndicator(identifier: String): MatrixTimelineItem { + return MatrixTimelineItem.Virtual( + uniqueId = "last_forward_indicator_$identifier", + virtual = VirtualTimelineItem.LastForwardIndicator + ) +} + +private fun List.latestEventIdentifier(): String { + return findLast { + when (it) { + is MatrixTimelineItem.Event -> true + else -> false + } + }?.let { + (it as MatrixTimelineItem.Event).uniqueId + } ?: "fake_id" +}