From 02e65e4eaccfa60a6471a5a8d4ff2c2bb7ae56ea Mon Sep 17 00:00:00 2001 From: ganfra Date: Fri, 24 Nov 2023 11:29:50 +0100 Subject: [PATCH] Timeline : Scroll to end of timeline when sending a new message #1877 --- changelog.d/1877.feature | 1 + .../impl/timeline/TimelinePresenter.kt | 31 ++++++++----- .../messages/impl/timeline/TimelineState.kt | 3 +- .../impl/timeline/TimelineStateProvider.kt | 3 +- .../messages/impl/timeline/TimelineView.kt | 37 ++++++++------- .../impl/timeline/model/NewEventState.kt | 25 +++++++++++ .../impl/timeline/TimelinePresenterTest.kt | 45 ++++++++++++++----- 7 files changed, 107 insertions(+), 38 deletions(-) create mode 100644 changelog.d/1877.feature create mode 100644 features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/NewEventState.kt diff --git a/changelog.d/1877.feature b/changelog.d/1877.feature new file mode 100644 index 0000000000..1156908c71 --- /dev/null +++ b/changelog.d/1877.feature @@ -0,0 +1 @@ +Scroll to end of timeline when sending a new message. 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 index 0bad8f8830..79cffe9142 100644 --- 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(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, prevMostRecentItemId: MutableState, - hasNewItemsState: MutableState + newEventState: MutableState ) = 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 } 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 index 62836db130..36fa6d33d2 100644 --- 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 ) diff --git 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 index 1cc7daa498..bbd73beede 100644 --- 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 = persistentListOf ), highlightedEventId = null, userHasPermissionToSendMessage = true, - hasNewItems = false, + newEventState = NewEventState.None, sessionState = aSessionState( isSessionVerified = true, isKeyBackupEnabled = true, 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 index 2b9433dd5b..b7ef91dc32 100644 --- 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, ) } diff --git 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 new file mode 100644 index 0000000000..9a00c9fd8f --- /dev/null +++ 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 +} 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 index 83fb402a7f..2fa75b094f 100644 --- 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