From 51816e0281e8a45aa40a65a2ade47a421e6fcc80 Mon Sep 17 00:00:00 2001 From: Jenna Vassar <5023996+jennaharris7@users.noreply.github.com> Date: Tue, 5 May 2026 13:04:37 -0700 Subject: [PATCH] Move jumpToUnreadButton to bottom, remove badge count --- .../impl/timeline/TimelinePresenter.kt | 53 +---- .../messages/impl/timeline/TimelineState.kt | 4 +- .../impl/timeline/TimelineStateProvider.kt | 7 +- .../messages/impl/timeline/TimelineView.kt | 123 ++++-------- .../impl/timeline/model/JumpToUnreadState.kt | 32 --- .../impl/timeline/model/NewEventState.kt | 7 +- .../model/event/TimelineItemEventContent.kt | 22 --- .../impl/timeline/TimelinePresenterTest.kt | 182 +++++------------- 8 files changed, 104 insertions(+), 326 deletions(-) delete mode 100644 features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/JumpToUnreadState.kt 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 6e0145f742..8dbb53d7cd 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,10 +30,8 @@ import io.element.android.features.messages.impl.crypto.sendfailure.resolve.Reso import io.element.android.features.messages.impl.timeline.components.MessageShieldData 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.model.JumpToUnreadState 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.isMessageContent import io.element.android.features.messages.impl.timeline.model.virtual.TimelineItemReadMarkerModel import io.element.android.features.messages.impl.timeline.model.virtual.TimelineItemTypingNotificationModel import io.element.android.features.messages.impl.typing.TypingNotificationState @@ -281,26 +279,16 @@ class TimelinePresenter( // read marker advances in place — the SDK swaps the marker virtual item to a new position // without changing the list length, e.g. when [markRoomAsFullyRead] is sent while at the // bottom of the room. - val jumpToUnreadState = remember { mutableStateOf(JumpToUnreadState.Disabled) } + val readMarkerIndex = remember { mutableStateOf(null) } LaunchedEffect(timelineItems, displayJumpToUnread) { if (!displayJumpToUnread) { - jumpToUnreadState.value = JumpToUnreadState.Disabled + readMarkerIndex.value = null return@LaunchedEffect } val items = timelineItems - jumpToUnreadState.value = withContext(dispatchers.computation) { - var markerIdx = -1 - var unread = 0 - for ((i, item) in items.withIndex()) { - if ((item as? TimelineItem.Virtual)?.model is TimelineItemReadMarkerModel) { - markerIdx = i - break - } - if (item is TimelineItem.Event && item.isCountableNewMessage()) { - unread++ - } - } - if (markerIdx < 0) JumpToUnreadState.NoMarker else JumpToUnreadState.Loaded(markerIdx, unread) + readMarkerIndex.value = withContext(dispatchers.computation) { + items.indexOfFirst { (it as? TimelineItem.Virtual)?.model is TimelineItemReadMarkerModel } + .takeIf { it >= 0 } } } @@ -353,7 +341,8 @@ class TimelinePresenter( resolveVerifiedUserSendFailureState = resolveVerifiedUserSendFailureState, displayThreadSummaries = displayThreadSummaries, displayFloatingDateBadge = displayFloatingDateBadge, - jumpToUnreadState = jumpToUnreadState.value, + displayJumpToUnread = displayJumpToUnread, + readMarkerIndex = readMarkerIndex.value, eventSink = ::handleEvent, ) } @@ -411,10 +400,6 @@ class TimelinePresenter( * 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 None from this method, but need to be reset from somewhere else. - * - * 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, @@ -438,19 +423,7 @@ class TimelinePresenter( if (hasNewEvent) { // Scroll to bottom if the new event is from me, even if sent from another device - if (newMostRecentItem.isMine) { - newEventState.value = NewEventState.FromMe - } else { - var delta = 0 - for (item in timelineItems) { - if (item.identifier() == prevMostRecentItemIdValue) break - if (item is TimelineItem.Event && item.isCountableNewMessage()) { - delta++ - } - } - val previousCount = (newEventState.value as? NewEventState.FromOther)?.messageCount ?: 0 - newEventState.value = NewEventState.FromOther(previousCount + delta) - } + newEventState.value = if (newMostRecentItem.isMine) NewEventState.FromMe else NewEventState.FromOther } prevMostRecentItemId.value = newMostRecentItemId } @@ -496,16 +469,6 @@ private fun FocusRequestState.onFocusEventRender(): FocusRequestState { } } -/** - * Whether this event should be counted toward the unread / new-message badges: a user-facing - * message from someone other than the local user, that wasn't pulled in via back-pagination. - */ -private fun TimelineItem.Event.isCountableNewMessage(): Boolean { - return !isMine && - origin != TimelineItemEventOrigin.PAGINATION && - content.isMessageContent() -} - // Workaround for not having the server names available, get possible server names from the user ids of the room members private fun calculateServerNamesForRoom(room: JoinedRoom): List { // If we have no room members, return right ahead 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 6f48a8d9fc..d66dfe3031 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 @@ -11,7 +11,6 @@ package io.element.android.features.messages.impl.timeline import androidx.compose.runtime.Immutable import io.element.android.features.messages.impl.crypto.sendfailure.resolve.ResolveVerifiedUserSendFailureState import io.element.android.features.messages.impl.timeline.components.MessageShieldData -import io.element.android.features.messages.impl.timeline.model.JumpToUnreadState 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.typing.TypingNotificationState @@ -36,7 +35,8 @@ data class TimelineState( val resolveVerifiedUserSendFailureState: ResolveVerifiedUserSendFailureState, val displayThreadSummaries: Boolean, val displayFloatingDateBadge: Boolean, - val jumpToUnreadState: JumpToUnreadState, + val displayJumpToUnread: Boolean, + val readMarkerIndex: 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 0ea48c3c35..5ba3a41618 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 @@ -12,7 +12,6 @@ import io.element.android.features.messages.impl.crypto.sendfailure.resolve.Reso import io.element.android.features.messages.impl.crypto.sendfailure.resolve.aResolveVerifiedUserSendFailureState import io.element.android.features.messages.impl.timeline.components.MessageShieldData import io.element.android.features.messages.impl.timeline.components.receipt.aReadReceiptData -import io.element.android.features.messages.impl.timeline.model.JumpToUnreadState 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 @@ -58,7 +57,8 @@ fun aTimelineState( resolveVerifiedUserSendFailureState: ResolveVerifiedUserSendFailureState = aResolveVerifiedUserSendFailureState(), displayThreadSummaries: Boolean = false, displayFloatingDateBadge: Boolean = false, - jumpToUnreadState: JumpToUnreadState = JumpToUnreadState.NoMarker, + displayJumpToUnread: Boolean = false, + readMarkerIndex: Int? = null, newEventState: NewEventState = NewEventState.None, eventSink: (TimelineEvent) -> Unit = {}, ): TimelineState { @@ -80,7 +80,8 @@ fun aTimelineState( resolveVerifiedUserSendFailureState = resolveVerifiedUserSendFailureState, displayThreadSummaries = displayThreadSummaries, displayFloatingDateBadge = displayFloatingDateBadge, - jumpToUnreadState = jumpToUnreadState, + displayJumpToUnread = displayJumpToUnread, + readMarkerIndex = readMarkerIndex, 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 b93f9e950d..d04f55bb2c 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 @@ -19,7 +19,6 @@ import androidx.compose.foundation.border import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.BoxScope import androidx.compose.foundation.layout.PaddingValues -import androidx.compose.foundation.layout.defaultMinSize import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.offset import androidx.compose.foundation.layout.padding @@ -49,9 +48,7 @@ import androidx.compose.ui.input.nestedscroll.nestedScroll import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.platform.LocalView import androidx.compose.ui.platform.rememberNestedScrollInteropConnection -import androidx.compose.ui.res.pluralStringResource import androidx.compose.ui.res.stringResource -import androidx.compose.ui.text.style.TextAlign import androidx.compose.ui.tooling.preview.PreviewParameter import androidx.compose.ui.unit.Dp import androidx.compose.ui.unit.IntOffset @@ -65,7 +62,6 @@ import io.element.android.features.messages.impl.timeline.components.toText 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.focus.FocusRequestStateView -import io.element.android.features.messages.impl.timeline.model.JumpToUnreadState 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 @@ -78,14 +74,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.theme.components.Text import io.element.android.libraries.designsystem.utils.animateScrollToItemCenter import io.element.android.libraries.matrix.api.core.EventId import io.element.android.libraries.matrix.api.timeline.Timeline import io.element.android.libraries.matrix.api.user.MatrixUser import io.element.android.libraries.testtags.TestTags import io.element.android.libraries.testtags.testTag -import io.element.android.libraries.ui.strings.CommonPlurals import io.element.android.libraries.ui.strings.CommonStrings import io.element.android.libraries.ui.utils.time.isTalkbackActive import io.element.android.wysiwyg.link.Link @@ -221,8 +215,8 @@ fun TimelineView( newEventState = state.newEventState, isLive = state.isLive, focusRequestState = state.focusRequestState, - jumpToUnreadState = state.jumpToUnreadState, - topInset = floatingDateTopOffset, + displayJumpToUnread = state.displayJumpToUnread, + readMarkerIndex = state.readMarkerIndex, onScrollFinishAt = ::onScrollFinishAt, onJumpToLive = ::onJumpToLive, onFocusEventRender = ::onFocusEventRender, @@ -306,8 +300,8 @@ private fun BoxScope.TimelineScrollHelper( forceJumpToBottomVisibility: Boolean, forceJumpToReadMarkerVisibility: Boolean, focusRequestState: FocusRequestState, - jumpToUnreadState: JumpToUnreadState, - topInset: Dp, + displayJumpToUnread: Boolean, + readMarkerIndex: Int?, onScrollFinishAt: (Int) -> Unit, onJumpToLive: () -> Unit, onFocusEventRender: () -> Unit, @@ -321,14 +315,10 @@ private fun BoxScope.TimelineScrollHelper( } val isJumpToUnreadVisible by remember { derivedStateOf { - when { - forceJumpToReadMarkerVisibility -> true - jumpToUnreadState !is JumpToUnreadState.Loaded -> false - else -> { - val lastVisibleIndex = lazyListState.layoutInfo.visibleItemsInfo.lastOrNull()?.index ?: return@derivedStateOf false - jumpToUnreadState.markerIndex > lastVisibleIndex - } - } + if (forceJumpToReadMarkerVisibility) return@derivedStateOf true + val markerIndex = readMarkerIndex ?: return@derivedStateOf false + val lastVisibleIndex = lazyListState.layoutInfo.visibleItemsInfo.lastOrNull()?.index ?: return@derivedStateOf false + markerIndex > lastVisibleIndex } } val isJumpToBottomVisible = !canAutoScroll || forceJumpToBottomVisibility || !isLive @@ -359,9 +349,9 @@ private fun BoxScope.TimelineScrollHelper( } fun jumpToReadMarker() { - val loaded = jumpToUnreadState as? JumpToUnreadState.Loaded ?: return + val markerIndex = readMarkerIndex ?: return coroutineScope.launch { - lazyListState.animateScrollToItemCenter(loaded.markerIndex) + lazyListState.animateScrollToItemCenter(markerIndex) } } @@ -403,28 +393,21 @@ private fun BoxScope.TimelineScrollHelper( icon = CompoundIcons.ChevronDown(), 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 (jumpToUnreadState is JumpToUnreadState.Disabled) 0 else newEventState.messageCount, + hasUnread = displayJumpToUnread && newEventState is NewEventState.FromOther, modifier = Modifier .align(Alignment.BottomEnd) .padding(end = 24.dp, bottom = 12.dp), onClick = ::jumpToBottom, ) - val unreadCount = (jumpToUnreadState as? JumpToUnreadState.Loaded)?.unreadCount ?: 0 - val jumpToUnreadDescription = if (unreadCount > 0) { - pluralStringResource(CommonPlurals.a11y_jump_to_unread_messages_count, unreadCount, unreadCount) - } else { - stringResource(id = CommonStrings.a11y_jump_to_unread_messages) - } JumpToPositionButton( icon = CompoundIcons.ChevronUp(), - contentDescription = jumpToUnreadDescription, + contentDescription = stringResource(id = CommonStrings.a11y_jump_to_unread_messages), isVisible = isJumpToUnreadVisible, - count = unreadCount, - // Top padding includes [topInset] so the FAB sits below any pinned-events banner. + hasUnread = true, + // Stacked directly above the scroll-to-bottom FAB: 12dp base + 36dp FAB + 8dp gap. modifier = Modifier - .align(Alignment.TopEnd) - .padding(end = 24.dp, top = topInset + 12.dp), + .align(Alignment.BottomEnd) + .padding(end = 24.dp, bottom = 56.dp), onClick = ::jumpToReadMarker, ) } @@ -434,7 +417,7 @@ private fun JumpToPositionButton( icon: ImageVector, contentDescription: String, isVisible: Boolean, - count: Int, + hasUnread: Boolean, onClick: () -> Unit, modifier: Modifier = Modifier, ) { @@ -459,8 +442,8 @@ private fun JumpToPositionButton( contentDescription = contentDescription, ) } - TimelineCountBadge( - count = count, + TimelineUnreadIndicator( + isVisible = hasUnread, modifier = Modifier .align(Alignment.TopEnd) .offset { IntOffset(x = 4.dp.roundToPx(), y = -4.dp.roundToPx()) }, @@ -469,39 +452,18 @@ private fun JumpToPositionButton( } } -/** - * Small accent badge overlaid on a timeline FAB. Shows the count when it's between 1 and 9, otherwise a dot. - * Renders nothing when [count] is zero or negative. - */ @Composable -private fun TimelineCountBadge( - count: Int, +private fun TimelineUnreadIndicator( + isVisible: Boolean, modifier: Modifier = Modifier, ) { - when { - count <= 0 -> return - count <= 9 -> Box( - modifier = modifier - .defaultMinSize(minWidth = 16.dp, minHeight = 16.dp) - .background(color = ElementTheme.colors.bgActionPrimaryRest, shape = CircleShape) - .border(width = 2.dp, color = ElementTheme.colors.iconOnSolidPrimary, shape = CircleShape) - .padding(horizontal = 4.dp), - contentAlignment = Alignment.Center, - ) { - Text( - text = count.toString(), - color = ElementTheme.colors.textOnSolidPrimary, - style = ElementTheme.typography.fontBodyXsMedium, - textAlign = TextAlign.Center, - ) - } - else -> Box( - modifier = modifier - .size(12.dp) - .background(color = ElementTheme.colors.bgActionPrimaryRest, shape = CircleShape) - .border(width = 2.dp, color = ElementTheme.colors.iconOnSolidPrimary, shape = CircleShape), - ) - } + if (!isVisible) return + Box( + modifier = modifier + .size(12.dp) + .background(color = ElementTheme.colors.iconSuccessPrimary, shape = CircleShape) + .border(width = 2.dp, color = ElementTheme.colors.bgCanvasDefault, shape = CircleShape), + ) } @PreviewsDayNight @@ -541,8 +503,8 @@ internal fun TimelineViewPreview( @Composable private fun TimelineViewWithReadMarker( - unreadMessagesCount: Int, - newMessagesCount: Int, + hasUnreadAbove: Boolean, + hasUnreadBelow: Boolean, ) { val timelineItems = persistentListOf( aTimelineItemEvent(isMine = false), @@ -558,11 +520,12 @@ private fun TimelineViewWithReadMarker( TimelineView( state = aTimelineState( timelineItems = timelineItems, + displayJumpToUnread = true, // Index points past the loaded items, mirroring the real-world state the FAB // represents: the user has scrolled past the read marker, so it's no longer in // view. The actual scroll target doesn't matter for a static preview. - jumpToUnreadState = JumpToUnreadState.Loaded(markerIndex = timelineItems.size, unreadCount = unreadMessagesCount), - newEventState = if (newMessagesCount > 0) NewEventState.FromOther(newMessagesCount) else NewEventState.None, + readMarkerIndex = if (hasUnreadAbove) timelineItems.size else null, + newEventState = if (hasUnreadBelow) NewEventState.FromOther else NewEventState.None, ), timelineProtectionState = aTimelineProtectionState(), onUserDataClick = {}, @@ -582,24 +545,12 @@ private fun TimelineViewWithReadMarker( @PreviewsDayNight @Composable -internal fun TimelineViewWithReadMarkerNoBadgesPreview() = ElementPreview { - TimelineViewWithReadMarker(unreadMessagesCount = 0, newMessagesCount = 0) +internal fun TimelineViewWithReadMarkerNoIndicatorsPreview() = ElementPreview { + TimelineViewWithReadMarker(hasUnreadAbove = false, hasUnreadBelow = false) } @PreviewsDayNight @Composable -internal fun TimelineViewWithReadMarkerNumericBadgePreview() = ElementPreview { - TimelineViewWithReadMarker(unreadMessagesCount = 3, newMessagesCount = 0) -} - -@PreviewsDayNight -@Composable -internal fun TimelineViewWithReadMarkerDotBadgesPreview() = ElementPreview { - TimelineViewWithReadMarker(unreadMessagesCount = 47, newMessagesCount = 0) -} - -@PreviewsDayNight -@Composable -internal fun TimelineViewWithReadMarkerBothCountsPreview() = ElementPreview { - TimelineViewWithReadMarker(unreadMessagesCount = 5, newMessagesCount = 3) +internal fun TimelineViewWithReadMarkerBothIndicatorsPreview() = ElementPreview { + TimelineViewWithReadMarker(hasUnreadAbove = true, hasUnreadBelow = true) } diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/JumpToUnreadState.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/JumpToUnreadState.kt deleted file mode 100644 index 8154445015..0000000000 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/JumpToUnreadState.kt +++ /dev/null @@ -1,32 +0,0 @@ -/* - * Copyright (c) 2025 Element Creations Ltd. - * Copyright 2023-2025 New Vector Ltd. - * - * SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial. - * Please see LICENSE files in the repository root for full details. - */ - -package io.element.android.features.messages.impl.timeline.model - -import androidx.compose.runtime.Immutable - -/** - * Drives the jump-to-unread FAB and the count badge on the scroll-to-bottom FAB. - * - * The two affordances share state because they're both gated on the same feature flag and both - * use counts derived from the same timeline scan. - */ -@Immutable -sealed interface JumpToUnreadState { - /** Feature flag is off — neither the FAB nor the new-message badge is shown. */ - data object Disabled : JumpToUnreadState - - /** Feature flag is on, but no read marker is present in the current timeline window. */ - data object NoMarker : JumpToUnreadState - - /** - * Feature flag is on and the read marker is loaded at [markerIndex]. The FAB shows when the - * marker is above the viewport; the badge displays [unreadCount] when greater than zero. - */ - data class Loaded(val markerIndex: Int, val unreadCount: Int) : JumpToUnreadState -} 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 eb60d6780f..f80640dd0f 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 @@ -13,15 +13,10 @@ 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. */ @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 + data object FromOther : NewEventState } diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/event/TimelineItemEventContent.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/event/TimelineItemEventContent.kt index fe2c264932..9c4c48d11e 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/event/TimelineItemEventContent.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/event/TimelineItemEventContent.kt @@ -99,28 +99,6 @@ fun TimelineItemEventContent.isEdited(): Boolean = when (this) { */ fun TimelineItemEventContent.isRedacted(): Boolean = this is TimelineItemRedactedContent -/** - * Whether the event content is a user-facing message that should be counted toward unread totals. - * Excludes state events, profile changes, membership changes, redactions, and unknown content. - */ -fun TimelineItemEventContent.isMessageContent(): Boolean = when (this) { - is TimelineItemTextBasedContent, - is TimelineItemAudioContent, - is TimelineItemEncryptedContent, - is TimelineItemFileContent, - is TimelineItemImageContent, - is TimelineItemStickerContent, - is TimelineItemLocationContent, - is TimelineItemPollContent, - is TimelineItemVoiceContent, - is TimelineItemVideoContent, - is TimelineItemLegacyCallInviteContent, - is TimelineItemRtcNotificationContent -> true - is TimelineItemStateContent, - is TimelineItemRedactedContent, - TimelineItemUnknownContent -> false -} - fun TimelineItemEventContentWithAttachment.duration(): Duration? { return when (this) { is TimelineItemAudioContent -> duration 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 bec7895c63..860d73a835 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 @@ -16,7 +16,6 @@ import io.element.android.features.messages.impl.fixtures.aMessageEvent import io.element.android.features.messages.impl.fixtures.aTimelineItemsFactoryCreator import io.element.android.features.messages.impl.timeline.components.MessageShieldData import io.element.android.features.messages.impl.timeline.components.aCriticalShield -import io.element.android.features.messages.impl.timeline.model.JumpToUnreadState 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.typing.aTypingNotificationState @@ -364,17 +363,14 @@ class TimelinePresenterTest { } consumeItemsUntilPredicate { it.timelineItems.size == 4 } awaitLastSequentialItem().also { state -> - // 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)) + assertThat(state.newEventState).isEqualTo(NewEventState.FromOther) } cancelAndIgnoreRemainingEvents() } } @Test - fun `present - jumpToUnreadState reports loaded marker and unread count, excluding state events`() = runTest { + fun `present - readMarkerIndex points at the read marker virtual item`() = runTest { val timelineItems = MutableStateFlow(emptyList()) val timeline = FakeTimeline(timelineItems = timelineItems) val presenter = createTimelinePresenter( @@ -394,47 +390,16 @@ class TimelinePresenterTest { MatrixTimelineItem.Event(UniqueId("msg-newest"), anEventTimelineItem(content = aMessageContent())), ) ) - consumeItemsUntilPredicate { it.jumpToUnreadState is JumpToUnreadState.Loaded }.last().also { state -> - // 2 message items above the marker; the membership state event is skipped. - assertThat(state.jumpToUnreadState).isEqualTo(JumpToUnreadState.Loaded(markerIndex = 3, unreadCount = 2)) + consumeItemsUntilPredicate { it.readMarkerIndex != null }.last().also { state -> + assertThat(state.readMarkerIndex).isEqualTo(3) + assertThat(state.displayJumpToUnread).isTrue() } cancelAndIgnoreRemainingEvents() } } @Test - fun `present - jumpToUnreadState count excludes own messages and PAGINATION-origin events`() = runTest { - val timelineItems = MutableStateFlow(emptyList()) - val timeline = FakeTimeline(timelineItems = timelineItems) - val presenter = createTimelinePresenter( - timeline = timeline, - featureFlagService = FakeFeatureFlagService(initialState = mapOf(FeatureFlags.JumpToUnread.key to true)), - ) - presenter.test { - awaitFirstItem() - // After processing (factory reverses): [other-newest, own-msg, paginated, read-marker, msg-old] - timelineItems.emit( - listOf( - MatrixTimelineItem.Event(UniqueId("msg-old"), anEventTimelineItem(content = aMessageContent())), - MatrixTimelineItem.Virtual(UniqueId("read-marker"), VirtualTimelineItem.ReadMarker), - MatrixTimelineItem.Event( - UniqueId("paginated"), - anEventTimelineItem(content = aMessageContent()).copy(origin = TimelineItemEventOrigin.PAGINATION), - ), - MatrixTimelineItem.Event(UniqueId("own-msg"), anEventTimelineItem(content = aMessageContent(), isOwn = true)), - MatrixTimelineItem.Event(UniqueId("other-newest"), anEventTimelineItem(content = aMessageContent())), - ) - ) - consumeItemsUntilPredicate { it.jumpToUnreadState is JumpToUnreadState.Loaded }.last().also { state -> - // Only `other-newest` counts: own-msg and paginated are filtered out. - assertThat(state.jumpToUnreadState).isEqualTo(JumpToUnreadState.Loaded(markerIndex = 3, unreadCount = 1)) - } - cancelAndIgnoreRemainingEvents() - } - } - - @Test - fun `present - jumpToUnreadState is NoMarker when no read marker present`() = runTest { + fun `present - readMarkerIndex is null when no read marker present`() = runTest { val timelineItems = MutableStateFlow(emptyList()) val timeline = FakeTimeline(timelineItems = timelineItems) val presenter = createTimelinePresenter( @@ -450,16 +415,41 @@ class TimelinePresenterTest { ) ) consumeItemsUntilPredicate { - it.timelineItems.size == 2 && it.jumpToUnreadState == JumpToUnreadState.NoMarker + it.timelineItems.size == 2 && it.readMarkerIndex == null }.last().also { state -> - assertThat(state.jumpToUnreadState).isEqualTo(JumpToUnreadState.NoMarker) + assertThat(state.readMarkerIndex).isNull() } cancelAndIgnoreRemainingEvents() } } @Test - fun `present - FromOther count increments by N when N events from others arrive in one batch`() = runTest { + fun `present - readMarkerIndex stays null when JumpToUnread feature flag is disabled`() = runTest { + val timelineItems = MutableStateFlow(emptyList()) + val timeline = FakeTimeline(timelineItems = timelineItems) + val presenter = createTimelinePresenter( + timeline = timeline, + featureFlagService = FakeFeatureFlagService(initialState = mapOf(FeatureFlags.JumpToUnread.key to false)), + ) + presenter.test { + awaitFirstItem() + timelineItems.emit( + listOf( + MatrixTimelineItem.Event(UniqueId("msg-old"), anEventTimelineItem(content = aMessageContent())), + MatrixTimelineItem.Virtual(UniqueId("read-marker"), VirtualTimelineItem.ReadMarker), + MatrixTimelineItem.Event(UniqueId("msg-newest"), anEventTimelineItem(content = aMessageContent())), + ) + ) + consumeItemsUntilPredicate { it.timelineItems.size == 3 }.last().also { state -> + assertThat(state.displayJumpToUnread).isFalse() + assertThat(state.readMarkerIndex).isNull() + } + cancelAndIgnoreRemainingEvents() + } + } + + @Test + fun `present - newEventState becomes FromOther when an event from another user arrives`() = runTest { val timelineItems = MutableStateFlow(emptyList()) val timeline = FakeTimeline(timelineItems = timelineItems) val presenter = createTimelinePresenter(timeline) @@ -471,16 +461,11 @@ class TimelinePresenterTest { listOf(MatrixTimelineItem.Event(UniqueId("seed"), anEventTimelineItem(content = aMessageContent()))) ) consumeItemsUntilPredicate { it.timelineItems.size == 1 } - // Three new events from another user arrive in a single batch. timelineItems.getAndUpdate { items -> - items + listOf( - MatrixTimelineItem.Event(UniqueId("1"), anEventTimelineItem(content = aMessageContent())), - MatrixTimelineItem.Event(UniqueId("2"), anEventTimelineItem(content = aMessageContent())), - MatrixTimelineItem.Event(UniqueId("3"), anEventTimelineItem(content = aMessageContent())), - ) + items + listOf(MatrixTimelineItem.Event(UniqueId("1"), anEventTimelineItem(content = aMessageContent()))) } - consumeItemsUntilPredicate { it.newEventState == NewEventState.FromOther(3) }.last().also { state -> - assertThat(state.newEventState).isEqualTo(NewEventState.FromOther(3)) + consumeItemsUntilPredicate { it.newEventState == NewEventState.FromOther }.last().also { state -> + assertThat(state.newEventState).isEqualTo(NewEventState.FromOther) } cancelAndIgnoreRemainingEvents() } @@ -503,8 +488,7 @@ class TimelinePresenterTest { timelineItems.getAndUpdate { items -> items + listOf(MatrixTimelineItem.Event(UniqueId("1"), anEventTimelineItem(content = aMessageContent()))) } - val countedState = consumeItemsUntilPredicate { it.newEventState == NewEventState.FromOther(1) }.last() - assertThat(countedState.newEventState).isEqualTo(NewEventState.FromOther(1)) + consumeItemsUntilPredicate { it.newEventState == NewEventState.FromOther } initialState.eventSink.invoke(TimelineEvent.OnScrollFinished(0)) consumeItemsUntilPredicate { it.newEventState == NewEventState.None }.last().also { state -> assertThat(state.newEventState).isEqualTo(NewEventState.None) @@ -514,7 +498,7 @@ class TimelinePresenterTest { } @Test - fun `present - newEventState transitions to FromMe when latest event is from me, dropping the count`() = runTest { + fun `present - newEventState transitions to FromMe when latest event is from me`() = runTest { val timelineItems = MutableStateFlow(emptyList()) val timeline = FakeTimeline(timelineItems = timelineItems) val presenter = createTimelinePresenter(timeline) @@ -524,12 +508,12 @@ class TimelinePresenterTest { listOf(MatrixTimelineItem.Event(UniqueId("seed"), anEventTimelineItem(content = aMessageContent()))) ) consumeItemsUntilPredicate { it.timelineItems.size == 1 } - // First, an event from another user increments the count. + // First, an event from another user moves us to FromOther. timelineItems.getAndUpdate { items -> items + listOf(MatrixTimelineItem.Event(UniqueId("1"), anEventTimelineItem(content = aMessageContent()))) } - consumeItemsUntilPredicate { it.newEventState == NewEventState.FromOther(1) } - // Then the local user sends a message: state moves to FromMe (which carries no count). + consumeItemsUntilPredicate { it.newEventState == NewEventState.FromOther } + // Then the local user sends a message: state moves to FromMe. timelineItems.getAndUpdate { items -> items + listOf( MatrixTimelineItem.Event(UniqueId("2"), anEventTimelineItem(content = aMessageContent(), isOwn = true)), @@ -537,14 +521,13 @@ class TimelinePresenterTest { } consumeItemsUntilPredicate { it.newEventState == NewEventState.FromMe }.last().also { state -> assertThat(state.newEventState).isEqualTo(NewEventState.FromMe) - assertThat(state.newEventState.messageCount).isEqualTo(0) } cancelAndIgnoreRemainingEvents() } } @Test - fun `present - FromOther count does not reset on OnScrollFinished firstIndex other than 0`() = runTest { + fun `present - newEventState does not reset on OnScrollFinished firstIndex other than 0`() = runTest { val timelineItems = MutableStateFlow(emptyList()) val timeline = FakeTimeline(timelineItems = timelineItems) val presenter = createTimelinePresenter(timeline) @@ -557,19 +540,18 @@ class TimelinePresenterTest { timelineItems.getAndUpdate { items -> items + listOf(MatrixTimelineItem.Event(UniqueId("1"), anEventTimelineItem(content = aMessageContent()))) } - consumeItemsUntilPredicate { it.newEventState == NewEventState.FromOther(1) } - // Scrolling stops above the bottom: the count must NOT reset. + consumeItemsUntilPredicate { it.newEventState == NewEventState.FromOther } + // Scrolling stops above the bottom: state must NOT reset. initialState.eventSink.invoke(TimelineEvent.OnScrollFinished(5)) advanceUntilIdle() - // No state should emit with the count back at 0. val drained = consumeItemsUntilTimeout() - assertThat(drained.any { it.newEventState.messageCount == 0 }).isFalse() + assertThat(drained.any { it.newEventState == NewEventState.None }).isFalse() cancelAndIgnoreRemainingEvents() } } @Test - fun `present - FromOther count does not increment for events with PAGINATION origin`() = runTest { + fun `present - newEventState stays None for events with PAGINATION origin`() = runTest { val timelineItems = MutableStateFlow(emptyList()) val timeline = FakeTimeline(timelineItems = timelineItems) val presenter = createTimelinePresenter(timeline) @@ -579,7 +561,7 @@ class TimelinePresenterTest { listOf(MatrixTimelineItem.Event(UniqueId("seed"), anEventTimelineItem(content = aMessageContent()))) ) consumeItemsUntilPredicate { it.timelineItems.size == 1 } - // A back-paginated event arrives. It should not bump the badge. + // A back-paginated event arrives. It should not flip newEventState. timelineItems.getAndUpdate { items -> items + listOf( MatrixTimelineItem.Event( @@ -589,38 +571,14 @@ class TimelinePresenterTest { ) } consumeItemsUntilPredicate { it.timelineItems.size == 2 }.last().also { state -> - assertThat(state.newEventState.messageCount).isEqualTo(0) + assertThat(state.newEventState).isEqualTo(NewEventState.None) } cancelAndIgnoreRemainingEvents() } } @Test - fun `present - FromOther count does not increment for state events`() = runTest { - val timelineItems = MutableStateFlow(emptyList()) - val timeline = FakeTimeline(timelineItems = timelineItems) - val presenter = createTimelinePresenter(timeline) - presenter.test { - awaitFirstItem() - timelineItems.emit( - listOf(MatrixTimelineItem.Event(UniqueId("seed"), anEventTimelineItem(content = aMessageContent()))) - ) - consumeItemsUntilPredicate { it.timelineItems.size == 1 } - // A membership change arrives. It should not bump the badge. - timelineItems.getAndUpdate { items -> - items + listOf( - MatrixTimelineItem.Event(UniqueId("membership"), anEventTimelineItem(content = aRoomMembershipContent())), - ) - } - consumeItemsUntilPredicate { it.timelineItems.size == 2 }.last().also { state -> - assertThat(state.newEventState.messageCount).isEqualTo(0) - } - cancelAndIgnoreRemainingEvents() - } - } - - @Test - fun `present - jumpToUnreadState markerIndex is 0 when the read marker is the only item`() = runTest { + fun `present - readMarkerIndex is 0 when the read marker is the only item`() = runTest { val timelineItems = MutableStateFlow(emptyList()) val timeline = FakeTimeline(timelineItems = timelineItems) val presenter = createTimelinePresenter( @@ -632,44 +590,8 @@ class TimelinePresenterTest { timelineItems.emit( listOf(MatrixTimelineItem.Virtual(UniqueId("read-marker"), VirtualTimelineItem.ReadMarker)) ) - consumeItemsUntilPredicate { it.jumpToUnreadState is JumpToUnreadState.Loaded }.last().also { state -> - assertThat(state.jumpToUnreadState).isEqualTo(JumpToUnreadState.Loaded(markerIndex = 0, unreadCount = 0)) - } - cancelAndIgnoreRemainingEvents() - } - } - - @Test - 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) - presenter.test { - awaitFirstItem() - // Seed prevMostRecentItemId so subsequent emissions count as new events. - timelineItems.emit( - listOf(MatrixTimelineItem.Event(UniqueId("seed"), anEventTimelineItem(content = aMessageContent()))) - ) - consumeItemsUntilPredicate { it.timelineItems.size == 1 } - // Batch 1: 1 new event → count = 1. - timelineItems.getAndUpdate { items -> - items + listOf(MatrixTimelineItem.Event(UniqueId("b1-1"), anEventTimelineItem(content = aMessageContent()))) - } - consumeItemsUntilPredicate { it.newEventState == NewEventState.FromOther(1) } - // Batch 2: 2 more new events → count = 3. - timelineItems.getAndUpdate { items -> - items + listOf( - MatrixTimelineItem.Event(UniqueId("b2-1"), anEventTimelineItem(content = aMessageContent())), - MatrixTimelineItem.Event(UniqueId("b2-2"), anEventTimelineItem(content = aMessageContent())), - ) - } - 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.newEventState == NewEventState.FromOther(4) }.last().also { state -> - assertThat(state.newEventState).isEqualTo(NewEventState.FromOther(4)) + consumeItemsUntilPredicate { it.readMarkerIndex != null }.last().also { state -> + assertThat(state.readMarkerIndex).isEqualTo(0) } cancelAndIgnoreRemainingEvents() }