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 5462112fd4..fe45796428 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 @@ -10,7 +10,6 @@ package io.element.android.features.messages.impl.timeline import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect -import androidx.compose.runtime.MutableIntState import androidx.compose.runtime.MutableState import androidx.compose.runtime.collectAsState import androidx.compose.runtime.derivedStateOf @@ -137,8 +136,7 @@ class TimelinePresenter( val prevMostRecentItemId = rememberSaveable { mutableStateOf(null) } - val newEventState = remember { mutableStateOf(NewEventState.None) } - val newMessagesCount = remember { mutableIntStateOf(0) } + val newEventState = remember { mutableStateOf(NewEventState.None) } val messageShieldDialogData: MutableState = remember { mutableStateOf(null) } val resolveVerifiedUserSendFailureState = resolveVerifiedUserSendFailurePresenter.present() @@ -177,7 +175,6 @@ class TimelinePresenter( if (isLive) { if (event.firstIndex == 0) { newEventState.value = NewEventState.None - newMessagesCount.intValue = 0 } Timber.tag(tag).d("## sendReadReceiptIfNeeded firstVisibleIndex: ${event.firstIndex}") sessionCoroutineScope.sendReadReceiptIfNeeded( @@ -278,7 +275,7 @@ class TimelinePresenter( } LaunchedEffect(timelineItems.size) { - computeNewItemState(timelineItems, prevMostRecentItemId, newEventState, newMessagesCount) + computeNewItemState(timelineItems, prevMostRecentItemId, newEventState) } // Read marker position + unread count, scanned off the main thread. The UI gates display via @@ -361,7 +358,6 @@ class TimelinePresenter( displayJumpToUnread = displayJumpToUnread, readMarkerIndex = readMarkerIndex.intValue, unreadMessagesCount = unreadMessagesCount.intValue, - newMessagesCount = newMessagesCount.intValue, eventSink = ::handleEvent, ) } @@ -420,16 +416,14 @@ class TimelinePresenter( * 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 None from this method, but need to be reset from somewhere else. * - * It also maintains [newMessagesCount], counting how many incoming messages from other users have arrived in - * each batch since [prevMostRecentItemId] — this drives the badge on the scroll-to-bottom button. The count is - * reset to zero when the most recent event is from the local user (they're back to active engagement); the - * scroll-finish handler resets it when the user returns to the bottom of the timeline. + * The [NewEventState.FromOther] variant carries the running count of incoming messages from other users + * since [prevMostRecentItemId] last advanced to a local-user event or the timeline returned to the bottom + * — this drives the badge on the scroll-to-bottom button. */ private suspend fun computeNewItemState( timelineItems: ImmutableList, prevMostRecentItemId: MutableState, newEventState: MutableState, - newMessagesCount: MutableIntState, ) = withContext(dispatchers.computation) { // FromMe is prioritized over FromOther, so skip if we already have a FromMe if (newEventState.value == NewEventState.FromMe) { @@ -448,14 +442,8 @@ class TimelinePresenter( if (hasNewEvent) { // Scroll to bottom if the new event is from me, even if sent from another device - val fromMe = newMostRecentItem.isMine - newEventState.value = if (fromMe) { - NewEventState.FromMe - } else { - NewEventState.FromOther - } - if (fromMe) { - newMessagesCount.intValue = 0 + if (newMostRecentItem.isMine) { + newEventState.value = NewEventState.FromMe } else { var delta = 0 for (item in timelineItems) { @@ -464,7 +452,8 @@ class TimelinePresenter( delta++ } } - newMessagesCount.intValue += delta + val previousCount = (newEventState.value as? NewEventState.FromOther)?.messageCount ?: 0 + newEventState.value = NewEventState.FromOther(previousCount + delta) } } 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 ccd4d44ea8..12dd01d57a 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 @@ -38,7 +38,6 @@ data class TimelineState( val displayJumpToUnread: Boolean, val readMarkerIndex: Int, val unreadMessagesCount: Int, - val newMessagesCount: Int, val eventSink: (TimelineEvent) -> Unit, ) { private val lastTimelineEvent = timelineItems.firstOrNull { it is TimelineItem.Event } as? TimelineItem.Event 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 08ddf56c8c..7fc3386ca7 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 @@ -60,7 +60,7 @@ fun aTimelineState( displayJumpToUnread: Boolean = true, readMarkerIndex: Int = -1, unreadMessagesCount: Int = 0, - newMessagesCount: Int = 0, + newEventState: NewEventState = NewEventState.None, eventSink: (TimelineEvent) -> Unit = {}, ): TimelineState { val focusedEventId = timelineItems.filterIsInstance().getOrNull(focusedEventIndex)?.eventId @@ -74,7 +74,7 @@ fun aTimelineState( timelineMode = timelineMode, timelineRoomInfo = timelineRoomInfo, renderReadReceipts = renderReadReceipts, - newEventState = NewEventState.None, + newEventState = newEventState, isLive = isLive, focusRequestState = focusRequestState, messageShieldDialogData = messageShield?.let { MessageShieldData(it) }, @@ -84,7 +84,6 @@ fun aTimelineState( displayJumpToUnread = displayJumpToUnread, readMarkerIndex = readMarkerIndex, unreadMessagesCount = unreadMessagesCount, - newMessagesCount = newMessagesCount, eventSink = eventSink, ) } 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 ce62f1ef9f..54930a2f2b 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 @@ -222,7 +222,6 @@ fun TimelineView( focusRequestState = state.focusRequestState, readMarkerIndex = state.readMarkerIndex, unreadMessagesCount = state.unreadMessagesCount, - newMessagesCount = state.newMessagesCount, displayJumpToUnread = state.displayJumpToUnread, topInset = floatingDateTopOffset, onScrollFinishAt = ::onScrollFinishAt, @@ -310,7 +309,6 @@ private fun BoxScope.TimelineScrollHelper( focusRequestState: FocusRequestState, readMarkerIndex: Int, unreadMessagesCount: Int, - newMessagesCount: Int, displayJumpToUnread: Boolean, topInset: Dp, onScrollFinishAt: (Int) -> Unit, @@ -386,8 +384,11 @@ private fun BoxScope.TimelineScrollHelper( } LaunchedEffect(canAutoScroll, newEventState) { - val shouldScrollToBottom = isScrollFinished && - (canAutoScroll && newEventState == NewEventState.FromOther || newEventState == NewEventState.FromMe) + val shouldScrollToBottom = isScrollFinished && when (newEventState) { + is NewEventState.FromOther -> canAutoScroll + NewEventState.FromMe -> true + NewEventState.None -> false + } if (shouldScrollToBottom) { scrollToBottom(force = true) } @@ -406,7 +407,7 @@ private fun BoxScope.TimelineScrollHelper( contentDescription = stringResource(id = CommonStrings.a11y_jump_to_bottom), isVisible = isJumpToBottomVisible, // Hide the badge entirely when the feature is off, regardless of the count value. - count = if (displayJumpToUnread) newMessagesCount else 0, + count = if (displayJumpToUnread) newEventState.messageCount else 0, modifier = Modifier .align(Alignment.BottomEnd) .padding(end = 24.dp, bottom = 12.dp), @@ -565,7 +566,7 @@ private fun TimelineViewWithReadMarker( // view. The actual scroll target doesn't matter for a static preview. readMarkerIndex = timelineItems.size, unreadMessagesCount = unreadMessagesCount, - newMessagesCount = newMessagesCount, + newEventState = if (newMessagesCount > 0) NewEventState.FromOther(newMessagesCount) else NewEventState.None, ), timelineProtectionState = aTimelineProtectionState(), onUserDataClick = {}, 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 index cac2798fdd..eb60d6780f 100644 --- 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 @@ -8,12 +8,20 @@ package io.element.android.features.messages.impl.timeline.model +import androidx.compose.runtime.Immutable + /** * 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. + * + * [FromOther] also carries the running count of incoming messages from other users since the + * timeline was last at the bottom — used to drive the badge on the scroll-to-bottom button. */ -enum class NewEventState { - None, - FromMe, - FromOther +@Immutable +sealed interface NewEventState { + val messageCount: Int get() = 0 + + data object None : NewEventState + data object FromMe : NewEventState + data class FromOther(override val messageCount: Int) : NewEventState } 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 e4ddbf6c82..502308cfe7 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 @@ -362,7 +362,10 @@ class TimelinePresenterTest { } consumeItemsUntilPredicate { it.timelineItems.size == 4 } awaitLastSequentialItem().also { state -> - assertThat(state.newEventState).isEqualTo(NewEventState.FromOther) + // Both events received during the FromMe window are counted now that the timeline + // has progressed past it: prevMostRecentItemId points at the local user's "1", + // so "2" and "3" are both new countable messages. + assertThat(state.newEventState).isEqualTo(NewEventState.FromOther(2)) } cancelAndIgnoreRemainingEvents() } @@ -450,13 +453,13 @@ class TimelinePresenterTest { } @Test - fun `present - newMessagesCount increments by N when N events from others arrive in one batch`() = runTest { + fun `present - FromOther count increments by N when N events from others arrive in one batch`() = runTest { val timelineItems = MutableStateFlow(emptyList()) val timeline = FakeTimeline(timelineItems = timelineItems) val presenter = createTimelinePresenter(timeline) presenter.test { val initialState = awaitFirstItem() - assertThat(initialState.newMessagesCount).isEqualTo(0) + assertThat(initialState.newEventState).isEqualTo(NewEventState.None) // Seed prevMostRecentItemId so subsequent emissions count as new events. timelineItems.emit( listOf(MatrixTimelineItem.Event(UniqueId("seed"), anEventTimelineItem(content = aMessageContent()))) @@ -470,16 +473,15 @@ class TimelinePresenterTest { MatrixTimelineItem.Event(UniqueId("3"), anEventTimelineItem(content = aMessageContent())), ) } - consumeItemsUntilPredicate { it.newMessagesCount == 3 }.last().also { state -> - assertThat(state.newMessagesCount).isEqualTo(3) - assertThat(state.newEventState).isEqualTo(NewEventState.FromOther) + consumeItemsUntilPredicate { it.newEventState == NewEventState.FromOther(3) }.last().also { state -> + assertThat(state.newEventState).isEqualTo(NewEventState.FromOther(3)) } cancelAndIgnoreRemainingEvents() } } @Test - fun `present - newMessagesCount resets to 0 on OnScrollFinished firstIndex 0`() = runTest { + fun `present - newEventState resets to None on OnScrollFinished firstIndex 0`() = runTest { val timelineItems = MutableStateFlow(emptyList()) val timeline = FakeTimeline( timelineItems = timelineItems, @@ -495,18 +497,18 @@ class TimelinePresenterTest { timelineItems.getAndUpdate { items -> items + listOf(MatrixTimelineItem.Event(UniqueId("1"), anEventTimelineItem(content = aMessageContent()))) } - val countedState = consumeItemsUntilPredicate { it.newMessagesCount == 1 }.last() - assertThat(countedState.newMessagesCount).isEqualTo(1) + val countedState = consumeItemsUntilPredicate { it.newEventState == NewEventState.FromOther(1) }.last() + assertThat(countedState.newEventState).isEqualTo(NewEventState.FromOther(1)) initialState.eventSink.invoke(TimelineEvent.OnScrollFinished(0)) - consumeItemsUntilPredicate { it.newMessagesCount == 0 }.last().also { state -> - assertThat(state.newMessagesCount).isEqualTo(0) + consumeItemsUntilPredicate { it.newEventState == NewEventState.None }.last().also { state -> + assertThat(state.newEventState).isEqualTo(NewEventState.None) } cancelAndIgnoreRemainingEvents() } } @Test - fun `present - newMessagesCount resets to 0 when latest event is from me`() = runTest { + fun `present - newEventState transitions to FromMe when latest event is from me, dropping the count`() = runTest { val timelineItems = MutableStateFlow(emptyList()) val timeline = FakeTimeline(timelineItems = timelineItems) val presenter = createTimelinePresenter(timeline) @@ -520,25 +522,23 @@ class TimelinePresenterTest { timelineItems.getAndUpdate { items -> items + listOf(MatrixTimelineItem.Event(UniqueId("1"), anEventTimelineItem(content = aMessageContent()))) } - consumeItemsUntilPredicate { it.newMessagesCount == 1 } - // Then the local user sends a message: count should reset. + consumeItemsUntilPredicate { it.newEventState == NewEventState.FromOther(1) } + // Then the local user sends a message: state moves to FromMe (which carries no count). timelineItems.getAndUpdate { items -> items + listOf( MatrixTimelineItem.Event(UniqueId("2"), anEventTimelineItem(content = aMessageContent(), isOwn = true)), ) } - consumeItemsUntilPredicate { - it.newEventState == NewEventState.FromMe && it.newMessagesCount == 0 - }.last().also { state -> - assertThat(state.newMessagesCount).isEqualTo(0) + consumeItemsUntilPredicate { it.newEventState == NewEventState.FromMe }.last().also { state -> assertThat(state.newEventState).isEqualTo(NewEventState.FromMe) + assertThat(state.newEventState.messageCount).isEqualTo(0) } cancelAndIgnoreRemainingEvents() } } @Test - fun `present - newMessagesCount does not reset on OnScrollFinished firstIndex other than 0`() = runTest { + fun `present - FromOther count does not reset on OnScrollFinished firstIndex other than 0`() = runTest { val timelineItems = MutableStateFlow(emptyList()) val timeline = FakeTimeline(timelineItems = timelineItems) val presenter = createTimelinePresenter(timeline) @@ -551,19 +551,19 @@ class TimelinePresenterTest { timelineItems.getAndUpdate { items -> items + listOf(MatrixTimelineItem.Event(UniqueId("1"), anEventTimelineItem(content = aMessageContent()))) } - consumeItemsUntilPredicate { it.newMessagesCount == 1 } + consumeItemsUntilPredicate { it.newEventState == NewEventState.FromOther(1) } // Scrolling stops above the bottom: the count must NOT reset. initialState.eventSink.invoke(TimelineEvent.OnScrollFinished(5)) advanceUntilIdle() - // No state should emit with newMessagesCount == 0. + // No state should emit with the count back at 0. val drained = consumeItemsUntilTimeout() - assertThat(drained.any { it.newMessagesCount == 0 }).isFalse() + assertThat(drained.any { it.newEventState.messageCount == 0 }).isFalse() cancelAndIgnoreRemainingEvents() } } @Test - fun `present - newMessagesCount does not increment for events with PAGINATION origin`() = runTest { + fun `present - FromOther count does not increment for events with PAGINATION origin`() = runTest { val timelineItems = MutableStateFlow(emptyList()) val timeline = FakeTimeline(timelineItems = timelineItems) val presenter = createTimelinePresenter(timeline) @@ -583,14 +583,14 @@ class TimelinePresenterTest { ) } consumeItemsUntilPredicate { it.timelineItems.size == 2 }.last().also { state -> - assertThat(state.newMessagesCount).isEqualTo(0) + assertThat(state.newEventState.messageCount).isEqualTo(0) } cancelAndIgnoreRemainingEvents() } } @Test - fun `present - newMessagesCount does not increment for state events`() = runTest { + fun `present - FromOther count does not increment for state events`() = runTest { val timelineItems = MutableStateFlow(emptyList()) val timeline = FakeTimeline(timelineItems = timelineItems) val presenter = createTimelinePresenter(timeline) @@ -607,7 +607,7 @@ class TimelinePresenterTest { ) } consumeItemsUntilPredicate { it.timelineItems.size == 2 }.last().also { state -> - assertThat(state.newMessagesCount).isEqualTo(0) + assertThat(state.newEventState.messageCount).isEqualTo(0) } cancelAndIgnoreRemainingEvents() } @@ -632,7 +632,7 @@ class TimelinePresenterTest { } @Test - fun `present - newMessagesCount accumulates across multiple batches as prevMostRecentItemId advances`() = runTest { + fun `present - FromOther count accumulates across multiple batches as prevMostRecentItemId advances`() = runTest { val timelineItems = MutableStateFlow(emptyList()) val timeline = FakeTimeline(timelineItems = timelineItems) val presenter = createTimelinePresenter(timeline) @@ -647,7 +647,7 @@ class TimelinePresenterTest { timelineItems.getAndUpdate { items -> items + listOf(MatrixTimelineItem.Event(UniqueId("b1-1"), anEventTimelineItem(content = aMessageContent()))) } - consumeItemsUntilPredicate { it.newMessagesCount == 1 } + consumeItemsUntilPredicate { it.newEventState == NewEventState.FromOther(1) } // Batch 2: 2 more new events → count = 3. timelineItems.getAndUpdate { items -> items + listOf( @@ -655,13 +655,13 @@ class TimelinePresenterTest { MatrixTimelineItem.Event(UniqueId("b2-2"), anEventTimelineItem(content = aMessageContent())), ) } - consumeItemsUntilPredicate { it.newMessagesCount == 3 } + consumeItemsUntilPredicate { it.newEventState == NewEventState.FromOther(3) } // Batch 3: 1 more new event → count = 4. timelineItems.getAndUpdate { items -> items + listOf(MatrixTimelineItem.Event(UniqueId("b3-1"), anEventTimelineItem(content = aMessageContent()))) } - consumeItemsUntilPredicate { it.newMessagesCount == 4 }.last().also { state -> - assertThat(state.newMessagesCount).isEqualTo(4) + consumeItemsUntilPredicate { it.newEventState == NewEventState.FromOther(4) }.last().also { state -> + assertThat(state.newEventState).isEqualTo(NewEventState.FromOther(4)) } cancelAndIgnoreRemainingEvents() }