Fold newMessagesCount into NewEventState.FromOther

This commit is contained in:
Jenna Vassar
2026-05-01 08:16:35 -07:00
parent af9da10254
commit 78aa500d23
6 changed files with 61 additions and 65 deletions
@@ -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<UniqueId?>(null) }
val newEventState = remember { mutableStateOf(NewEventState.None) }
val newMessagesCount = remember { mutableIntStateOf(0) }
val newEventState = remember { mutableStateOf<NewEventState>(NewEventState.None) }
val messageShieldDialogData: MutableState<MessageShieldData?> = 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<TimelineItem>,
prevMostRecentItemId: MutableState<UniqueId?>,
newEventState: MutableState<NewEventState>,
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
@@ -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
@@ -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<TimelineItem.Event>().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,
)
}
@@ -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 = {},
@@ -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
}
@@ -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<MatrixTimelineItem>())
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<MatrixTimelineItem>())
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<MatrixTimelineItem>())
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<MatrixTimelineItem>())
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<MatrixTimelineItem>())
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<MatrixTimelineItem>())
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<MatrixTimelineItem>())
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()
}