From 7f5ce768600b65eb54213435e5f954962537ae23 Mon Sep 17 00:00:00 2001 From: Jenna Vassar <5023996+jennaharris7@users.noreply.github.com> Date: Wed, 29 Apr 2026 15:41:05 -0700 Subject: [PATCH 01/20] Feature: add jump to unread button with badge count --- .../features/messages/impl/MessagesView.kt | 38 ++- .../impl/timeline/TimelinePresenter.kt | 72 ++++- .../messages/impl/timeline/TimelineState.kt | 4 + .../impl/timeline/TimelineStateProvider.kt | 16 + .../messages/impl/timeline/TimelineView.kt | 203 ++++++++++-- .../model/event/TimelineItemEventContent.kt | 22 ++ .../impl/timeline/TimelinePresenterTest.kt | 302 ++++++++++++++++++ .../libraries/featureflag/api/FeatureFlags.kt | 7 + .../src/main/res/values/temporary.xml | 5 + .../tests/konsist/KonsistPreviewTest.kt | 2 + 10 files changed, 634 insertions(+), 37 deletions(-) diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesView.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesView.kt index 5a0b14b820c..4ffbaed4dcf 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesView.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesView.kt @@ -500,7 +500,10 @@ private fun MessagesViewContent( pinnedMessagesCount = (state.pinnedMessagesBannerState as? PinnedMessagesBannerState.Visible)?.pinnedMessagesCount() ?: 0, ) val density = LocalDensity.current - var pinnedBannerHeightDp by remember { mutableStateOf(0.dp) } + // Combined height of every banner overlaid above the timeline (pinned messages, + // knock requests). Used to push both the floating date badge and the jump-to-unread + // FAB below any banner that's currently showing. + var topBannersHeightDp by remember { mutableStateOf(0.dp) } TimelineView( state = state.timelineState, @@ -516,28 +519,31 @@ private fun MessagesViewContent( onReadReceiptClick = onReadReceiptClick, forceJumpToBottomVisibility = forceJumpToBottomVisibility, nestedScrollConnection = scrollBehavior.nestedScrollConnection, - floatingDateTopOffset = pinnedBannerHeightDp, + floatingDateTopOffset = topBannersHeightDp, ) if (state.timelineState.timelineMode !is Timeline.Mode.Thread) { - AnimatedVisibility( - visible = state.pinnedMessagesBannerState is PinnedMessagesBannerState.Visible && scrollBehavior.isVisible, - modifier = Modifier.onSizeChanged { pinnedBannerHeightDp = with(density) { it.height.toDp() } }, - enter = expandVertically(), - exit = shrinkVertically(), + Column( + modifier = Modifier.onSizeChanged { topBannersHeightDp = with(density) { it.height.toDp() } }, ) { - fun focusOnPinnedEvent(eventId: EventId) { - state.timelineState.eventSink( - TimelineEvent.FocusOnEvent(eventId = eventId, debounce = FOCUS_ON_PINNED_EVENT_DEBOUNCE_DURATION_IN_MILLIS.milliseconds) + AnimatedVisibility( + visible = state.pinnedMessagesBannerState is PinnedMessagesBannerState.Visible && scrollBehavior.isVisible, + enter = expandVertically(), + exit = shrinkVertically(), + ) { + fun focusOnPinnedEvent(eventId: EventId) { + state.timelineState.eventSink( + TimelineEvent.FocusOnEvent(eventId = eventId, debounce = FOCUS_ON_PINNED_EVENT_DEBOUNCE_DURATION_IN_MILLIS.milliseconds) + ) + } + PinnedMessagesBannerView( + state = state.pinnedMessagesBannerState, + onClick = ::focusOnPinnedEvent, + onViewAllClick = onViewAllPinnedMessagesClick, ) } - PinnedMessagesBannerView( - state = state.pinnedMessagesBannerState, - onClick = ::focusOnPinnedEvent, - onViewAllClick = onViewAllPinnedMessagesClick, - ) + knockRequestsBannerView() } - knockRequestsBannerView() } } } 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 edd8d446dc3..82f7717a8d7 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,16 +10,19 @@ 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 import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableIntStateOf import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.produceState import androidx.compose.runtime.remember import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.runtime.saveable.rememberSaveable import androidx.compose.runtime.setValue +import androidx.compose.runtime.snapshots.Snapshot import dev.zacsweers.metro.Assisted import dev.zacsweers.metro.AssistedFactory import dev.zacsweers.metro.AssistedInject @@ -32,6 +35,8 @@ import io.element.android.features.messages.impl.timeline.factories.TimelineItem import io.element.android.features.messages.impl.timeline.factories.TimelineItemsFactoryConfig 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 import io.element.android.features.messages.impl.userEventPermissions @@ -133,6 +138,7 @@ class TimelinePresenter( val prevMostRecentItemId = rememberSaveable { mutableStateOf(null) } val newEventState = remember { mutableStateOf(NewEventState.None) } + val newMessagesCount = remember { mutableIntStateOf(0) } val messageShieldDialogData: MutableState = remember { mutableStateOf(null) } val resolveVerifiedUserSendFailureState = resolveVerifiedUserSendFailurePresenter.present() @@ -152,6 +158,9 @@ class TimelinePresenter( val displayFloatingDateBadge by produceState(false) { value = featureFlagService.isFeatureEnabled(FeatureFlags.FloatingDateBadge) } + val displayJumpToUnread by produceState(false) { + value = featureFlagService.isFeatureEnabled(FeatureFlags.JumpToUnread) + } fun handleEvent(event: TimelineEvent) { when (event) { @@ -168,6 +177,7 @@ 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( @@ -268,7 +278,39 @@ class TimelinePresenter( } LaunchedEffect(timelineItems.size) { - computeNewItemState(timelineItems, prevMostRecentItemId, newEventState) + computeNewItemState(timelineItems, prevMostRecentItemId, newEventState, newMessagesCount) + } + + // Read marker position + unread count, scanned off the main thread. The UI gates display via + // [displayJumpToUnread]; the values are always computed so the state stays up to date if the + // feature flag flips at runtime. + val readMarkerIndex = remember { mutableIntStateOf(-1) } + val unreadMessagesCount = remember { mutableIntStateOf(0) } + LaunchedEffect(timelineItems) { + val items = timelineItems + 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.isMine && + item.origin != TimelineItemEventOrigin.PAGINATION && + item.content.isMessageContent() + ) { + unread++ + } + } + // Apply both writes atomically so consumers never see a half-updated pair + // (e.g. a non-negative markerIdx with the previous unread count). + Snapshot.withMutableSnapshot { + readMarkerIndex.intValue = markerIdx + unreadMessagesCount.intValue = if (markerIdx < 0) 0 else unread + } + } } LaunchedEffect(timelineItems.size, focusRequestState.value) { @@ -320,6 +362,10 @@ class TimelinePresenter( resolveVerifiedUserSendFailureState = resolveVerifiedUserSendFailureState, displayThreadSummaries = displayThreadSummaries, displayFloatingDateBadge = displayFloatingDateBadge, + displayJumpToUnread = displayJumpToUnread, + readMarkerIndex = readMarkerIndex.intValue, + unreadMessagesCount = unreadMessagesCount.intValue, + newMessagesCount = newMessagesCount.intValue, eventSink = ::handleEvent, ) } @@ -377,11 +423,17 @@ 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. + * + * 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. */ private suspend fun computeNewItemState( timelineItems: ImmutableList, prevMostRecentItemId: MutableState, - newEventState: 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) { @@ -406,6 +458,22 @@ class TimelinePresenter( } else { NewEventState.FromOther } + if (fromMe) { + newMessagesCount.intValue = 0 + } else { + var delta = 0 + for (item in timelineItems) { + if (item.identifier() == prevMostRecentItemIdValue) break + if (item is TimelineItem.Event && + item.origin != TimelineItemEventOrigin.PAGINATION && + !item.isMine && + item.content.isMessageContent() + ) { + delta++ + } + } + newMessagesCount.intValue += 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 1869ad69068..ccd4d44ea81 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 @@ -35,6 +35,10 @@ data class TimelineState( val resolveVerifiedUserSendFailureState: ResolveVerifiedUserSendFailureState, val displayThreadSummaries: Boolean, val displayFloatingDateBadge: Boolean, + 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 9840ac5107f..e30eb1be972 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 @@ -23,6 +23,7 @@ import io.element.android.features.messages.impl.timeline.model.anAggregatedReac import io.element.android.features.messages.impl.timeline.model.event.TimelineItemEventContent import io.element.android.features.messages.impl.timeline.model.event.aTimelineItemStateEventContent import io.element.android.features.messages.impl.timeline.model.event.aTimelineItemTextContent +import io.element.android.features.messages.impl.timeline.model.virtual.TimelineItemReadMarkerModel import io.element.android.features.messages.impl.timeline.model.virtual.aTimelineItemDaySeparatorModel import io.element.android.features.messages.impl.typing.TypingNotificationState import io.element.android.features.messages.impl.typing.aTypingNotificationState @@ -57,6 +58,10 @@ fun aTimelineState( resolveVerifiedUserSendFailureState: ResolveVerifiedUserSendFailureState = aResolveVerifiedUserSendFailureState(), displayThreadSummaries: Boolean = false, displayFloatingDateBadge: Boolean = false, + displayJumpToUnread: Boolean = true, + readMarkerIndex: Int = -1, + unreadMessagesCount: Int = 0, + newMessagesCount: Int = 0, eventSink: (TimelineEvent) -> Unit = {}, ): TimelineState { val focusedEventId = timelineItems.filterIsInstance().getOrNull(focusedEventIndex)?.eventId @@ -77,10 +82,21 @@ fun aTimelineState( resolveVerifiedUserSendFailureState = resolveVerifiedUserSendFailureState, displayThreadSummaries = displayThreadSummaries, displayFloatingDateBadge = displayFloatingDateBadge, + displayJumpToUnread = displayJumpToUnread, + readMarkerIndex = readMarkerIndex, + unreadMessagesCount = unreadMessagesCount, + newMessagesCount = newMessagesCount, eventSink = eventSink, ) } +internal fun aTimelineItemReadMarker(): TimelineItem.Virtual { + return TimelineItem.Virtual( + id = UniqueId(UUID.randomUUID().toString()), + model = TimelineItemReadMarkerModel, + ) +} + internal fun aTimelineItemList(content: TimelineItemEventContent): ImmutableList { return persistentListOf( // 3 items (First Middle Last) with isMine = false 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 2105cf9df7b..8c7d890d2f9 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 @@ -14,10 +14,14 @@ import androidx.compose.animation.core.tween import androidx.compose.animation.fadeIn import androidx.compose.animation.scaleIn import androidx.compose.animation.scaleOut +import androidx.compose.foundation.background +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 import androidx.compose.foundation.layout.size import androidx.compose.foundation.lazy.LazyColumn @@ -39,15 +43,18 @@ import androidx.compose.runtime.setValue import androidx.compose.runtime.snapshotFlow import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier -import androidx.compose.ui.draw.rotate +import androidx.compose.ui.graphics.vector.ImageVector import androidx.compose.ui.input.nestedscroll.NestedScrollConnection 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 import androidx.compose.ui.unit.dp import io.element.android.compound.theme.ElementTheme import io.element.android.compound.tokens.generated.CompoundIcons @@ -70,15 +77,18 @@ 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 +import kotlinx.collections.immutable.persistentListOf import kotlinx.coroutines.delay import kotlinx.coroutines.flow.collectLatest import kotlinx.coroutines.flow.combine @@ -106,6 +116,7 @@ fun TimelineView( modifier: Modifier = Modifier, lazyListState: LazyListState = rememberLazyListState(), forceJumpToBottomVisibility: Boolean = false, + forceJumpToReadMarkerVisibility: Boolean = false, nestedScrollConnection: NestedScrollConnection = rememberNestedScrollInteropConnection(), floatingDateTopOffset: Dp = 0.dp, ) { @@ -205,9 +216,15 @@ fun TimelineView( hasAnyEvent = state.hasAnyEvent, lazyListState = lazyListState, forceJumpToBottomVisibility = forceJumpToBottomVisibility, + forceJumpToReadMarkerVisibility = forceJumpToReadMarkerVisibility, newEventState = state.newEventState, isLive = state.isLive, focusRequestState = state.focusRequestState, + readMarkerIndex = state.readMarkerIndex, + unreadMessagesCount = state.unreadMessagesCount, + newMessagesCount = state.newMessagesCount, + displayJumpToUnread = state.displayJumpToUnread, + topInset = floatingDateTopOffset, onScrollFinishAt = ::onScrollFinishAt, onJumpToLive = ::onJumpToLive, onFocusEventRender = ::onFocusEventRender, @@ -289,7 +306,13 @@ private fun BoxScope.TimelineScrollHelper( newEventState: NewEventState, isLive: Boolean, forceJumpToBottomVisibility: Boolean, + forceJumpToReadMarkerVisibility: Boolean, focusRequestState: FocusRequestState, + readMarkerIndex: Int, + unreadMessagesCount: Int, + newMessagesCount: Int, + displayJumpToUnread: Boolean, + topInset: Dp, onScrollFinishAt: (Int) -> Unit, onJumpToLive: () -> Unit, onFocusEventRender: () -> Unit, @@ -301,6 +324,19 @@ private fun BoxScope.TimelineScrollHelper( lazyListState.firstVisibleItemIndex < 3 && isLive } } + val isReadMarkerOffTop by remember { + derivedStateOf { + if (!displayJumpToUnread || readMarkerIndex < 0) { + false + } else if (forceJumpToReadMarkerVisibility) { + true + } else { + val lastVisibleIndex = lazyListState.layoutInfo.visibleItemsInfo.lastOrNull()?.index ?: return@derivedStateOf false + readMarkerIndex > lastVisibleIndex + } + } + } + val isJumpToBottomVisible = !canAutoScroll || forceJumpToBottomVisibility || !isLive var jumpToLiveHandled by remember { mutableStateOf(true) } /** @@ -327,6 +363,13 @@ private fun BoxScope.TimelineScrollHelper( } } + fun jumpToReadMarker() { + if (readMarkerIndex < 0) return + coroutineScope.launch { + lazyListState.animateScrollToItemCenter(readMarkerIndex) + } + } + LaunchedEffect(jumpToLiveHandled, isLive) { if (!jumpToLiveHandled && isLive) { lazyListState.scrollToItem(0) @@ -358,19 +401,41 @@ private fun BoxScope.TimelineScrollHelper( } } - JumpToBottomButton( - // Use inverse of canAutoScroll otherwise we might briefly see the before the scroll animation is triggered - isVisible = !canAutoScroll || forceJumpToBottomVisibility || !isLive, + TimelineFab( + 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 (displayJumpToUnread) newMessagesCount else 0, modifier = Modifier .align(Alignment.BottomEnd) .padding(end = 24.dp, bottom = 12.dp), - onClick = { jumpToBottom() }, + onClick = ::jumpToBottom, + ) + val jumpToUnreadDescription = if (unreadMessagesCount > 0) { + pluralStringResource(CommonPlurals.a11y_jump_to_unread_messages_count, unreadMessagesCount, unreadMessagesCount) + } else { + stringResource(id = CommonStrings.a11y_jump_to_unread_messages) + } + TimelineFab( + icon = CompoundIcons.ChevronUp(), + contentDescription = jumpToUnreadDescription, + isVisible = isReadMarkerOffTop, + count = unreadMessagesCount, + // Top padding includes [topInset] so the FAB sits below any pinned-events banner. + modifier = Modifier + .align(Alignment.TopEnd) + .padding(end = 24.dp, top = topInset + 12.dp), + onClick = ::jumpToReadMarker, ) } @Composable -private fun JumpToBottomButton( +private fun TimelineFab( + icon: ImageVector, + contentDescription: String, isVisible: Boolean, + count: Int, onClick: () -> Unit, modifier: Modifier = Modifier, ) { @@ -380,22 +445,64 @@ private fun JumpToBottomButton( enter = scaleIn(animationSpec = tween(100)), exit = scaleOut(animationSpec = tween(100)), ) { - FloatingActionButton( - onClick = onClick, - elevation = FloatingActionButtonDefaults.elevation(4.dp, 4.dp, 4.dp, 4.dp), - shape = CircleShape, - modifier = Modifier.size(36.dp), - containerColor = ElementTheme.colors.bgSubtleSecondary, - contentColor = ElementTheme.colors.iconSecondary, - ) { - Icon( + Box { + FloatingActionButton( + onClick = onClick, + elevation = FloatingActionButtonDefaults.elevation(4.dp, 4.dp, 4.dp, 4.dp), + shape = CircleShape, + modifier = Modifier.size(36.dp), + containerColor = ElementTheme.colors.bgSubtleSecondary, + contentColor = ElementTheme.colors.iconSecondary, + ) { + Icon( + modifier = Modifier.size(24.dp), + imageVector = icon, + contentDescription = contentDescription, + ) + } + TimelineCountBadge( + count = count, modifier = Modifier - .size(24.dp) - .rotate(90f), - imageVector = CompoundIcons.ArrowRight(), - contentDescription = stringResource(id = CommonStrings.a11y_jump_to_bottom) + .align(Alignment.TopEnd) + .offset { IntOffset(x = 4.dp.roundToPx(), y = -4.dp.roundToPx()) }, + ) + } + } +} + +/** + * 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, + modifier: Modifier = Modifier, +) { + if (count <= 0) return + if (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), + ) } } @@ -433,3 +540,61 @@ internal fun TimelineViewPreview( ) } } + +@Composable +private fun TimelineViewWithReadMarker( + unreadMessagesCount: Int, + newMessagesCount: Int, +) { + val readMarker = aTimelineItemReadMarker() + val timelineItems = persistentListOf( + aTimelineItemEvent(isMine = false), + aTimelineItemEvent(isMine = false), + aTimelineItemEvent(isMine = true), + readMarker, + aTimelineItemEvent(isMine = false), + aTimelineItemEvent(isMine = false), + ) + CompositionLocalProvider( + LocalTimelineItemPresenterFactories provides aFakeTimelineItemPresenterFactories(), + ) { + TimelineView( + state = aTimelineState( + timelineItems = timelineItems, + readMarkerIndex = timelineItems.indexOf(readMarker), + unreadMessagesCount = unreadMessagesCount, + newMessagesCount = newMessagesCount, + ), + timelineProtectionState = aTimelineProtectionState(), + onUserDataClick = {}, + onLinkClick = {}, + onContentClick = {}, + onMessageLongClick = {}, + onSwipeToReply = {}, + onReactionClick = { _, _ -> }, + onReactionLongClick = { _, _ -> }, + onMoreReactionsClick = {}, + onReadReceiptClick = {}, + forceJumpToBottomVisibility = true, + forceJumpToReadMarkerVisibility = true, + ) + } +} + +@PreviewsDayNight +@Composable +internal fun TimelineViewWithReadMarkerNoBadgesPreview() = ElementPreview { + TimelineViewWithReadMarker(unreadMessagesCount = 0, newMessagesCount = 0) +} + +@PreviewsDayNight +@Composable +internal fun TimelineViewWithReadMarkerPreview() = ElementPreview { + TimelineViewWithReadMarker(unreadMessagesCount = 3, newMessagesCount = 12) +} + +@PreviewsDayNight +@Composable +internal fun TimelineViewWithReadMarkerDotBadgesPreview() = ElementPreview { + TimelineViewWithReadMarker(unreadMessagesCount = 47, newMessagesCount = 99) +} 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 9c4c48d11ea..fe2c264932e 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,6 +99,28 @@ 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 194694714b3..e4ddbf6c82d 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 @@ -42,6 +42,7 @@ import io.element.android.libraries.matrix.api.timeline.Timeline import io.element.android.libraries.matrix.api.timeline.item.event.EventReaction import io.element.android.libraries.matrix.api.timeline.item.event.ReactionSender import io.element.android.libraries.matrix.api.timeline.item.event.Receipt +import io.element.android.libraries.matrix.api.timeline.item.event.TimelineItemEventOrigin import io.element.android.libraries.matrix.api.timeline.item.virtual.VirtualTimelineItem import io.element.android.libraries.matrix.test.AN_EVENT_ID import io.element.android.libraries.matrix.test.AN_EVENT_ID_2 @@ -58,12 +59,14 @@ import io.element.android.libraries.matrix.test.room.powerlevels.FakeRoomPermiss import io.element.android.libraries.matrix.test.timeline.FakeTimeline import io.element.android.libraries.matrix.test.timeline.aMessageContent import io.element.android.libraries.matrix.test.timeline.anEventTimelineItem +import io.element.android.libraries.matrix.test.timeline.item.event.aRoomMembershipContent import io.element.android.libraries.matrix.ui.components.aMatrixUserList import io.element.android.libraries.preferences.test.InMemorySessionPreferencesStore 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.consumeItemsUntilPredicate +import io.element.android.tests.testutils.consumeItemsUntilTimeout import io.element.android.tests.testutils.lambda.any import io.element.android.tests.testutils.lambda.assert import io.element.android.tests.testutils.lambda.lambdaError @@ -365,6 +368,305 @@ class TimelinePresenterTest { } } + @Test + fun `present - unreadMessagesCount counts message-content items between newest and read marker, excluding state events`() = runTest { + val timelineItems = MutableStateFlow(emptyList()) + val timeline = FakeTimeline(timelineItems = timelineItems) + val presenter = createTimelinePresenter(timeline) + presenter.test { + val initialState = awaitFirstItem() + assertThat(initialState.readMarkerIndex).isEqualTo(-1) + assertThat(initialState.unreadMessagesCount).isEqualTo(0) + // SDK delivers items oldest-first; the factory reverses so output index 0 is the newest. + // After processing: [msg-newest, membership, msg-2, 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("msg-2"), anEventTimelineItem(content = aMessageContent())), + MatrixTimelineItem.Event(UniqueId("membership"), anEventTimelineItem(content = aRoomMembershipContent())), + MatrixTimelineItem.Event(UniqueId("msg-newest"), anEventTimelineItem(content = aMessageContent())), + ) + ) + consumeItemsUntilPredicate { it.readMarkerIndex >= 0 }.last().also { state -> + // 2 message items above the marker; the membership state event is skipped. + assertThat(state.readMarkerIndex).isEqualTo(3) + assertThat(state.unreadMessagesCount).isEqualTo(2) + } + cancelAndIgnoreRemainingEvents() + } + } + + @Test + fun `present - unreadMessagesCount excludes own messages and PAGINATION-origin events`() = runTest { + val timelineItems = MutableStateFlow(emptyList()) + val timeline = FakeTimeline(timelineItems = timelineItems) + val presenter = createTimelinePresenter(timeline) + 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.readMarkerIndex >= 0 }.last().also { state -> + assertThat(state.readMarkerIndex).isEqualTo(3) + // Only `other-newest` counts: own-msg and paginated are filtered out. + assertThat(state.unreadMessagesCount).isEqualTo(1) + } + cancelAndIgnoreRemainingEvents() + } + } + + @Test + fun `present - readMarkerIndex is -1 when no read marker present`() = runTest { + val timelineItems = MutableStateFlow(emptyList()) + val timeline = FakeTimeline(timelineItems = timelineItems) + val presenter = createTimelinePresenter(timeline) + presenter.test { + val initialState = awaitFirstItem() + assertThat(initialState.readMarkerIndex).isEqualTo(-1) + assertThat(initialState.unreadMessagesCount).isEqualTo(0) + timelineItems.emit( + listOf( + MatrixTimelineItem.Event(UniqueId("1"), anEventTimelineItem(content = aMessageContent())), + MatrixTimelineItem.Event(UniqueId("2"), anEventTimelineItem(content = aMessageContent())), + ) + ) + consumeItemsUntilPredicate { it.timelineItems.size == 2 }.last().also { state -> + assertThat(state.readMarkerIndex).isEqualTo(-1) + assertThat(state.unreadMessagesCount).isEqualTo(0) + } + cancelAndIgnoreRemainingEvents() + } + } + + @Test + fun `present - newMessagesCount 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) + // Seed prevMostRecentItemId so subsequent emissions count as new events. + timelineItems.emit( + 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())), + ) + } + consumeItemsUntilPredicate { it.newMessagesCount == 3 }.last().also { state -> + assertThat(state.newMessagesCount).isEqualTo(3) + assertThat(state.newEventState).isEqualTo(NewEventState.FromOther) + } + cancelAndIgnoreRemainingEvents() + } + } + + @Test + fun `present - newMessagesCount resets to 0 on OnScrollFinished firstIndex 0`() = runTest { + val timelineItems = MutableStateFlow(emptyList()) + val timeline = FakeTimeline( + timelineItems = timelineItems, + markAsReadResult = { Result.success(Unit) }, + ) + val presenter = createTimelinePresenter(timeline) + presenter.test { + val initialState = awaitFirstItem() + timelineItems.emit( + listOf(MatrixTimelineItem.Event(UniqueId("seed"), anEventTimelineItem(content = aMessageContent()))) + ) + consumeItemsUntilPredicate { it.timelineItems.size == 1 } + timelineItems.getAndUpdate { items -> + items + listOf(MatrixTimelineItem.Event(UniqueId("1"), anEventTimelineItem(content = aMessageContent()))) + } + val countedState = consumeItemsUntilPredicate { it.newMessagesCount == 1 }.last() + assertThat(countedState.newMessagesCount).isEqualTo(1) + initialState.eventSink.invoke(TimelineEvent.OnScrollFinished(0)) + consumeItemsUntilPredicate { it.newMessagesCount == 0 }.last().also { state -> + assertThat(state.newMessagesCount).isEqualTo(0) + } + cancelAndIgnoreRemainingEvents() + } + } + + @Test + fun `present - newMessagesCount resets to 0 when latest event is from me`() = 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 } + // First, an event from another user increments the count. + 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. + 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) + assertThat(state.newEventState).isEqualTo(NewEventState.FromMe) + } + cancelAndIgnoreRemainingEvents() + } + } + + @Test + fun `present - newMessagesCount does not reset on OnScrollFinished firstIndex other than 0`() = runTest { + val timelineItems = MutableStateFlow(emptyList()) + val timeline = FakeTimeline(timelineItems = timelineItems) + val presenter = createTimelinePresenter(timeline) + presenter.test { + val initialState = awaitFirstItem() + timelineItems.emit( + listOf(MatrixTimelineItem.Event(UniqueId("seed"), anEventTimelineItem(content = aMessageContent()))) + ) + consumeItemsUntilPredicate { it.timelineItems.size == 1 } + timelineItems.getAndUpdate { items -> + items + listOf(MatrixTimelineItem.Event(UniqueId("1"), anEventTimelineItem(content = aMessageContent()))) + } + consumeItemsUntilPredicate { it.newMessagesCount == 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. + val drained = consumeItemsUntilTimeout() + assertThat(drained.any { it.newMessagesCount == 0 }).isFalse() + cancelAndIgnoreRemainingEvents() + } + } + + @Test + fun `present - newMessagesCount does not increment for events with PAGINATION origin`() = 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 back-paginated event arrives. It should not bump the badge. + timelineItems.getAndUpdate { items -> + items + listOf( + MatrixTimelineItem.Event( + UniqueId("paginated"), + anEventTimelineItem(content = aMessageContent()).copy(origin = TimelineItemEventOrigin.PAGINATION), + ) + ) + } + consumeItemsUntilPredicate { it.timelineItems.size == 2 }.last().also { state -> + assertThat(state.newMessagesCount).isEqualTo(0) + } + cancelAndIgnoreRemainingEvents() + } + } + + @Test + fun `present - newMessagesCount 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.newMessagesCount).isEqualTo(0) + } + cancelAndIgnoreRemainingEvents() + } + } + + @Test + 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(timeline) + presenter.test { + awaitFirstItem() + timelineItems.emit( + listOf(MatrixTimelineItem.Virtual(UniqueId("read-marker"), VirtualTimelineItem.ReadMarker)) + ) + consumeItemsUntilPredicate { it.readMarkerIndex >= 0 }.last().also { state -> + assertThat(state.readMarkerIndex).isEqualTo(0) + assertThat(state.unreadMessagesCount).isEqualTo(0) + } + cancelAndIgnoreRemainingEvents() + } + } + + @Test + fun `present - newMessagesCount 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.newMessagesCount == 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.newMessagesCount == 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) + } + cancelAndIgnoreRemainingEvents() + } + } + @Test fun `present - reaction ordering`() = runTest { val timelineItems = MutableStateFlow(emptyList()) diff --git a/libraries/featureflag/api/src/main/kotlin/io/element/android/libraries/featureflag/api/FeatureFlags.kt b/libraries/featureflag/api/src/main/kotlin/io/element/android/libraries/featureflag/api/FeatureFlags.kt index 15e61f42608..60e164df0f1 100644 --- a/libraries/featureflag/api/src/main/kotlin/io/element/android/libraries/featureflag/api/FeatureFlags.kt +++ b/libraries/featureflag/api/src/main/kotlin/io/element/android/libraries/featureflag/api/FeatureFlags.kt @@ -136,6 +136,13 @@ enum class FeatureFlags( defaultValue = { false }, isFinished = false, ), + JumpToUnread( + key = "feature.jump_to_unread", + title = "Jump to unread messages", + description = "Show a button to jump to the read marker, plus a count badge on the scroll-to-bottom button when new messages arrive while scrolled away.", + defaultValue = { false }, + isFinished = false, + ), SlashCommand( key = "feature.slash_command", title = "Parse slash commands in the message composer", diff --git a/libraries/ui-strings/src/main/res/values/temporary.xml b/libraries/ui-strings/src/main/res/values/temporary.xml index ba6c431d8b5..26c83aaa530 100644 --- a/libraries/ui-strings/src/main/res/values/temporary.xml +++ b/libraries/ui-strings/src/main/res/values/temporary.xml @@ -7,4 +7,9 @@ "Black" + "Jump to unread messages" + + "Jump to %1$d unread message" + "Jump to %1$d unread messages" + diff --git a/tests/konsist/src/test/kotlin/io/element/android/tests/konsist/KonsistPreviewTest.kt b/tests/konsist/src/test/kotlin/io/element/android/tests/konsist/KonsistPreviewTest.kt index 433237cd13a..8817561e8f8 100644 --- a/tests/konsist/src/test/kotlin/io/element/android/tests/konsist/KonsistPreviewTest.kt +++ b/tests/konsist/src/test/kotlin/io/element/android/tests/konsist/KonsistPreviewTest.kt @@ -160,6 +160,8 @@ class KonsistPreviewTest { "TimelineItemVoiceViewUnifiedPreview", "TimelineVideoWithCaptionRowPreview", "TimelineViewMessageShieldPreview", + "TimelineViewWithReadMarkerDotBadgesPreview", + "TimelineViewWithReadMarkerNoBadgesPreview", "UserAvatarColorsPreview", "UserProfileHeaderSectionWithVerificationViolationPreview", "VoiceItemViewPlayPreview", From 9f26079b842ea6a53b4189fa8a33cb21b677d142 Mon Sep 17 00:00:00 2001 From: Jenna Vassar <5023996+jennaharris7@users.noreply.github.com> Date: Wed, 29 Apr 2026 16:34:47 -0700 Subject: [PATCH 02/20] Add missing preview: both fabs have counts --- .../android/features/messages/impl/timeline/TimelineView.kt | 6 ++++++ .../io/element/android/tests/konsist/KonsistPreviewTest.kt | 1 + 2 files changed, 7 insertions(+) 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 8c7d890d2f9..cef9af5dd59 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 @@ -598,3 +598,9 @@ internal fun TimelineViewWithReadMarkerPreview() = ElementPreview { internal fun TimelineViewWithReadMarkerDotBadgesPreview() = ElementPreview { TimelineViewWithReadMarker(unreadMessagesCount = 47, newMessagesCount = 99) } + +@PreviewsDayNight +@Composable +internal fun TimelineViewWithReadMarkerBothCountsPreview() = ElementPreview { + TimelineViewWithReadMarker(unreadMessagesCount = 5, newMessagesCount = 3) +} diff --git a/tests/konsist/src/test/kotlin/io/element/android/tests/konsist/KonsistPreviewTest.kt b/tests/konsist/src/test/kotlin/io/element/android/tests/konsist/KonsistPreviewTest.kt index 8817561e8f8..cbe4b674ccc 100644 --- a/tests/konsist/src/test/kotlin/io/element/android/tests/konsist/KonsistPreviewTest.kt +++ b/tests/konsist/src/test/kotlin/io/element/android/tests/konsist/KonsistPreviewTest.kt @@ -160,6 +160,7 @@ class KonsistPreviewTest { "TimelineItemVoiceViewUnifiedPreview", "TimelineVideoWithCaptionRowPreview", "TimelineViewMessageShieldPreview", + "TimelineViewWithReadMarkerBothCountsPreview", "TimelineViewWithReadMarkerDotBadgesPreview", "TimelineViewWithReadMarkerNoBadgesPreview", "UserAvatarColorsPreview", From 2579e68edcbeed68cf6a840040e80d7e694ac439 Mon Sep 17 00:00:00 2001 From: Jenna Vassar <5023996+jennaharris7@users.noreply.github.com> Date: Wed, 29 Apr 2026 17:29:51 -0700 Subject: [PATCH 03/20] Don't show NEW timeline divider when not applicable in previews --- .../messages/impl/timeline/TimelineStateProvider.kt | 8 -------- .../features/messages/impl/timeline/TimelineView.kt | 10 ++++++---- 2 files changed, 6 insertions(+), 12 deletions(-) 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 e30eb1be972..08ddf56c8c8 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 @@ -23,7 +23,6 @@ import io.element.android.features.messages.impl.timeline.model.anAggregatedReac import io.element.android.features.messages.impl.timeline.model.event.TimelineItemEventContent import io.element.android.features.messages.impl.timeline.model.event.aTimelineItemStateEventContent import io.element.android.features.messages.impl.timeline.model.event.aTimelineItemTextContent -import io.element.android.features.messages.impl.timeline.model.virtual.TimelineItemReadMarkerModel import io.element.android.features.messages.impl.timeline.model.virtual.aTimelineItemDaySeparatorModel import io.element.android.features.messages.impl.typing.TypingNotificationState import io.element.android.features.messages.impl.typing.aTypingNotificationState @@ -90,13 +89,6 @@ fun aTimelineState( ) } -internal fun aTimelineItemReadMarker(): TimelineItem.Virtual { - return TimelineItem.Virtual( - id = UniqueId(UUID.randomUUID().toString()), - model = TimelineItemReadMarkerModel, - ) -} - internal fun aTimelineItemList(content: TimelineItemEventContent): ImmutableList { return persistentListOf( // 3 items (First Middle Last) with isMine = false 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 cef9af5dd59..51bd67a3829 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 @@ -546,12 +546,11 @@ private fun TimelineViewWithReadMarker( unreadMessagesCount: Int, newMessagesCount: Int, ) { - val readMarker = aTimelineItemReadMarker() val timelineItems = persistentListOf( aTimelineItemEvent(isMine = false), aTimelineItemEvent(isMine = false), aTimelineItemEvent(isMine = true), - readMarker, + aTimelineItemEvent(isMine = false), aTimelineItemEvent(isMine = false), aTimelineItemEvent(isMine = false), ) @@ -561,7 +560,10 @@ private fun TimelineViewWithReadMarker( TimelineView( state = aTimelineState( timelineItems = timelineItems, - readMarkerIndex = timelineItems.indexOf(readMarker), + // 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. + readMarkerIndex = timelineItems.size, unreadMessagesCount = unreadMessagesCount, newMessagesCount = newMessagesCount, ), @@ -590,7 +592,7 @@ internal fun TimelineViewWithReadMarkerNoBadgesPreview() = ElementPreview { @PreviewsDayNight @Composable internal fun TimelineViewWithReadMarkerPreview() = ElementPreview { - TimelineViewWithReadMarker(unreadMessagesCount = 3, newMessagesCount = 12) + TimelineViewWithReadMarker(unreadMessagesCount = 3, newMessagesCount = 0) } @PreviewsDayNight From ef7cb35de81c8c2d69f56d1eef62a5bb567efbc0 Mon Sep 17 00:00:00 2001 From: Jenna Vassar <5023996+jennaharris7@users.noreply.github.com> Date: Wed, 29 Apr 2026 18:13:38 -0700 Subject: [PATCH 04/20] Quality checks/fixes --- .../impl/timeline/TimelinePresenter.kt | 22 ++++++++++--------- .../messages/impl/timeline/TimelineView.kt | 2 +- .../libraries/featureflag/api/FeatureFlags.kt | 3 ++- 3 files changed, 15 insertions(+), 12 deletions(-) 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 82f7717a8d7..5462112fd48 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 @@ -296,11 +296,7 @@ class TimelinePresenter( markerIdx = i break } - if (item is TimelineItem.Event && - !item.isMine && - item.origin != TimelineItemEventOrigin.PAGINATION && - item.content.isMessageContent() - ) { + if (item is TimelineItem.Event && item.isCountableNewMessage()) { unread++ } } @@ -464,11 +460,7 @@ class TimelinePresenter( var delta = 0 for (item in timelineItems) { if (item.identifier() == prevMostRecentItemIdValue) break - if (item is TimelineItem.Event && - item.origin != TimelineItemEventOrigin.PAGINATION && - !item.isMine && - item.content.isMessageContent() - ) { + if (item is TimelineItem.Event && item.isCountableNewMessage()) { delta++ } } @@ -519,6 +511,16 @@ 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/TimelineView.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineView.kt index 51bd67a3829..ce62f1ef9f6 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 @@ -598,7 +598,7 @@ internal fun TimelineViewWithReadMarkerPreview() = ElementPreview { @PreviewsDayNight @Composable internal fun TimelineViewWithReadMarkerDotBadgesPreview() = ElementPreview { - TimelineViewWithReadMarker(unreadMessagesCount = 47, newMessagesCount = 99) + TimelineViewWithReadMarker(unreadMessagesCount = 47, newMessagesCount = 0) } @PreviewsDayNight diff --git a/libraries/featureflag/api/src/main/kotlin/io/element/android/libraries/featureflag/api/FeatureFlags.kt b/libraries/featureflag/api/src/main/kotlin/io/element/android/libraries/featureflag/api/FeatureFlags.kt index 60e164df0f1..6c8f9e9d8ff 100644 --- a/libraries/featureflag/api/src/main/kotlin/io/element/android/libraries/featureflag/api/FeatureFlags.kt +++ b/libraries/featureflag/api/src/main/kotlin/io/element/android/libraries/featureflag/api/FeatureFlags.kt @@ -139,7 +139,8 @@ enum class FeatureFlags( JumpToUnread( key = "feature.jump_to_unread", title = "Jump to unread messages", - description = "Show a button to jump to the read marker, plus a count badge on the scroll-to-bottom button when new messages arrive while scrolled away.", + description = "Show a button to jump to the read marker, plus a count badge on the scroll-to-bottom button " + + "when new messages arrive while scrolled away.", defaultValue = { false }, isFinished = false, ), From 78aa500d23eaf00fabfa47c30b902c327848a6a7 Mon Sep 17 00:00:00 2001 From: Jenna Vassar <5023996+jennaharris7@users.noreply.github.com> Date: Fri, 1 May 2026 08:16:35 -0700 Subject: [PATCH 05/20] Fold newMessagesCount into NewEventState.FromOther --- .../impl/timeline/TimelinePresenter.kt | 29 +++------ .../messages/impl/timeline/TimelineState.kt | 1 - .../impl/timeline/TimelineStateProvider.kt | 5 +- .../messages/impl/timeline/TimelineView.kt | 13 ++-- .../impl/timeline/model/NewEventState.kt | 16 +++-- .../impl/timeline/TimelinePresenterTest.kt | 62 +++++++++---------- 6 files changed, 61 insertions(+), 65 deletions(-) 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 5462112fd48..fe457964289 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 ccd4d44ea81..12dd01d57ae 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 08ddf56c8c8..7fc3386ca7f 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 ce62f1ef9f6..54930a2f2bf 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 cac2798fdd8..eb60d6780fe 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 e4ddbf6c82d..502308cfe7e 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() } From 7148f2f28f6d012c1eca395233396a4199ae9140 Mon Sep 17 00:00:00 2001 From: Jenna Vassar <5023996+jennaharris7@users.noreply.github.com> Date: Fri, 1 May 2026 09:01:32 -0700 Subject: [PATCH 06/20] Semantic fixes: clearer naming conventions, when statements --- .../impl/timeline/TimelinePresenter.kt | 7 ++-- .../messages/impl/timeline/TimelineView.kt | 35 +++++++++---------- .../tests/konsist/KonsistPreviewTest.kt | 1 + 3 files changed, 22 insertions(+), 21 deletions(-) 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 fe457964289..dbb23b774eb 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 @@ -278,9 +278,10 @@ class TimelinePresenter( computeNewItemState(timelineItems, prevMostRecentItemId, newEventState) } - // Read marker position + unread count, scanned off the main thread. The UI gates display via - // [displayJumpToUnread]; the values are always computed so the state stays up to date if the - // feature flag flips at runtime. + // Keyed on the full [timelineItems] reference (not just .size) so we re-scan when the + // 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 readMarkerIndex = remember { mutableIntStateOf(-1) } val unreadMessagesCount = remember { mutableIntStateOf(0) } LaunchedEffect(timelineItems) { 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 54930a2f2bf..9a324afd2f8 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 @@ -322,15 +322,15 @@ private fun BoxScope.TimelineScrollHelper( lazyListState.firstVisibleItemIndex < 3 && isLive } } - val isReadMarkerOffTop by remember { + val isJumpToUnreadVisible by remember { derivedStateOf { - if (!displayJumpToUnread || readMarkerIndex < 0) { - false - } else if (forceJumpToReadMarkerVisibility) { - true - } else { - val lastVisibleIndex = lazyListState.layoutInfo.visibleItemsInfo.lastOrNull()?.index ?: return@derivedStateOf false - readMarkerIndex > lastVisibleIndex + when { + forceJumpToReadMarkerVisibility -> true + !displayJumpToUnread || readMarkerIndex < 0 -> false + else -> { + val lastVisibleIndex = lazyListState.layoutInfo.visibleItemsInfo.lastOrNull()?.index ?: return@derivedStateOf false + readMarkerIndex > lastVisibleIndex + } } } } @@ -402,7 +402,7 @@ private fun BoxScope.TimelineScrollHelper( } } - TimelineFab( + JumpToPositionButton( icon = CompoundIcons.ChevronDown(), contentDescription = stringResource(id = CommonStrings.a11y_jump_to_bottom), isVisible = isJumpToBottomVisible, @@ -418,10 +418,10 @@ private fun BoxScope.TimelineScrollHelper( } else { stringResource(id = CommonStrings.a11y_jump_to_unread_messages) } - TimelineFab( + JumpToPositionButton( icon = CompoundIcons.ChevronUp(), contentDescription = jumpToUnreadDescription, - isVisible = isReadMarkerOffTop, + isVisible = isJumpToUnreadVisible, count = unreadMessagesCount, // Top padding includes [topInset] so the FAB sits below any pinned-events banner. modifier = Modifier @@ -432,7 +432,7 @@ private fun BoxScope.TimelineScrollHelper( } @Composable -private fun TimelineFab( +private fun JumpToPositionButton( icon: ImageVector, contentDescription: String, isVisible: Boolean, @@ -480,9 +480,9 @@ private fun TimelineCountBadge( count: Int, modifier: Modifier = Modifier, ) { - if (count <= 0) return - if (count <= 9) { - Box( + when { + count <= 0 -> return + count <= 9 -> Box( modifier = modifier .defaultMinSize(minWidth = 16.dp, minHeight = 16.dp) .background(color = ElementTheme.colors.bgActionPrimaryRest, shape = CircleShape) @@ -497,8 +497,7 @@ private fun TimelineCountBadge( textAlign = TextAlign.Center, ) } - } else { - Box( + else -> Box( modifier = modifier .size(12.dp) .background(color = ElementTheme.colors.bgActionPrimaryRest, shape = CircleShape) @@ -592,7 +591,7 @@ internal fun TimelineViewWithReadMarkerNoBadgesPreview() = ElementPreview { @PreviewsDayNight @Composable -internal fun TimelineViewWithReadMarkerPreview() = ElementPreview { +internal fun TimelineViewWithReadMarkerNumericBadgePreview() = ElementPreview { TimelineViewWithReadMarker(unreadMessagesCount = 3, newMessagesCount = 0) } diff --git a/tests/konsist/src/test/kotlin/io/element/android/tests/konsist/KonsistPreviewTest.kt b/tests/konsist/src/test/kotlin/io/element/android/tests/konsist/KonsistPreviewTest.kt index cbe4b674ccc..23a91fda00f 100644 --- a/tests/konsist/src/test/kotlin/io/element/android/tests/konsist/KonsistPreviewTest.kt +++ b/tests/konsist/src/test/kotlin/io/element/android/tests/konsist/KonsistPreviewTest.kt @@ -163,6 +163,7 @@ class KonsistPreviewTest { "TimelineViewWithReadMarkerBothCountsPreview", "TimelineViewWithReadMarkerDotBadgesPreview", "TimelineViewWithReadMarkerNoBadgesPreview", + "TimelineViewWithReadMarkerNumericBadgePreview", "UserAvatarColorsPreview", "UserProfileHeaderSectionWithVerificationViolationPreview", "VoiceItemViewPlayPreview", From 200b98b72b2b8ae106dd246613a5369536780b58 Mon Sep 17 00:00:00 2001 From: Jenna Vassar <5023996+jennaharris7@users.noreply.github.com> Date: Fri, 1 May 2026 09:13:40 -0700 Subject: [PATCH 07/20] Consolidate JumpToUnreadState --- .../impl/timeline/TimelinePresenter.kt | 25 ++++---- .../messages/impl/timeline/TimelineState.kt | 5 +- .../impl/timeline/TimelineStateProvider.kt | 9 +-- .../messages/impl/timeline/TimelineView.kt | 29 ++++----- .../impl/timeline/model/JumpToUnreadState.kt | 32 ++++++++++ .../impl/timeline/TimelinePresenterTest.kt | 60 +++++++++++-------- 6 files changed, 94 insertions(+), 66 deletions(-) create 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 dbb23b774eb..6e0145f7429 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 @@ -14,14 +14,12 @@ import androidx.compose.runtime.MutableState import androidx.compose.runtime.collectAsState import androidx.compose.runtime.derivedStateOf import androidx.compose.runtime.getValue -import androidx.compose.runtime.mutableIntStateOf import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.produceState import androidx.compose.runtime.remember import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.runtime.saveable.rememberSaveable import androidx.compose.runtime.setValue -import androidx.compose.runtime.snapshots.Snapshot import dev.zacsweers.metro.Assisted import dev.zacsweers.metro.AssistedFactory import dev.zacsweers.metro.AssistedInject @@ -32,6 +30,7 @@ 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 @@ -282,11 +281,14 @@ 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 readMarkerIndex = remember { mutableIntStateOf(-1) } - val unreadMessagesCount = remember { mutableIntStateOf(0) } - LaunchedEffect(timelineItems) { + val jumpToUnreadState = remember { mutableStateOf(JumpToUnreadState.Disabled) } + LaunchedEffect(timelineItems, displayJumpToUnread) { + if (!displayJumpToUnread) { + jumpToUnreadState.value = JumpToUnreadState.Disabled + return@LaunchedEffect + } val items = timelineItems - withContext(dispatchers.computation) { + jumpToUnreadState.value = withContext(dispatchers.computation) { var markerIdx = -1 var unread = 0 for ((i, item) in items.withIndex()) { @@ -298,12 +300,7 @@ class TimelinePresenter( unread++ } } - // Apply both writes atomically so consumers never see a half-updated pair - // (e.g. a non-negative markerIdx with the previous unread count). - Snapshot.withMutableSnapshot { - readMarkerIndex.intValue = markerIdx - unreadMessagesCount.intValue = if (markerIdx < 0) 0 else unread - } + if (markerIdx < 0) JumpToUnreadState.NoMarker else JumpToUnreadState.Loaded(markerIdx, unread) } } @@ -356,9 +353,7 @@ class TimelinePresenter( resolveVerifiedUserSendFailureState = resolveVerifiedUserSendFailureState, displayThreadSummaries = displayThreadSummaries, displayFloatingDateBadge = displayFloatingDateBadge, - displayJumpToUnread = displayJumpToUnread, - readMarkerIndex = readMarkerIndex.intValue, - unreadMessagesCount = unreadMessagesCount.intValue, + jumpToUnreadState = jumpToUnreadState.value, eventSink = ::handleEvent, ) } 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 12dd01d57ae..6f48a8d9fc8 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,6 +11,7 @@ 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 @@ -35,9 +36,7 @@ data class TimelineState( val resolveVerifiedUserSendFailureState: ResolveVerifiedUserSendFailureState, val displayThreadSummaries: Boolean, val displayFloatingDateBadge: Boolean, - val displayJumpToUnread: Boolean, - val readMarkerIndex: Int, - val unreadMessagesCount: Int, + val jumpToUnreadState: JumpToUnreadState, 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 7fc3386ca7f..0ea48c3c354 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,6 +12,7 @@ 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 @@ -57,9 +58,7 @@ fun aTimelineState( resolveVerifiedUserSendFailureState: ResolveVerifiedUserSendFailureState = aResolveVerifiedUserSendFailureState(), displayThreadSummaries: Boolean = false, displayFloatingDateBadge: Boolean = false, - displayJumpToUnread: Boolean = true, - readMarkerIndex: Int = -1, - unreadMessagesCount: Int = 0, + jumpToUnreadState: JumpToUnreadState = JumpToUnreadState.NoMarker, newEventState: NewEventState = NewEventState.None, eventSink: (TimelineEvent) -> Unit = {}, ): TimelineState { @@ -81,9 +80,7 @@ fun aTimelineState( resolveVerifiedUserSendFailureState = resolveVerifiedUserSendFailureState, displayThreadSummaries = displayThreadSummaries, displayFloatingDateBadge = displayFloatingDateBadge, - displayJumpToUnread = displayJumpToUnread, - readMarkerIndex = readMarkerIndex, - unreadMessagesCount = unreadMessagesCount, + jumpToUnreadState = jumpToUnreadState, 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 9a324afd2f8..b93f9e950d5 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 @@ -65,6 +65,7 @@ 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 @@ -220,9 +221,7 @@ fun TimelineView( newEventState = state.newEventState, isLive = state.isLive, focusRequestState = state.focusRequestState, - readMarkerIndex = state.readMarkerIndex, - unreadMessagesCount = state.unreadMessagesCount, - displayJumpToUnread = state.displayJumpToUnread, + jumpToUnreadState = state.jumpToUnreadState, topInset = floatingDateTopOffset, onScrollFinishAt = ::onScrollFinishAt, onJumpToLive = ::onJumpToLive, @@ -307,9 +306,7 @@ private fun BoxScope.TimelineScrollHelper( forceJumpToBottomVisibility: Boolean, forceJumpToReadMarkerVisibility: Boolean, focusRequestState: FocusRequestState, - readMarkerIndex: Int, - unreadMessagesCount: Int, - displayJumpToUnread: Boolean, + jumpToUnreadState: JumpToUnreadState, topInset: Dp, onScrollFinishAt: (Int) -> Unit, onJumpToLive: () -> Unit, @@ -326,10 +323,10 @@ private fun BoxScope.TimelineScrollHelper( derivedStateOf { when { forceJumpToReadMarkerVisibility -> true - !displayJumpToUnread || readMarkerIndex < 0 -> false + jumpToUnreadState !is JumpToUnreadState.Loaded -> false else -> { val lastVisibleIndex = lazyListState.layoutInfo.visibleItemsInfo.lastOrNull()?.index ?: return@derivedStateOf false - readMarkerIndex > lastVisibleIndex + jumpToUnreadState.markerIndex > lastVisibleIndex } } } @@ -362,9 +359,9 @@ private fun BoxScope.TimelineScrollHelper( } fun jumpToReadMarker() { - if (readMarkerIndex < 0) return + val loaded = jumpToUnreadState as? JumpToUnreadState.Loaded ?: return coroutineScope.launch { - lazyListState.animateScrollToItemCenter(readMarkerIndex) + lazyListState.animateScrollToItemCenter(loaded.markerIndex) } } @@ -407,14 +404,15 @@ 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) newEventState.messageCount else 0, + count = if (jumpToUnreadState is JumpToUnreadState.Disabled) 0 else newEventState.messageCount, modifier = Modifier .align(Alignment.BottomEnd) .padding(end = 24.dp, bottom = 12.dp), onClick = ::jumpToBottom, ) - val jumpToUnreadDescription = if (unreadMessagesCount > 0) { - pluralStringResource(CommonPlurals.a11y_jump_to_unread_messages_count, unreadMessagesCount, unreadMessagesCount) + 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) } @@ -422,7 +420,7 @@ private fun BoxScope.TimelineScrollHelper( icon = CompoundIcons.ChevronUp(), contentDescription = jumpToUnreadDescription, isVisible = isJumpToUnreadVisible, - count = unreadMessagesCount, + count = unreadCount, // Top padding includes [topInset] so the FAB sits below any pinned-events banner. modifier = Modifier .align(Alignment.TopEnd) @@ -563,8 +561,7 @@ private fun TimelineViewWithReadMarker( // 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. - readMarkerIndex = timelineItems.size, - unreadMessagesCount = unreadMessagesCount, + jumpToUnreadState = JumpToUnreadState.Loaded(markerIndex = timelineItems.size, unreadCount = unreadMessagesCount), newEventState = if (newMessagesCount > 0) NewEventState.FromOther(newMessagesCount) else NewEventState.None, ), timelineProtectionState = aTimelineProtectionState(), 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 new file mode 100644 index 00000000000..81544450155 --- /dev/null +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/JumpToUnreadState.kt @@ -0,0 +1,32 @@ +/* + * 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/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 502308cfe7e..bec7895c638 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,6 +16,7 @@ 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 @@ -27,6 +28,7 @@ import io.element.android.features.poll.api.actions.SendPollResponseAction import io.element.android.features.poll.test.actions.FakeEndPollAction import io.element.android.features.poll.test.actions.FakeSendPollResponseAction import io.element.android.features.roomcall.api.aStandByCallState +import io.element.android.libraries.featureflag.api.FeatureFlags import io.element.android.libraries.featureflag.test.FakeFeatureFlagService import io.element.android.libraries.matrix.api.core.EventId import io.element.android.libraries.matrix.api.core.RoomId @@ -372,14 +374,15 @@ class TimelinePresenterTest { } @Test - fun `present - unreadMessagesCount counts message-content items between newest and read marker, excluding state events`() = runTest { + fun `present - jumpToUnreadState reports loaded marker and unread count, excluding state events`() = runTest { val timelineItems = MutableStateFlow(emptyList()) val timeline = FakeTimeline(timelineItems = timelineItems) - val presenter = createTimelinePresenter(timeline) + val presenter = createTimelinePresenter( + timeline = timeline, + featureFlagService = FakeFeatureFlagService(initialState = mapOf(FeatureFlags.JumpToUnread.key to true)), + ) presenter.test { - val initialState = awaitFirstItem() - assertThat(initialState.readMarkerIndex).isEqualTo(-1) - assertThat(initialState.unreadMessagesCount).isEqualTo(0) + awaitFirstItem() // SDK delivers items oldest-first; the factory reverses so output index 0 is the newest. // After processing: [msg-newest, membership, msg-2, read-marker, msg-old] timelineItems.emit( @@ -391,20 +394,22 @@ class TimelinePresenterTest { MatrixTimelineItem.Event(UniqueId("msg-newest"), anEventTimelineItem(content = aMessageContent())), ) ) - consumeItemsUntilPredicate { it.readMarkerIndex >= 0 }.last().also { state -> + consumeItemsUntilPredicate { it.jumpToUnreadState is JumpToUnreadState.Loaded }.last().also { state -> // 2 message items above the marker; the membership state event is skipped. - assertThat(state.readMarkerIndex).isEqualTo(3) - assertThat(state.unreadMessagesCount).isEqualTo(2) + assertThat(state.jumpToUnreadState).isEqualTo(JumpToUnreadState.Loaded(markerIndex = 3, unreadCount = 2)) } cancelAndIgnoreRemainingEvents() } } @Test - fun `present - unreadMessagesCount excludes own messages and PAGINATION-origin events`() = runTest { + 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) + 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] @@ -420,33 +425,34 @@ class TimelinePresenterTest { MatrixTimelineItem.Event(UniqueId("other-newest"), anEventTimelineItem(content = aMessageContent())), ) ) - consumeItemsUntilPredicate { it.readMarkerIndex >= 0 }.last().also { state -> - assertThat(state.readMarkerIndex).isEqualTo(3) + consumeItemsUntilPredicate { it.jumpToUnreadState is JumpToUnreadState.Loaded }.last().also { state -> // Only `other-newest` counts: own-msg and paginated are filtered out. - assertThat(state.unreadMessagesCount).isEqualTo(1) + assertThat(state.jumpToUnreadState).isEqualTo(JumpToUnreadState.Loaded(markerIndex = 3, unreadCount = 1)) } cancelAndIgnoreRemainingEvents() } } @Test - fun `present - readMarkerIndex is -1 when no read marker present`() = runTest { + fun `present - jumpToUnreadState is NoMarker when no read marker present`() = runTest { val timelineItems = MutableStateFlow(emptyList()) val timeline = FakeTimeline(timelineItems = timelineItems) - val presenter = createTimelinePresenter(timeline) + val presenter = createTimelinePresenter( + timeline = timeline, + featureFlagService = FakeFeatureFlagService(initialState = mapOf(FeatureFlags.JumpToUnread.key to true)), + ) presenter.test { - val initialState = awaitFirstItem() - assertThat(initialState.readMarkerIndex).isEqualTo(-1) - assertThat(initialState.unreadMessagesCount).isEqualTo(0) + awaitFirstItem() timelineItems.emit( listOf( MatrixTimelineItem.Event(UniqueId("1"), anEventTimelineItem(content = aMessageContent())), MatrixTimelineItem.Event(UniqueId("2"), anEventTimelineItem(content = aMessageContent())), ) ) - consumeItemsUntilPredicate { it.timelineItems.size == 2 }.last().also { state -> - assertThat(state.readMarkerIndex).isEqualTo(-1) - assertThat(state.unreadMessagesCount).isEqualTo(0) + consumeItemsUntilPredicate { + it.timelineItems.size == 2 && it.jumpToUnreadState == JumpToUnreadState.NoMarker + }.last().also { state -> + assertThat(state.jumpToUnreadState).isEqualTo(JumpToUnreadState.NoMarker) } cancelAndIgnoreRemainingEvents() } @@ -614,18 +620,20 @@ class TimelinePresenterTest { } @Test - fun `present - readMarkerIndex is 0 when the read marker is the only item`() = runTest { + fun `present - jumpToUnreadState markerIndex is 0 when the read marker is the only item`() = runTest { val timelineItems = MutableStateFlow(emptyList()) val timeline = FakeTimeline(timelineItems = timelineItems) - val presenter = createTimelinePresenter(timeline) + val presenter = createTimelinePresenter( + timeline = timeline, + featureFlagService = FakeFeatureFlagService(initialState = mapOf(FeatureFlags.JumpToUnread.key to true)), + ) presenter.test { awaitFirstItem() timelineItems.emit( listOf(MatrixTimelineItem.Virtual(UniqueId("read-marker"), VirtualTimelineItem.ReadMarker)) ) - consumeItemsUntilPredicate { it.readMarkerIndex >= 0 }.last().also { state -> - assertThat(state.readMarkerIndex).isEqualTo(0) - assertThat(state.unreadMessagesCount).isEqualTo(0) + consumeItemsUntilPredicate { it.jumpToUnreadState is JumpToUnreadState.Loaded }.last().also { state -> + assertThat(state.jumpToUnreadState).isEqualTo(JumpToUnreadState.Loaded(markerIndex = 0, unreadCount = 0)) } cancelAndIgnoreRemainingEvents() } 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 08/20] 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 | 156 +++++------------- 8 files changed, 91 insertions(+), 313 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 6e0145f7429..8dbb53d7cde 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 6f48a8d9fc8..d66dfe3031b 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 0ea48c3c354..5ba3a416181 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 b93f9e950d5..d04f55bb2c8 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) -} - -@PreviewsDayNight -@Composable -internal fun TimelineViewWithReadMarkerNumericBadgePreview() = ElementPreview { - TimelineViewWithReadMarker(unreadMessagesCount = 3, newMessagesCount = 0) -} - -@PreviewsDayNight -@Composable -internal fun TimelineViewWithReadMarkerDotBadgesPreview() = ElementPreview { - TimelineViewWithReadMarker(unreadMessagesCount = 47, newMessagesCount = 0) +internal fun TimelineViewWithReadMarkerNoIndicatorsPreview() = ElementPreview { + TimelineViewWithReadMarker(hasUnreadAbove = false, hasUnreadBelow = false) } @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 81544450155..00000000000 --- 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 eb60d6780fe..f80640dd0fd 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 fe2c264932e..9c4c48d11ea 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 bec7895c638..860d73a835d 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,16 +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 { + fun `present - readMarkerIndex is null when no read marker present`() = runTest { val timelineItems = MutableStateFlow(emptyList()) val timeline = FakeTimeline(timelineItems = timelineItems) val presenter = createTimelinePresenter( @@ -412,54 +408,48 @@ class TimelinePresenterTest { ) 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())), + MatrixTimelineItem.Event(UniqueId("1"), anEventTimelineItem(content = aMessageContent())), + MatrixTimelineItem.Event(UniqueId("2"), 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)) + consumeItemsUntilPredicate { + it.timelineItems.size == 2 && it.readMarkerIndex == null + }.last().also { state -> + assertThat(state.readMarkerIndex).isNull() } cancelAndIgnoreRemainingEvents() } } @Test - fun `present - jumpToUnreadState is NoMarker when no read marker present`() = 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 true)), + featureFlagService = FakeFeatureFlagService(initialState = mapOf(FeatureFlags.JumpToUnread.key to false)), ) presenter.test { awaitFirstItem() timelineItems.emit( listOf( - MatrixTimelineItem.Event(UniqueId("1"), anEventTimelineItem(content = aMessageContent())), - MatrixTimelineItem.Event(UniqueId("2"), anEventTimelineItem(content = aMessageContent())), + 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 == 2 && it.jumpToUnreadState == JumpToUnreadState.NoMarker - }.last().also { state -> - assertThat(state.jumpToUnreadState).isEqualTo(JumpToUnreadState.NoMarker) + consumeItemsUntilPredicate { it.timelineItems.size == 3 }.last().also { state -> + assertThat(state.displayJumpToUnread).isFalse() + 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 - 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) - } - 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) + assertThat(state.newEventState).isEqualTo(NewEventState.None) } 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() } From aed149731bcd4d68e49ca7fb0d87aab61591717d Mon Sep 17 00:00:00 2001 From: Jenna Vassar <5023996+jennaharris7@users.noreply.github.com> Date: Tue, 5 May 2026 17:26:57 -0700 Subject: [PATCH 09/20] Add mark as read buttons --- .../messages/impl/timeline/TimelineEvent.kt | 2 + .../impl/timeline/TimelinePresenter.kt | 8 ++ .../messages/impl/timeline/TimelineView.kt | 118 +++++++++++++----- .../impl/timeline/TimelinePresenterTest.kt | 34 +++++ .../theme/components/DropdownMenuItem.kt | 3 +- .../src/main/res/values/temporary.xml | 1 + 6 files changed, 135 insertions(+), 31 deletions(-) diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineEvent.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineEvent.kt index e9a6ce55496..5330fc00d35 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineEvent.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineEvent.kt @@ -25,6 +25,8 @@ sealed interface TimelineEvent { data object HideShieldDialog : TimelineEvent + data object MarkAllAsRead : TimelineEvent + /** * Events coming from a timeline item. */ 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 8dbb53d7cde..6f5be451632 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 @@ -95,6 +95,7 @@ class TimelinePresenter( private val roomCallStatePresenter: Presenter, private val featureFlagService: FeatureFlagService, private val analyticsService: AnalyticsService, + private val markAsFullyRead: MarkAsFullyRead, ) : Presenter { private val tag = "TimelinePresenter" @@ -224,6 +225,13 @@ class TimelinePresenter( timelineController.focusOnLive() } TimelineEvent.HideShieldDialog -> messageShieldDialogData.value = null + TimelineEvent.MarkAllAsRead -> sessionCoroutineScope.launch { + val latestEventId = room.liveTimeline.getLatestEventId().getOrElse { + Timber.tag(tag).w(it, "Failed to get latest event id to mark as fully read") + return@launch + } ?: return@launch + markAsFullyRead(room.roomId, latestEventId) + } is TimelineEvent.ShowShieldDialog -> messageShieldDialogData.value = event.messageShieldData is TimelineEvent.ComputeVerifiedUserSendFailure -> { resolveVerifiedUserSendFailureState.eventSink(ResolveVerifiedUserSendFailureEvent.ComputeForMessage(event.event)) 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 d04f55bb2c8..7bc84ed7188 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 @@ -16,19 +16,24 @@ import androidx.compose.animation.scaleIn import androidx.compose.animation.scaleOut import androidx.compose.foundation.background import androidx.compose.foundation.border +import androidx.compose.foundation.clickable +import androidx.compose.foundation.combinedClickable import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.BoxScope +import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.PaddingValues +import androidx.compose.foundation.layout.Row +import androidx.compose.foundation.layout.Spacer import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.offset import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.size +import androidx.compose.foundation.layout.width import androidx.compose.foundation.lazy.LazyColumn import androidx.compose.foundation.lazy.LazyListState import androidx.compose.foundation.lazy.items import androidx.compose.foundation.lazy.rememberLazyListState import androidx.compose.foundation.shape.CircleShape -import androidx.compose.material3.FloatingActionButtonDefaults import androidx.compose.runtime.Composable import androidx.compose.runtime.CompositionLocalProvider import androidx.compose.runtime.LaunchedEffect @@ -42,6 +47,8 @@ import androidx.compose.runtime.setValue import androidx.compose.runtime.snapshotFlow import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier +import androidx.compose.ui.draw.clip +import androidx.compose.ui.draw.shadow import androidx.compose.ui.graphics.vector.ImageVector import androidx.compose.ui.input.nestedscroll.NestedScrollConnection import androidx.compose.ui.input.nestedscroll.nestedScroll @@ -51,6 +58,7 @@ import androidx.compose.ui.platform.rememberNestedScrollInteropConnection import androidx.compose.ui.res.stringResource import androidx.compose.ui.tooling.preview.PreviewParameter import androidx.compose.ui.unit.Dp +import androidx.compose.ui.unit.DpOffset import androidx.compose.ui.unit.IntOffset import androidx.compose.ui.unit.dp import io.element.android.compound.theme.ElementTheme @@ -72,8 +80,10 @@ import io.element.android.libraries.androidutils.system.copyToClipboard import io.element.android.libraries.designsystem.components.dialogs.AlertDialog 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.text.roundToPx +import io.element.android.libraries.designsystem.theme.components.DropdownMenu 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 @@ -131,6 +141,10 @@ fun TimelineView( state.eventSink(TimelineEvent.JumpToLive) } + fun onMarkAllAsRead() { + state.eventSink(TimelineEvent.MarkAllAsRead) + } + val context = LocalContext.current val toastMessage = stringResource(CommonStrings.common_copied_to_clipboard) val view = LocalView.current @@ -220,6 +234,7 @@ fun TimelineView( onScrollFinishAt = ::onScrollFinishAt, onJumpToLive = ::onJumpToLive, onFocusEventRender = ::onFocusEventRender, + onMarkAllAsRead = ::onMarkAllAsRead, ) if (state.displayFloatingDateBadge && useReverseLayout) { @@ -305,6 +320,7 @@ private fun BoxScope.TimelineScrollHelper( onScrollFinishAt: (Int) -> Unit, onJumpToLive: () -> Unit, onFocusEventRender: () -> Unit, + onMarkAllAsRead: () -> Unit, ) { val coroutineScope = rememberCoroutineScope() val isScrollFinished by remember { derivedStateOf { !lazyListState.isScrollInProgress } } @@ -389,27 +405,30 @@ private fun BoxScope.TimelineScrollHelper( } } - JumpToPositionButton( - icon = CompoundIcons.ChevronDown(), - contentDescription = stringResource(id = CommonStrings.a11y_jump_to_bottom), - isVisible = isJumpToBottomVisible, - hasUnread = displayJumpToUnread && newEventState is NewEventState.FromOther, + Column( modifier = Modifier .align(Alignment.BottomEnd) - .padding(end = 24.dp, bottom = 12.dp), - onClick = ::jumpToBottom, - ) - JumpToPositionButton( - icon = CompoundIcons.ChevronUp(), - contentDescription = stringResource(id = CommonStrings.a11y_jump_to_unread_messages), - isVisible = isJumpToUnreadVisible, - hasUnread = true, - // Stacked directly above the scroll-to-bottom FAB: 12dp base + 36dp FAB + 8dp gap. - modifier = Modifier - .align(Alignment.BottomEnd) - .padding(end = 24.dp, bottom = 56.dp), - onClick = ::jumpToReadMarker, - ) + .padding(end = 24.dp, bottom = 24.dp) + ) { + JumpToPositionButton( + icon = CompoundIcons.ChevronUp(), + contentDescription = stringResource(id = CommonStrings.a11y_jump_to_unread_messages), + modifier = Modifier.padding(bottom = if (isJumpToBottomVisible) 12.dp else 0.dp), + isVisible = isJumpToUnreadVisible, + hasUnread = true, + onClick = ::jumpToReadMarker, + onMarkAsRead = onMarkAllAsRead, + ) + JumpToPositionButton( + icon = CompoundIcons.ChevronDown(), + contentDescription = stringResource(id = CommonStrings.a11y_jump_to_bottom), + isVisible = isJumpToBottomVisible, + hasUnread = displayJumpToUnread && newEventState is NewEventState.FromOther, + onClick = ::jumpToBottom, + onMarkAsRead = onMarkAllAsRead, + dotAlignment = Alignment.BottomCenter, + ) + } } @Composable @@ -419,7 +438,9 @@ private fun JumpToPositionButton( isVisible: Boolean, hasUnread: Boolean, onClick: () -> Unit, + onMarkAsRead: () -> Unit, modifier: Modifier = Modifier, + dotAlignment: Alignment = Alignment.TopCenter, ) { AnimatedVisibility( modifier = modifier, @@ -427,26 +448,63 @@ private fun JumpToPositionButton( enter = scaleIn(animationSpec = tween(100)), exit = scaleOut(animationSpec = tween(100)), ) { + var menuExpanded by remember { mutableStateOf(false) } Box { - FloatingActionButton( - onClick = onClick, - elevation = FloatingActionButtonDefaults.elevation(4.dp, 4.dp, 4.dp, 4.dp), - shape = CircleShape, - modifier = Modifier.size(36.dp), - containerColor = ElementTheme.colors.bgSubtleSecondary, - contentColor = ElementTheme.colors.iconSecondary, + Box( + modifier = Modifier + .size(36.dp) + .shadow(elevation = 0.dp, shape = CircleShape) + .background(color = ElementTheme.colors.bgCanvasDefault, shape = CircleShape) + .clip(CircleShape) + .border(1.dp, ElementTheme.colors.borderDisabled, CircleShape) + .combinedClickable( + onClick = onClick, + onLongClick = { menuExpanded = true }, + ) + .testTag(TestTags.floatingActionButton), + contentAlignment = Alignment.Center, ) { Icon( modifier = Modifier.size(24.dp), imageVector = icon, contentDescription = contentDescription, + tint = ElementTheme.colors.iconSecondary, ) + DropdownMenu( + expanded = menuExpanded, + onDismissRequest = { menuExpanded = false }, + minWidth = 0.dp, + offset = DpOffset(x = -44.dp, y = 40.dp) + ) { + Row( + modifier = Modifier + .clickable { + menuExpanded = false + onMarkAsRead() + } + .padding(horizontal = 12.dp, vertical = 8.dp), + verticalAlignment = Alignment.CenterVertically, + ) { + Icon( + imageVector = CompoundIcons.MarkAsRead(), + contentDescription = null, + tint = ElementTheme.colors.iconPrimary, + ) + Spacer(modifier = Modifier.width(8.dp)) + Text( + text = stringResource(id = CommonStrings.action_mark_as_read), + color = ElementTheme.colors.textPrimary, + style = ElementTheme.typography.fontBodyLgRegular, + ) + } + } } + val dotYOffset = if (dotAlignment == Alignment.BottomCenter) 4.dp else (-4).dp TimelineUnreadIndicator( isVisible = hasUnread, modifier = Modifier - .align(Alignment.TopEnd) - .offset { IntOffset(x = 4.dp.roundToPx(), y = -4.dp.roundToPx()) }, + .align(dotAlignment) + .offset { IntOffset(x = 0, y = dotYOffset.roundToPx()) }, ) } } 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 860d73a835d..b5f0e938470 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 @@ -1206,6 +1206,38 @@ class TimelinePresenterTest { } } + @Test + fun `present - MarkAllAsRead invokes markAsFullyRead with latest event id`() = runTest { + val markAsFullyReadRecorder = lambdaRecorder { _, _ -> } + val presenter = createTimelinePresenter( + timeline = FakeTimeline(getLatestEventIdResult = { Result.success(AN_EVENT_ID) }), + markAsFullyRead = FakeMarkAsFullyRead(markAsFullyReadRecorder), + ) + presenter.test { + val initialState = awaitFirstItem() + initialState.eventSink(TimelineEvent.MarkAllAsRead) + advanceUntilIdle() + markAsFullyReadRecorder.assertions().isCalledOnce().with(value(A_ROOM_ID), value(AN_EVENT_ID)) + cancelAndIgnoreRemainingEvents() + } + } + + @Test + fun `present - MarkAllAsRead does not invoke markAsFullyRead when latest event id lookup fails`() = runTest { + val markAsFullyReadRecorder = lambdaRecorder { _, _ -> } + val presenter = createTimelinePresenter( + timeline = FakeTimeline(getLatestEventIdResult = { Result.failure(RuntimeException("boom")) }), + markAsFullyRead = FakeMarkAsFullyRead(markAsFullyReadRecorder), + ) + presenter.test { + val initialState = awaitFirstItem() + initialState.eventSink(TimelineEvent.MarkAllAsRead) + advanceUntilIdle() + markAsFullyReadRecorder.assertions().isNeverCalled() + cancelAndIgnoreRemainingEvents() + } + } + private suspend fun ReceiveTurbine.awaitFirstItem(): T { return awaitItem() } @@ -1244,6 +1276,7 @@ class TimelinePresenterTest { sessionPreferencesStore: InMemorySessionPreferencesStore = InMemorySessionPreferencesStore(), timelineItemIndexer: TimelineItemIndexer = TimelineItemIndexer(), featureFlagService: FakeFeatureFlagService = FakeFeatureFlagService(), + markAsFullyRead: MarkAsFullyRead = FakeMarkAsFullyRead { _, _ -> }, ): TimelinePresenter { return TimelinePresenter( timelineItemsFactoryCreator = aTimelineItemsFactoryCreator(), @@ -1262,6 +1295,7 @@ class TimelinePresenterTest { roomCallStatePresenter = { aStandByCallState() }, featureFlagService = featureFlagService, analyticsService = FakeAnalyticsService(), + markAsFullyRead = markAsFullyRead, ) } } diff --git a/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/theme/components/DropdownMenuItem.kt b/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/theme/components/DropdownMenuItem.kt index 31c3f3c49b4..5c0e0f914b9 100644 --- a/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/theme/components/DropdownMenuItem.kt +++ b/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/theme/components/DropdownMenuItem.kt @@ -35,6 +35,7 @@ fun DropdownMenuItem( leadingIcon: @Composable (() -> Unit)? = null, trailingIcon: @Composable (() -> Unit)? = null, enabled: Boolean = true, + contentPadding: PaddingValues = DropDownMenuItemDefaults.contentPadding, interactionSource: MutableInteractionSource = remember { MutableInteractionSource() }, ) { androidx.compose.material3.DropdownMenuItem( @@ -49,7 +50,7 @@ fun DropdownMenuItem( trailingIcon = trailingIcon, enabled = enabled, colors = DropDownMenuItemDefaults.colors(), - contentPadding = DropDownMenuItemDefaults.contentPadding, + contentPadding = contentPadding, interactionSource = interactionSource ) } diff --git a/libraries/ui-strings/src/main/res/values/temporary.xml b/libraries/ui-strings/src/main/res/values/temporary.xml index 26c83aaa530..e930968f8fc 100644 --- a/libraries/ui-strings/src/main/res/values/temporary.xml +++ b/libraries/ui-strings/src/main/res/values/temporary.xml @@ -7,6 +7,7 @@ "Black" + "Mark as read" "Jump to unread messages" "Jump to %1$d unread message" From 880a1896172b4e6424bf3fad8a6e669eaa4f935f Mon Sep 17 00:00:00 2001 From: Jenna Vassar <5023996+jennaharris7@users.noreply.github.com> Date: Tue, 5 May 2026 17:27:36 -0700 Subject: [PATCH 10/20] Code cleanup, accessibility checks --- .../home/impl/roomlist/RoomListContextMenu.kt | 2 +- .../impl/roomlist/RoomListContextMenuTest.kt | 2 +- .../messages/impl/timeline/TimelinePresenter.kt | 5 +++++ .../messages/impl/timeline/TimelineView.kt | 11 ++++++++--- .../impl/timeline/TimelinePresenterTest.kt | 16 ++++++++++++++++ .../theme/components/DropdownMenuItem.kt | 3 +-- .../android/libraries/testtags/TestTags.kt | 6 ++++++ 7 files changed, 38 insertions(+), 7 deletions(-) diff --git a/features/home/impl/src/main/kotlin/io/element/android/features/home/impl/roomlist/RoomListContextMenu.kt b/features/home/impl/src/main/kotlin/io/element/android/features/home/impl/roomlist/RoomListContextMenu.kt index 66982961fd9..b3da4f89c7e 100644 --- a/features/home/impl/src/main/kotlin/io/element/android/features/home/impl/roomlist/RoomListContextMenu.kt +++ b/features/home/impl/src/main/kotlin/io/element/android/features/home/impl/roomlist/RoomListContextMenu.kt @@ -111,7 +111,7 @@ private fun RoomListModalBottomSheetContent( ListItem( headlineContent = { Text( - text = stringResource(id = R.string.screen_roomlist_mark_as_read), + text = stringResource(id = CommonStrings.action_mark_as_read), style = MaterialTheme.typography.bodyLarge, ) }, diff --git a/features/home/impl/src/test/kotlin/io/element/android/features/home/impl/roomlist/RoomListContextMenuTest.kt b/features/home/impl/src/test/kotlin/io/element/android/features/home/impl/roomlist/RoomListContextMenuTest.kt index 5fa2adf9d6a..c66cb03fadd 100644 --- a/features/home/impl/src/test/kotlin/io/element/android/features/home/impl/roomlist/RoomListContextMenuTest.kt +++ b/features/home/impl/src/test/kotlin/io/element/android/features/home/impl/roomlist/RoomListContextMenuTest.kt @@ -36,7 +36,7 @@ class RoomListContextMenuTest { contextMenu = contextMenu, eventSink = eventsRecorder, ) - clickOn(R.string.screen_roomlist_mark_as_read) + clickOn(CommonStrings.action_mark_as_read) eventsRecorder.assertList( listOf( RoomListEvent.HideContextMenu, 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 6f5be451632..5fb63a4f4bc 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 @@ -287,6 +287,11 @@ 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. + // + // Limitation: when the read marker is outside the loaded window (gaps, pagination), this + // returns null and the jump-to-unread button stays hidden. Proper fix needs an SDK + // accessor for the m.fully_read marker plus FocusedOnEvent navigation on click; gated + // behind FeatureFlags.JumpToUnread until that lands. val readMarkerIndex = remember { mutableStateOf(null) } LaunchedEffect(timelineItems, displayJumpToUnread) { if (!displayJumpToUnread) { 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 7bc84ed7188..d67323abd92 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 @@ -48,7 +48,6 @@ import androidx.compose.runtime.snapshotFlow import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.draw.clip -import androidx.compose.ui.draw.shadow import androidx.compose.ui.graphics.vector.ImageVector import androidx.compose.ui.input.nestedscroll.NestedScrollConnection import androidx.compose.ui.input.nestedscroll.nestedScroll @@ -88,6 +87,7 @@ 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.TestTag import io.element.android.libraries.testtags.TestTags import io.element.android.libraries.testtags.testTag import io.element.android.libraries.ui.strings.CommonStrings @@ -418,6 +418,7 @@ private fun BoxScope.TimelineScrollHelper( hasUnread = true, onClick = ::jumpToReadMarker, onMarkAsRead = onMarkAllAsRead, + testTag = TestTags.jumpToUnreadButton, ) JumpToPositionButton( icon = CompoundIcons.ChevronDown(), @@ -426,6 +427,7 @@ private fun BoxScope.TimelineScrollHelper( hasUnread = displayJumpToUnread && newEventState is NewEventState.FromOther, onClick = ::jumpToBottom, onMarkAsRead = onMarkAllAsRead, + testTag = TestTags.jumpToBottomButton, dotAlignment = Alignment.BottomCenter, ) } @@ -439,6 +441,7 @@ private fun JumpToPositionButton( hasUnread: Boolean, onClick: () -> Unit, onMarkAsRead: () -> Unit, + testTag: TestTag, modifier: Modifier = Modifier, dotAlignment: Alignment = Alignment.TopCenter, ) { @@ -453,15 +456,15 @@ private fun JumpToPositionButton( Box( modifier = Modifier .size(36.dp) - .shadow(elevation = 0.dp, shape = CircleShape) .background(color = ElementTheme.colors.bgCanvasDefault, shape = CircleShape) .clip(CircleShape) .border(1.dp, ElementTheme.colors.borderDisabled, CircleShape) .combinedClickable( onClick = onClick, onLongClick = { menuExpanded = true }, + onLongClickLabel = stringResource(CommonStrings.action_open_context_menu), ) - .testTag(TestTags.floatingActionButton), + .testTag(testTag), contentAlignment = Alignment.Center, ) { Icon( @@ -476,6 +479,8 @@ private fun JumpToPositionButton( minWidth = 0.dp, offset = DpOffset(x = -44.dp, y = 40.dp) ) { + // Hand-rolled instead of DropdownMenuItem: padding here is tighter + // than DropdownMenuItem's 12.dp default to match the Figma spec. Row( modifier = Modifier .clickable { 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 b5f0e938470..7f083af3b00 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 @@ -1238,6 +1238,22 @@ class TimelinePresenterTest { } } + @Test + fun `present - MarkAllAsRead does not invoke markAsFullyRead when there is no latest event`() = runTest { + val markAsFullyReadRecorder = lambdaRecorder { _, _ -> } + val presenter = createTimelinePresenter( + timeline = FakeTimeline(getLatestEventIdResult = { Result.success(null) }), + markAsFullyRead = FakeMarkAsFullyRead(markAsFullyReadRecorder), + ) + presenter.test { + val initialState = awaitFirstItem() + initialState.eventSink(TimelineEvent.MarkAllAsRead) + advanceUntilIdle() + markAsFullyReadRecorder.assertions().isNeverCalled() + cancelAndIgnoreRemainingEvents() + } + } + private suspend fun ReceiveTurbine.awaitFirstItem(): T { return awaitItem() } diff --git a/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/theme/components/DropdownMenuItem.kt b/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/theme/components/DropdownMenuItem.kt index 5c0e0f914b9..31c3f3c49b4 100644 --- a/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/theme/components/DropdownMenuItem.kt +++ b/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/theme/components/DropdownMenuItem.kt @@ -35,7 +35,6 @@ fun DropdownMenuItem( leadingIcon: @Composable (() -> Unit)? = null, trailingIcon: @Composable (() -> Unit)? = null, enabled: Boolean = true, - contentPadding: PaddingValues = DropDownMenuItemDefaults.contentPadding, interactionSource: MutableInteractionSource = remember { MutableInteractionSource() }, ) { androidx.compose.material3.DropdownMenuItem( @@ -50,7 +49,7 @@ fun DropdownMenuItem( trailingIcon = trailingIcon, enabled = enabled, colors = DropDownMenuItemDefaults.colors(), - contentPadding = contentPadding, + contentPadding = DropDownMenuItemDefaults.contentPadding, interactionSource = interactionSource ) } diff --git a/libraries/testtags/src/main/kotlin/io/element/android/libraries/testtags/TestTags.kt b/libraries/testtags/src/main/kotlin/io/element/android/libraries/testtags/TestTags.kt index e564ddac414..87ca215099d 100644 --- a/libraries/testtags/src/main/kotlin/io/element/android/libraries/testtags/TestTags.kt +++ b/libraries/testtags/src/main/kotlin/io/element/android/libraries/testtags/TestTags.kt @@ -98,6 +98,12 @@ object TestTags { */ val floatingActionButton = TestTag("floating-action-button") + /** + * Timeline jump-to-position buttons (long-press exposes "Mark as read"). + */ + val jumpToUnreadButton = TestTag("jump-to-unread-button") + val jumpToBottomButton = TestTag("jump-to-bottom-button") + /** * Timeline. */ From 784f5df426008bffa72e6660d837158ef6d1f349 Mon Sep 17 00:00:00 2001 From: Jenna Vassar <5023996+jennaharris7@users.noreply.github.com> Date: Tue, 5 May 2026 18:44:11 -0700 Subject: [PATCH 11/20] Mark as read menu ui tweaks --- .../messages/impl/timeline/TimelineView.kt | 121 +++++++++++++----- 1 file changed, 90 insertions(+), 31 deletions(-) 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 d67323abd92..81f40b7a61a 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 @@ -10,8 +10,10 @@ package io.element.android.features.messages.impl.timeline import android.view.HapticFeedbackConstants import androidx.compose.animation.AnimatedVisibility +import androidx.compose.animation.core.MutableTransitionState import androidx.compose.animation.core.tween import androidx.compose.animation.fadeIn +import androidx.compose.animation.fadeOut import androidx.compose.animation.scaleIn import androidx.compose.animation.scaleOut import androidx.compose.foundation.background @@ -34,6 +36,7 @@ import androidx.compose.foundation.lazy.LazyListState import androidx.compose.foundation.lazy.items import androidx.compose.foundation.lazy.rememberLazyListState import androidx.compose.foundation.shape.CircleShape +import androidx.compose.foundation.shape.RoundedCornerShape import androidx.compose.runtime.Composable import androidx.compose.runtime.CompositionLocalProvider import androidx.compose.runtime.LaunchedEffect @@ -48,18 +51,26 @@ import androidx.compose.runtime.snapshotFlow import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.draw.clip +import androidx.compose.ui.draw.shadow +import androidx.compose.ui.graphics.TransformOrigin import androidx.compose.ui.graphics.vector.ImageVector import androidx.compose.ui.input.nestedscroll.NestedScrollConnection import androidx.compose.ui.input.nestedscroll.nestedScroll import androidx.compose.ui.platform.LocalContext +import androidx.compose.ui.platform.LocalDensity import androidx.compose.ui.platform.LocalView import androidx.compose.ui.platform.rememberNestedScrollInteropConnection import androidx.compose.ui.res.stringResource import androidx.compose.ui.tooling.preview.PreviewParameter import androidx.compose.ui.unit.Dp -import androidx.compose.ui.unit.DpOffset import androidx.compose.ui.unit.IntOffset +import androidx.compose.ui.unit.IntRect +import androidx.compose.ui.unit.IntSize +import androidx.compose.ui.unit.LayoutDirection import androidx.compose.ui.unit.dp +import androidx.compose.ui.window.Popup +import androidx.compose.ui.window.PopupPositionProvider +import androidx.compose.ui.window.PopupProperties import io.element.android.compound.theme.ElementTheme import io.element.android.compound.tokens.generated.CompoundIcons import io.element.android.features.messages.impl.crypto.sendfailure.resolve.ResolveVerifiedUserSendFailureView @@ -80,7 +91,6 @@ import io.element.android.libraries.designsystem.components.dialogs.AlertDialog import io.element.android.libraries.designsystem.preview.ElementPreview import io.element.android.libraries.designsystem.preview.PreviewsDayNight import io.element.android.libraries.designsystem.text.roundToPx -import io.element.android.libraries.designsystem.theme.components.DropdownMenu 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 @@ -448,8 +458,8 @@ private fun JumpToPositionButton( AnimatedVisibility( modifier = modifier, visible = isVisible, - enter = scaleIn(animationSpec = tween(100)), - exit = scaleOut(animationSpec = tween(100)), + enter = scaleIn(animationSpec = tween(220), initialScale = 0.8f) + fadeIn(animationSpec = tween(220)), + exit = scaleOut(animationSpec = tween(180), targetScale = 0.8f) + fadeOut(animationSpec = tween(180)), ) { var menuExpanded by remember { mutableStateOf(false) } Box { @@ -473,34 +483,60 @@ private fun JumpToPositionButton( contentDescription = contentDescription, tint = ElementTheme.colors.iconSecondary, ) - DropdownMenu( - expanded = menuExpanded, - onDismissRequest = { menuExpanded = false }, - minWidth = 0.dp, - offset = DpOffset(x = -44.dp, y = 40.dp) - ) { - // Hand-rolled instead of DropdownMenuItem: padding here is tighter - // than DropdownMenuItem's 12.dp default to match the Figma spec. - Row( - modifier = Modifier - .clickable { - menuExpanded = false - onMarkAsRead() - } - .padding(horizontal = 12.dp, vertical = 8.dp), - verticalAlignment = Alignment.CenterVertically, + val menuTransitionState = remember { MutableTransitionState(false) } + .apply { targetState = menuExpanded } + if (menuTransitionState.currentState || menuTransitionState.targetState) { + val gapPx = with(LocalDensity.current) { 8.dp.roundToPx() } + val positionProvider = remember(gapPx) { CenterStartOfAnchorPositionProvider(gapPx) } + Popup( + popupPositionProvider = positionProvider, + onDismissRequest = { menuExpanded = false }, + properties = PopupProperties(focusable = true), ) { - Icon( - imageVector = CompoundIcons.MarkAsRead(), - contentDescription = null, - tint = ElementTheme.colors.iconPrimary, - ) - Spacer(modifier = Modifier.width(8.dp)) - Text( - text = stringResource(id = CommonStrings.action_mark_as_read), - color = ElementTheme.colors.textPrimary, - style = ElementTheme.typography.fontBodyLgRegular, - ) + // Anchor the scale to the right-center edge so the menu visually grows + // outward from the FAB it's attached to. + val transformOrigin = TransformOrigin(pivotFractionX = 1f, pivotFractionY = 0.5f) + AnimatedVisibility( + visibleState = menuTransitionState, + enter = scaleIn( + animationSpec = tween(180), + initialScale = 0.9f, + transformOrigin = transformOrigin, + ) + fadeIn(animationSpec = tween(180)), + exit = scaleOut( + animationSpec = tween(140), + targetScale = 0.9f, + transformOrigin = transformOrigin, + ) + fadeOut(animationSpec = tween(140)), + ) { + // Hand-rolled instead of DropdownMenuItem: padding here is tighter + // than DropdownMenuItem's 12.dp default to match the Figma spec. + Row( + modifier = Modifier + .shadow(elevation = 1.dp, shape = RoundedCornerShape(8.dp)) + .clip(RoundedCornerShape(8.dp)) + .background(ElementTheme.colors.bgCanvasDefaultLevel1) + .border(1.dp, ElementTheme.colors.borderDisabled, RoundedCornerShape(8.dp)) + .clickable { + menuExpanded = false + onMarkAsRead() + } + .padding(horizontal = 12.dp, vertical = 12.dp), + verticalAlignment = Alignment.CenterVertically, + ) { + Icon( + imageVector = CompoundIcons.MarkAsRead(), + contentDescription = null, + tint = ElementTheme.colors.iconTertiary, + ) + Spacer(modifier = Modifier.width(8.dp)) + Text( + text = stringResource(id = CommonStrings.action_mark_as_read), + color = ElementTheme.colors.textPrimary, + style = ElementTheme.typography.fontBodyLgRegular, + ) + } + } } } } @@ -617,3 +653,26 @@ internal fun TimelineViewWithReadMarkerNoIndicatorsPreview() = ElementPreview { internal fun TimelineViewWithReadMarkerBothIndicatorsPreview() = ElementPreview { TimelineViewWithReadMarker(hasUnreadAbove = true, hasUnreadBelow = true) } + +/** + * Anchors the popup so its right edge sits [gapPx] to the left of the anchor and its vertical + * center matches the anchor's. Adapts to localized menu widths and FAB size; coerced to stay + * on-screen. + */ +private class CenterStartOfAnchorPositionProvider( + private val gapPx: Int, +) : PopupPositionProvider { + override fun calculatePosition( + anchorBounds: IntRect, + windowSize: IntSize, + layoutDirection: LayoutDirection, + popupContentSize: IntSize, + ): IntOffset { + val x = anchorBounds.left - popupContentSize.width - gapPx + val y = anchorBounds.top + (anchorBounds.height - popupContentSize.height) / 2 + return IntOffset( + x = x.coerceIn(0, (windowSize.width - popupContentSize.width).coerceAtLeast(0)), + y = y.coerceIn(0, (windowSize.height - popupContentSize.height).coerceAtLeast(0)), + ) + } +} From 0daf7ecc7dbfcf0048901b38da30299c1d7a0a3f Mon Sep 17 00:00:00 2001 From: Jenna Vassar <5023996+jennaharris7@users.noreply.github.com> Date: Wed, 6 May 2026 06:49:42 -0700 Subject: [PATCH 12/20] Konsist fixes Co-Authored-By: Claude Opus 4.7 (1M context) --- .../io/element/android/tests/konsist/KonsistPreviewTest.kt | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/konsist/src/test/kotlin/io/element/android/tests/konsist/KonsistPreviewTest.kt b/tests/konsist/src/test/kotlin/io/element/android/tests/konsist/KonsistPreviewTest.kt index 23a91fda00f..6dc0e7fc565 100644 --- a/tests/konsist/src/test/kotlin/io/element/android/tests/konsist/KonsistPreviewTest.kt +++ b/tests/konsist/src/test/kotlin/io/element/android/tests/konsist/KonsistPreviewTest.kt @@ -160,10 +160,8 @@ class KonsistPreviewTest { "TimelineItemVoiceViewUnifiedPreview", "TimelineVideoWithCaptionRowPreview", "TimelineViewMessageShieldPreview", - "TimelineViewWithReadMarkerBothCountsPreview", - "TimelineViewWithReadMarkerDotBadgesPreview", - "TimelineViewWithReadMarkerNoBadgesPreview", - "TimelineViewWithReadMarkerNumericBadgePreview", + "TimelineViewWithReadMarkerBothIndicatorsPreview", + "TimelineViewWithReadMarkerNoIndicatorsPreview", "UserAvatarColorsPreview", "UserProfileHeaderSectionWithVerificationViolationPreview", "VoiceItemViewPlayPreview", From 4b479638a4e54d104d8ecd76636671144b951d49 Mon Sep 17 00:00:00 2001 From: Jenna Vassar <5023996+jennaharris7@users.noreply.github.com> Date: Wed, 6 May 2026 11:55:52 -0700 Subject: [PATCH 13/20] Keep jump to unread button position stationary --- .../messages/impl/timeline/TimelineView.kt | 28 ++++++++++--------- 1 file changed, 15 insertions(+), 13 deletions(-) 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 81f40b7a61a..a2a40071c3c 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 @@ -339,7 +339,7 @@ private fun BoxScope.TimelineScrollHelper( lazyListState.firstVisibleItemIndex < 3 && isLive } } - val isJumpToUnreadVisible by remember { + val isJumpToUnreadVisible by remember(readMarkerIndex, forceJumpToReadMarkerVisibility) { derivedStateOf { if (forceJumpToReadMarkerVisibility) return@derivedStateOf true val markerIndex = readMarkerIndex ?: return@derivedStateOf false @@ -418,28 +418,30 @@ private fun BoxScope.TimelineScrollHelper( Column( modifier = Modifier .align(Alignment.BottomEnd) - .padding(end = 24.dp, bottom = 24.dp) + .padding(end = 24.dp, bottom = 16.dp) ) { JumpToPositionButton( icon = CompoundIcons.ChevronUp(), contentDescription = stringResource(id = CommonStrings.a11y_jump_to_unread_messages), - modifier = Modifier.padding(bottom = if (isJumpToBottomVisible) 12.dp else 0.dp), + modifier = Modifier.padding(bottom = 12.dp), isVisible = isJumpToUnreadVisible, hasUnread = true, onClick = ::jumpToReadMarker, onMarkAsRead = onMarkAllAsRead, testTag = TestTags.jumpToUnreadButton, ) - JumpToPositionButton( - icon = CompoundIcons.ChevronDown(), - contentDescription = stringResource(id = CommonStrings.a11y_jump_to_bottom), - isVisible = isJumpToBottomVisible, - hasUnread = displayJumpToUnread && newEventState is NewEventState.FromOther, - onClick = ::jumpToBottom, - onMarkAsRead = onMarkAllAsRead, - testTag = TestTags.jumpToBottomButton, - dotAlignment = Alignment.BottomCenter, - ) + Box(modifier = Modifier.size(36.dp)) { + JumpToPositionButton( + icon = CompoundIcons.ChevronDown(), + contentDescription = stringResource(id = CommonStrings.a11y_jump_to_bottom), + isVisible = isJumpToBottomVisible, + hasUnread = displayJumpToUnread && newEventState is NewEventState.FromOther, + onClick = ::jumpToBottom, + onMarkAsRead = onMarkAllAsRead, + testTag = TestTags.jumpToBottomButton, + dotAlignment = Alignment.BottomCenter, + ) + } } } From e0046e9862bc4d0475e5a60b7cd98363e465230f Mon Sep 17 00:00:00 2001 From: Jenna Vassar <5023996+jennaharris7@users.noreply.github.com> Date: Mon, 11 May 2026 11:27:31 -0700 Subject: [PATCH 14/20] Return null instead of return@launch Co-authored-by: Benoit Marty --- .../features/messages/impl/timeline/TimelinePresenter.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 5fb63a4f4bc..2507b046f26 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 @@ -228,7 +228,7 @@ class TimelinePresenter( TimelineEvent.MarkAllAsRead -> sessionCoroutineScope.launch { val latestEventId = room.liveTimeline.getLatestEventId().getOrElse { Timber.tag(tag).w(it, "Failed to get latest event id to mark as fully read") - return@launch + null } ?: return@launch markAsFullyRead(room.roomId, latestEventId) } From 65ed527384a6d77ad5c804b421ed473f03a10197 Mon Sep 17 00:00:00 2001 From: Jenna Vassar <5023996+jennaharris7@users.noreply.github.com> Date: Mon, 11 May 2026 11:34:28 -0700 Subject: [PATCH 15/20] Add comment to explain jump to bottom button containing box --- .../android/features/messages/impl/timeline/TimelineView.kt | 2 ++ 1 file changed, 2 insertions(+) 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 a2a40071c3c..443fc1681ad 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 @@ -430,6 +430,8 @@ private fun BoxScope.TimelineScrollHelper( onMarkAsRead = onMarkAllAsRead, testTag = TestTags.jumpToUnreadButton, ) + // Reserves space for the jump to bottom button so the jump to unread button above + // stays in the same position regardless of whether the jump to bottom button is visible. Box(modifier = Modifier.size(36.dp)) { JumpToPositionButton( icon = CompoundIcons.ChevronDown(), From 8d8ac9373efc1cae4d868868025669dc11f226fe Mon Sep 17 00:00:00 2001 From: Jenna Vassar <5023996+jennaharris7@users.noreply.github.com> Date: Mon, 11 May 2026 11:59:10 -0700 Subject: [PATCH 16/20] Rename preview to reflect actual state --- .../android/features/messages/impl/timeline/TimelineView.kt | 5 ++++- .../io/element/android/tests/konsist/KonsistPreviewTest.kt | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) 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 443fc1681ad..cec1d57e588 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 @@ -604,6 +604,9 @@ internal fun TimelineViewPreview( } } +// The jump-to-unread FAB's indicator is always rendered when the FAB is visible — in +// production the FAB only appears when unread content exists above. So `hasUnreadAbove` +// here only varies the scroll-target state, not the upper indicator's visibility. @Composable private fun TimelineViewWithReadMarker( hasUnreadAbove: Boolean, @@ -648,7 +651,7 @@ private fun TimelineViewWithReadMarker( @PreviewsDayNight @Composable -internal fun TimelineViewWithReadMarkerNoIndicatorsPreview() = ElementPreview { +internal fun TimelineViewWithReadMarkerJumpToUnreadIndicatorOnlyPreview() = ElementPreview { TimelineViewWithReadMarker(hasUnreadAbove = false, hasUnreadBelow = false) } diff --git a/tests/konsist/src/test/kotlin/io/element/android/tests/konsist/KonsistPreviewTest.kt b/tests/konsist/src/test/kotlin/io/element/android/tests/konsist/KonsistPreviewTest.kt index 6dc0e7fc565..0a8d5d6b893 100644 --- a/tests/konsist/src/test/kotlin/io/element/android/tests/konsist/KonsistPreviewTest.kt +++ b/tests/konsist/src/test/kotlin/io/element/android/tests/konsist/KonsistPreviewTest.kt @@ -161,7 +161,7 @@ class KonsistPreviewTest { "TimelineVideoWithCaptionRowPreview", "TimelineViewMessageShieldPreview", "TimelineViewWithReadMarkerBothIndicatorsPreview", - "TimelineViewWithReadMarkerNoIndicatorsPreview", + "TimelineViewWithReadMarkerJumpToUnreadIndicatorOnlyPreview", "UserAvatarColorsPreview", "UserProfileHeaderSectionWithVerificationViolationPreview", "VoiceItemViewPlayPreview", From bdb42759decb4abd89dccf947063cb53d2026227 Mon Sep 17 00:00:00 2001 From: Jenna Vassar <5023996+jennaharris7@users.noreply.github.com> Date: Thu, 21 May 2026 07:11:53 -0700 Subject: [PATCH 17/20] Show jump-to-unread when read marker is outside the loaded window When the m.fully_read marker event is older than the loaded timeline window, no virtual ReadMarker item is inserted and the chevron-up FAB silently no-ops. Surfaces the SDK's new fully_read_event_id on RoomInfo and routes the tap through the existing FocusOnEvent flow so the FAB appears in catch-up scenarios and lands the user on the marker. Bumps matrix-rust-components-kotlin to 26.05.20 to pick up the FFI field; absorbs the ClientBuildException.InvalidRawKey and suspend SpaceRoomList.rooms / subscribeToRoomUpdate API breaks. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../impl/timeline/TimelinePresenter.kt | 34 ++-- .../messages/impl/timeline/TimelineState.kt | 17 +- .../impl/timeline/TimelineStateProvider.kt | 4 +- .../messages/impl/timeline/TimelineView.kt | 36 +++- .../impl/timeline/TimelinePresenterTest.kt | 178 ++++++++++++++++-- .../space/impl/root/SpaceStateProvider.kt | 1 + gradle/libs.versions.toml | 2 +- .../libraries/matrix/api/room/RoomInfo.kt | 5 + .../impl/auth/AuthenticationException.kt | 1 + .../matrix/impl/room/RoomInfoMapper.kt | 1 + .../impl/fixtures/factories/RoomInfo.kt | 2 + .../fixtures/fakes/FakeFfiSpaceRoomList.kt | 6 +- .../matrix/impl/room/RoomInfoMapperTest.kt | 3 + .../matrix/test/room/RoomInfoFixture.kt | 2 + .../matrix/test/room/RoomSummaryFixture.kt | 1 + 15 files changed, 254 insertions(+), 39 deletions(-) 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 ac08c9adfd5..bfc951f6ee8 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 @@ -291,20 +291,32 @@ class TimelinePresenter( // without changing the list length, e.g. when [markRoomAsFullyRead] is sent while at the // bottom of the room. // - // Limitation: when the read marker is outside the loaded window (gaps, pagination), this - // returns null and the jump-to-unread button stays hidden. Proper fix needs an SDK - // accessor for the m.fully_read marker plus FocusedOnEvent navigation on click; gated - // behind FeatureFlags.JumpToUnread until that lands. - val readMarkerIndex = remember { mutableStateOf(null) } - LaunchedEffect(timelineItems, displayJumpToUnread) { + // The state has three shapes: + // - InWindow: the SDK has materialised a virtual ReadMarker item in the loaded window; + // tapping the FAB smoothly scrolls to its index. + // - OutOfWindow: the marker event is older than the loaded window, so the SDK gives us + // only the event id via RoomInfo.fullyReadEventId; tapping triggers a focused-event + // load via the existing TimelineEvent.FocusOnEvent path. + // - Hidden: feature flag off, no marker, caught-up (marker loaded but no virtual item), + // or initial load (no items yet). + val jumpToUnread = remember { mutableStateOf(JumpToUnreadState.Hidden) } + LaunchedEffect(timelineItems, displayJumpToUnread, roomInfo.fullyReadEventId) { if (!displayJumpToUnread) { - readMarkerIndex.value = null + jumpToUnread.value = JumpToUnreadState.Hidden return@LaunchedEffect } val items = timelineItems - readMarkerIndex.value = withContext(dispatchers.computation) { - items.indexOfFirst { (it as? TimelineItem.Virtual)?.model is TimelineItemReadMarkerModel } - .takeIf { it >= 0 } + val fullyReadEventId = roomInfo.fullyReadEventId + jumpToUnread.value = withContext(dispatchers.computation) { + val markerIndex = items.indexOfFirst { + (it as? TimelineItem.Virtual)?.model is TimelineItemReadMarkerModel + } + when { + markerIndex >= 0 -> JumpToUnreadState.InWindow(markerIndex) + fullyReadEventId != null && items.isNotEmpty() && !timelineItemIndexer.isKnown(fullyReadEventId) -> + JumpToUnreadState.OutOfWindow(fullyReadEventId) + else -> JumpToUnreadState.Hidden + } } } @@ -358,7 +370,7 @@ class TimelinePresenter( displayThreadSummaries = displayThreadSummaries, displayFloatingDateBadge = displayFloatingDateBadge, displayJumpToUnread = displayJumpToUnread, - readMarkerIndex = readMarkerIndex.value, + jumpToUnread = jumpToUnread.value, eventSink = ::handleEvent, ) } 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 d66dfe3031b..c5b563a9c34 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 @@ -36,7 +36,7 @@ data class TimelineState( val displayThreadSummaries: Boolean, val displayFloatingDateBadge: Boolean, val displayJumpToUnread: Boolean, - val readMarkerIndex: Int?, + val jumpToUnread: JumpToUnreadState, val eventSink: (TimelineEvent) -> Unit, ) { private val lastTimelineEvent = timelineItems.firstOrNull { it is TimelineItem.Event } as? TimelineItem.Event @@ -85,3 +85,18 @@ data class TimelineRoomInfo( val typingNotificationState: TypingNotificationState, val predecessorRoom: PredecessorRoom?, ) + +/** + * Whether the jump-to-unread FAB should be shown, and if so, how tapping it + * should bring the user to the read marker. + */ +@Immutable +sealed interface JumpToUnreadState { + data object Hidden : JumpToUnreadState + + /** The read marker is materialised at [index] in the loaded timeline — smooth scroll to it. */ + data class InWindow(val index: Int) : JumpToUnreadState + + /** The read marker event is older than the loaded window — load it via focused-event navigation. */ + data class OutOfWindow(val eventId: EventId) : JumpToUnreadState +} 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 2d4e22e79b6..81b31342680 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( displayThreadSummaries: Boolean = false, displayFloatingDateBadge: Boolean = false, displayJumpToUnread: Boolean = false, - readMarkerIndex: Int? = null, + jumpToUnread: JumpToUnreadState = JumpToUnreadState.Hidden, newEventState: NewEventState = NewEventState.None, eventSink: (TimelineEvent) -> Unit = {}, ): TimelineState { @@ -83,7 +83,7 @@ fun aTimelineState( displayThreadSummaries = displayThreadSummaries, displayFloatingDateBadge = displayFloatingDateBadge, displayJumpToUnread = displayJumpToUnread, - readMarkerIndex = readMarkerIndex, + jumpToUnread = jumpToUnread, 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 e9117d7ee7b..cca3c1f96c5 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 @@ -155,6 +155,10 @@ fun TimelineView( state.eventSink(TimelineEvent.MarkAllAsRead) } + fun onFocusOnEvent(eventId: EventId) { + state.eventSink(TimelineEvent.FocusOnEvent(eventId)) + } + val context = LocalContext.current val toastMessage = stringResource(CommonStrings.common_copied_to_clipboard) val view = LocalView.current @@ -240,11 +244,12 @@ fun TimelineView( isLive = state.isLive, focusRequestState = state.focusRequestState, displayJumpToUnread = state.displayJumpToUnread, - readMarkerIndex = state.readMarkerIndex, + jumpToUnread = state.jumpToUnread, onScrollFinishAt = ::onScrollFinishAt, onJumpToLive = ::onJumpToLive, onFocusEventRender = ::onFocusEventRender, onMarkAllAsRead = ::onMarkAllAsRead, + onFocusOnEvent = ::onFocusOnEvent, ) if (state.displayFloatingDateBadge && useReverseLayout) { @@ -326,11 +331,12 @@ private fun BoxScope.TimelineScrollHelper( forceJumpToReadMarkerVisibility: Boolean, focusRequestState: FocusRequestState, displayJumpToUnread: Boolean, - readMarkerIndex: Int?, + jumpToUnread: JumpToUnreadState, onScrollFinishAt: (Int) -> Unit, onJumpToLive: () -> Unit, onFocusEventRender: () -> Unit, onMarkAllAsRead: () -> Unit, + onFocusOnEvent: (EventId) -> Unit, ) { val coroutineScope = rememberCoroutineScope() val isScrollFinished by remember { derivedStateOf { !lazyListState.isScrollInProgress } } @@ -339,12 +345,19 @@ private fun BoxScope.TimelineScrollHelper( lazyListState.firstVisibleItemIndex < 3 && isLive } } - val isJumpToUnreadVisible by remember(readMarkerIndex, forceJumpToReadMarkerVisibility) { + val isJumpToUnreadVisible by remember(jumpToUnread, forceJumpToReadMarkerVisibility) { derivedStateOf { if (forceJumpToReadMarkerVisibility) return@derivedStateOf true - val markerIndex = readMarkerIndex ?: return@derivedStateOf false - val lastVisibleIndex = lazyListState.layoutInfo.visibleItemsInfo.lastOrNull()?.index ?: return@derivedStateOf false - markerIndex > lastVisibleIndex + when (val jtu = jumpToUnread) { + JumpToUnreadState.Hidden -> false + // Marker is outside the loaded window — we have no on-screen anchor, so just show. + is JumpToUnreadState.OutOfWindow -> true + // Marker is in the loaded window — hide once it's scrolled into the visible range. + is JumpToUnreadState.InWindow -> { + val lastVisibleIndex = lazyListState.layoutInfo.visibleItemsInfo.lastOrNull()?.index ?: return@derivedStateOf false + jtu.index > lastVisibleIndex + } + } } } val isJumpToBottomVisible = !canAutoScroll || forceJumpToBottomVisibility || !isLive @@ -375,9 +388,12 @@ private fun BoxScope.TimelineScrollHelper( } fun jumpToReadMarker() { - val markerIndex = readMarkerIndex ?: return - coroutineScope.launch { - lazyListState.animateScrollToItemCenter(markerIndex) + when (val jtu = jumpToUnread) { + JumpToUnreadState.Hidden -> Unit + is JumpToUnreadState.InWindow -> coroutineScope.launch { + lazyListState.animateScrollToItemCenter(jtu.index) + } + is JumpToUnreadState.OutOfWindow -> onFocusOnEvent(jtu.eventId) } } @@ -630,7 +646,7 @@ private fun TimelineViewWithReadMarker( // 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. - readMarkerIndex = if (hasUnreadAbove) timelineItems.size else null, + jumpToUnread = if (hasUnreadAbove) JumpToUnreadState.InWindow(timelineItems.size) else JumpToUnreadState.Hidden, newEventState = if (hasUnreadBelow) NewEventState.FromOther else NewEventState.None, ), timelineProtectionState = aTimelineProtectionState(), 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 f5ede957c8c..7cd0f1f1266 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 @@ -56,6 +56,7 @@ import io.element.android.libraries.matrix.test.A_UNIQUE_ID_2 import io.element.android.libraries.matrix.test.A_USER_ID import io.element.android.libraries.matrix.test.room.FakeBaseRoom import io.element.android.libraries.matrix.test.room.FakeJoinedRoom +import io.element.android.libraries.matrix.test.room.aRoomInfo import io.element.android.libraries.matrix.test.room.aRoomMember import io.element.android.libraries.matrix.test.room.powerlevels.FakeRoomPermissions import io.element.android.libraries.matrix.test.timeline.FakeTimeline @@ -371,7 +372,7 @@ class TimelinePresenterTest { } @Test - fun `present - readMarkerIndex points at the read marker virtual item`() = runTest { + fun `present - jumpToUnread is InWindow at the read marker virtual item index`() = runTest { val timelineItems = MutableStateFlow(emptyList()) val timeline = FakeTimeline(timelineItems = timelineItems) val presenter = createTimelinePresenter( @@ -391,8 +392,8 @@ class TimelinePresenterTest { MatrixTimelineItem.Event(UniqueId("msg-newest"), anEventTimelineItem(content = aMessageContent())), ) ) - consumeItemsUntilPredicate { it.readMarkerIndex != null }.last().also { state -> - assertThat(state.readMarkerIndex).isEqualTo(3) + consumeItemsUntilPredicate { it.jumpToUnread is JumpToUnreadState.InWindow }.last().also { state -> + assertThat(state.jumpToUnread).isEqualTo(JumpToUnreadState.InWindow(index = 3)) assertThat(state.displayJumpToUnread).isTrue() } cancelAndIgnoreRemainingEvents() @@ -400,7 +401,7 @@ class TimelinePresenterTest { } @Test - fun `present - readMarkerIndex is null when no read marker present`() = runTest { + fun `present - jumpToUnread is Hidden when no read marker and no fullyReadEventId`() = runTest { val timelineItems = MutableStateFlow(emptyList()) val timeline = FakeTimeline(timelineItems = timelineItems) val presenter = createTimelinePresenter( @@ -416,16 +417,16 @@ class TimelinePresenterTest { ) ) consumeItemsUntilPredicate { - it.timelineItems.size == 2 && it.readMarkerIndex == null + it.timelineItems.size == 2 && it.jumpToUnread == JumpToUnreadState.Hidden }.last().also { state -> - assertThat(state.readMarkerIndex).isNull() + assertThat(state.jumpToUnread).isEqualTo(JumpToUnreadState.Hidden) } cancelAndIgnoreRemainingEvents() } } @Test - fun `present - readMarkerIndex stays null when JumpToUnread feature flag is disabled`() = runTest { + fun `present - jumpToUnread is Hidden when JumpToUnread feature flag is disabled`() = runTest { val timelineItems = MutableStateFlow(emptyList()) val timeline = FakeTimeline(timelineItems = timelineItems) val presenter = createTimelinePresenter( @@ -443,7 +444,7 @@ class TimelinePresenterTest { ) consumeItemsUntilPredicate { it.timelineItems.size == 3 }.last().also { state -> assertThat(state.displayJumpToUnread).isFalse() - assertThat(state.readMarkerIndex).isNull() + assertThat(state.jumpToUnread).isEqualTo(JumpToUnreadState.Hidden) } cancelAndIgnoreRemainingEvents() } @@ -579,7 +580,7 @@ class TimelinePresenterTest { } @Test - fun `present - readMarkerIndex is 0 when the read marker is the only item`() = runTest { + fun `present - jumpToUnread is InWindow at index 0 when the read marker is the only item`() = runTest { val timelineItems = MutableStateFlow(emptyList()) val timeline = FakeTimeline(timelineItems = timelineItems) val presenter = createTimelinePresenter( @@ -591,8 +592,163 @@ class TimelinePresenterTest { timelineItems.emit( listOf(MatrixTimelineItem.Virtual(UniqueId("read-marker"), VirtualTimelineItem.ReadMarker)) ) - consumeItemsUntilPredicate { it.readMarkerIndex != null }.last().also { state -> - assertThat(state.readMarkerIndex).isEqualTo(0) + consumeItemsUntilPredicate { it.jumpToUnread is JumpToUnreadState.InWindow }.last().also { state -> + assertThat(state.jumpToUnread).isEqualTo(JumpToUnreadState.InWindow(index = 0)) + } + cancelAndIgnoreRemainingEvents() + } + } + + @Test + fun `present - jumpToUnread is OutOfWindow when fullyReadEventId is set but not in the loaded window`() = runTest { + val timelineItems = MutableStateFlow(emptyList()) + val timeline = FakeTimeline(timelineItems = timelineItems) + val fullyReadEventId = EventId("\$older-than-loaded-window") + val room = FakeJoinedRoom( + liveTimeline = timeline, + baseRoom = FakeBaseRoom( + roomPermissions = roomPermissions(), + initialRoomInfo = aRoomInfo(fullyReadEventId = fullyReadEventId), + ), + ) + val presenter = createTimelinePresenter( + timeline = timeline, + room = room, + featureFlagService = FakeFeatureFlagService(initialState = mapOf(FeatureFlags.JumpToUnread.key to true)), + ) + presenter.test { + awaitFirstItem() + // Loaded items don't include the fullyReadEventId and the SDK didn't materialise a ReadMarker. + timelineItems.emit( + listOf( + MatrixTimelineItem.Event(UniqueId("1"), anEventTimelineItem(eventId = AN_EVENT_ID, content = aMessageContent())), + MatrixTimelineItem.Event(UniqueId("2"), anEventTimelineItem(eventId = AN_EVENT_ID_2, content = aMessageContent())), + ) + ) + consumeItemsUntilPredicate { it.jumpToUnread is JumpToUnreadState.OutOfWindow }.last().also { state -> + assertThat(state.jumpToUnread).isEqualTo(JumpToUnreadState.OutOfWindow(eventId = fullyReadEventId)) + } + cancelAndIgnoreRemainingEvents() + } + } + + @Test + fun `present - jumpToUnread is Hidden when fullyReadEventId IS in the loaded window`() = runTest { + val timelineItems = MutableStateFlow(emptyList()) + val timeline = FakeTimeline(timelineItems = timelineItems) + val room = FakeJoinedRoom( + liveTimeline = timeline, + baseRoom = FakeBaseRoom( + roomPermissions = roomPermissions(), + // The user is caught up: the marker event is loaded, but the SDK didn't insert a + // virtual ReadMarker because there are no items newer than it. + initialRoomInfo = aRoomInfo(fullyReadEventId = AN_EVENT_ID), + ), + ) + val presenter = createTimelinePresenter( + timeline = timeline, + room = room, + featureFlagService = FakeFeatureFlagService(initialState = mapOf(FeatureFlags.JumpToUnread.key to true)), + ) + presenter.test { + awaitFirstItem() + // A loaded item has eventId == AN_EVENT_ID (default of anEventTimelineItem). + timelineItems.emit( + listOf( + MatrixTimelineItem.Event(UniqueId("1"), anEventTimelineItem(content = aMessageContent())), + ) + ) + consumeItemsUntilPredicate { it.timelineItems.size == 1 }.last().also { state -> + assertThat(state.jumpToUnread).isEqualTo(JumpToUnreadState.Hidden) + } + cancelAndIgnoreRemainingEvents() + } + } + + @Test + fun `present - jumpToUnread is Hidden when fullyReadEventId is set but the timeline is empty`() = runTest { + val timelineItems = MutableStateFlow(emptyList()) + val timeline = FakeTimeline(timelineItems = timelineItems) + val room = FakeJoinedRoom( + liveTimeline = timeline, + baseRoom = FakeBaseRoom( + roomPermissions = roomPermissions(), + initialRoomInfo = aRoomInfo(fullyReadEventId = AN_EVENT_ID), + ), + ) + val presenter = createTimelinePresenter( + timeline = timeline, + room = room, + featureFlagService = FakeFeatureFlagService(initialState = mapOf(FeatureFlags.JumpToUnread.key to true)), + ) + presenter.test { + val initialState = awaitFirstItem() + // Without any timeline items, the FAB must stay hidden — the user is mid-load. + assertThat(initialState.jumpToUnread).isEqualTo(JumpToUnreadState.Hidden) + advanceUntilIdle() + val drained = consumeItemsUntilTimeout() + assertThat(drained.any { it.jumpToUnread != JumpToUnreadState.Hidden }).isFalse() + cancelAndIgnoreRemainingEvents() + } + } + + @Test + fun `present - jumpToUnread is Hidden when fullyReadEventId is set but the feature flag is off`() = runTest { + val timelineItems = MutableStateFlow(emptyList()) + val timeline = FakeTimeline(timelineItems = timelineItems) + val room = FakeJoinedRoom( + liveTimeline = timeline, + baseRoom = FakeBaseRoom( + roomPermissions = roomPermissions(), + initialRoomInfo = aRoomInfo(fullyReadEventId = AN_EVENT_ID), + ), + ) + val presenter = createTimelinePresenter( + timeline = timeline, + room = room, + featureFlagService = FakeFeatureFlagService(initialState = mapOf(FeatureFlags.JumpToUnread.key to false)), + ) + presenter.test { + awaitFirstItem() + timelineItems.emit( + listOf( + MatrixTimelineItem.Event(UniqueId("1"), anEventTimelineItem(content = aMessageContent())), + ) + ) + consumeItemsUntilPredicate { it.timelineItems.size == 1 }.last().also { state -> + assertThat(state.jumpToUnread).isEqualTo(JumpToUnreadState.Hidden) + } + cancelAndIgnoreRemainingEvents() + } + } + + @Test + fun `present - jumpToUnread prefers InWindow when both a virtual marker and fullyReadEventId are present`() = runTest { + val timelineItems = MutableStateFlow(emptyList()) + val timeline = FakeTimeline(timelineItems = timelineItems) + val room = FakeJoinedRoom( + liveTimeline = timeline, + baseRoom = FakeBaseRoom( + roomPermissions = roomPermissions(), + initialRoomInfo = aRoomInfo(fullyReadEventId = AN_EVENT_ID), + ), + ) + val presenter = createTimelinePresenter( + timeline = timeline, + room = room, + featureFlagService = FakeFeatureFlagService(initialState = mapOf(FeatureFlags.JumpToUnread.key to true)), + ) + 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.jumpToUnread is JumpToUnreadState.InWindow }.last().also { state -> + assertThat(state.jumpToUnread).isInstanceOf(JumpToUnreadState.InWindow::class.java) } cancelAndIgnoreRemainingEvents() } diff --git a/features/space/impl/src/main/kotlin/io/element/android/features/space/impl/root/SpaceStateProvider.kt b/features/space/impl/src/main/kotlin/io/element/android/features/space/impl/root/SpaceStateProvider.kt index 20f4918a98f..6b8d714cb74 100644 --- a/features/space/impl/src/main/kotlin/io/element/android/features/space/impl/root/SpaceStateProvider.kt +++ b/features/space/impl/src/main/kotlin/io/element/android/features/space/impl/root/SpaceStateProvider.kt @@ -131,6 +131,7 @@ private fun aSpaceInfo( numUnreadMessages = 0, numUnreadNotifications = 0, numUnreadMentions = 0, + fullyReadEventId = null, heroes = persistentListOf(), pinnedEventIds = persistentListOf(), creators = persistentListOf(), diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 7a793205308..cf95945d051 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -178,7 +178,7 @@ test_detekt_test = { module = "io.gitlab.arturbosch.detekt:detekt-test", version # https://github.com/matrix-org/matrix-rust-components-kotlin/commits/main/sdk/sdk-android/src/main/kotlin/org/matrix/rustcomponents/sdk/matrix_sdk_ffi.kt # All new features should not be implemented in the pull request that upgrades the version, developers should # only fix API breaks and may add some TODOs. -matrix_sdk = "org.matrix.rustcomponents:sdk-android:26.05.7" +matrix_sdk = "org.matrix.rustcomponents:sdk-android:26.05.20" # Others coil = { module = "io.coil-kt.coil3:coil", version.ref = "coil" } diff --git a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/RoomInfo.kt b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/RoomInfo.kt index b9ed8d61b13..715c7348a19 100644 --- a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/RoomInfo.kt +++ b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/RoomInfo.kt @@ -70,6 +70,11 @@ data class RoomInfo( * notification settings. */ val numUnreadMentions: Long, + /** + * Event ID of the user's `m.fully_read` marker for this room, if any. + * Can be set even when the event is older than the loaded timeline window. + */ + val fullyReadEventId: EventId?, val heroes: ImmutableList, val pinnedEventIds: ImmutableList, val creators: ImmutableList, diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/auth/AuthenticationException.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/auth/AuthenticationException.kt index 20dbf76a319..fac5227f6ac 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/auth/AuthenticationException.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/auth/AuthenticationException.kt @@ -28,6 +28,7 @@ fun Throwable.mapAuthenticationException(): AuthenticationException { } is ClientBuildException.WellKnownLookupFailed -> AuthenticationException.Generic(message) is ClientBuildException.EventCache -> AuthenticationException.Generic(message) + is ClientBuildException.InvalidRawKey -> AuthenticationException.Generic(message) } is OAuthException -> when (this) { is OAuthException.Generic -> AuthenticationException.OAuth(message) diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RoomInfoMapper.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RoomInfoMapper.kt index 0e9aadc65bd..a1d0dd5cd30 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RoomInfoMapper.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RoomInfoMapper.kt @@ -71,6 +71,7 @@ class RoomInfoMapper { numUnreadMessages = it.numUnreadMessages.toLong(), numUnreadMentions = it.numUnreadMentions.toLong(), numUnreadNotifications = it.numUnreadNotifications.toLong(), + fullyReadEventId = it.fullyReadEventId?.let(::EventId), historyVisibility = it.historyVisibility.map(), successorRoom = it.successorRoom?.map(), roomVersion = it.roomVersion, diff --git a/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/fixtures/factories/RoomInfo.kt b/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/fixtures/factories/RoomInfo.kt index 2ce64154f72..a809f168035 100644 --- a/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/fixtures/factories/RoomInfo.kt +++ b/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/fixtures/factories/RoomInfo.kt @@ -52,6 +52,7 @@ internal fun aRustRoomInfo( numUnreadMessages: ULong = 0uL, numUnreadNotifications: ULong = 0uL, numUnreadMentions: ULong = 0uL, + fullyReadEventId: String? = null, pinnedEventIds: List = listOf(), roomCreators: List? = emptyList(), joinRule: JoinRule? = null, @@ -93,6 +94,7 @@ internal fun aRustRoomInfo( numUnreadMessages = numUnreadMessages, numUnreadNotifications = numUnreadNotifications, numUnreadMentions = numUnreadMentions, + fullyReadEventId = fullyReadEventId, pinnedEventIds = pinnedEventIds, creators = roomCreators, joinRule = joinRule, diff --git a/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/fixtures/fakes/FakeFfiSpaceRoomList.kt b/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/fixtures/fakes/FakeFfiSpaceRoomList.kt index c0ecc53d4f3..c85fb287b76 100644 --- a/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/fixtures/fakes/FakeFfiSpaceRoomList.kt +++ b/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/fixtures/fakes/FakeFfiSpaceRoomList.kt @@ -40,8 +40,8 @@ class FakeFfiSpaceRoomList( return paginationStateResult() } - override fun rooms(): List { - return roomsResult() + override suspend fun rooms(): List = simulateLongTask { + roomsResult() } override fun subscribeToPaginationStateUpdates(listener: SpaceRoomListPaginationStateListener): TaskHandle { @@ -53,7 +53,7 @@ class FakeFfiSpaceRoomList( spaceRoomListPaginationStateListener?.onUpdate(state) } - override fun subscribeToRoomUpdate(listener: SpaceRoomListEntriesListener): TaskHandle { + override suspend fun subscribeToRoomUpdate(listener: SpaceRoomListEntriesListener): TaskHandle { spaceRoomListEntriesListener = listener return FakeFfiTaskHandle() } diff --git a/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/room/RoomInfoMapperTest.kt b/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/room/RoomInfoMapperTest.kt index 56b480d97fc..75386964ca4 100644 --- a/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/room/RoomInfoMapperTest.kt +++ b/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/room/RoomInfoMapperTest.kt @@ -79,6 +79,7 @@ class RoomInfoMapperTest { numUnreadMessages = 12uL, numUnreadNotifications = 13uL, numUnreadMentions = 14uL, + fullyReadEventId = AN_EVENT_ID.value, pinnedEventIds = listOf(AN_EVENT_ID.value), roomCreators = listOf(A_USER_ID.value), historyVisibility = RustRoomHistoryVisibility.Joined, @@ -131,6 +132,7 @@ class RoomInfoMapperTest { numUnreadMessages = 12L, numUnreadNotifications = 13L, numUnreadMentions = 14L, + fullyReadEventId = AN_EVENT_ID, historyVisibility = RoomHistoryVisibility.Joined, successorRoom = null, roomVersion = "12", @@ -223,6 +225,7 @@ class RoomInfoMapperTest { numUnreadMessages = 12L, numUnreadNotifications = 13L, numUnreadMentions = 14L, + fullyReadEventId = null, historyVisibility = RoomHistoryVisibility.Joined, roomVersion = "12", privilegedCreatorRole = true, diff --git a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/room/RoomInfoFixture.kt b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/room/RoomInfoFixture.kt index c15330e9dc8..7fba39c9095 100644 --- a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/room/RoomInfoFixture.kt +++ b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/room/RoomInfoFixture.kt @@ -66,6 +66,7 @@ fun aRoomInfo( numUnreadMessages: Long = 0, numUnreadNotifications: Long = 0, numUnreadMentions: Long = 0, + fullyReadEventId: EventId? = null, historyVisibility: RoomHistoryVisibility = RoomHistoryVisibility.Joined, roomVersion: String? = "11", privilegedCreatorRole: Boolean = false, @@ -105,6 +106,7 @@ fun aRoomInfo( numUnreadMessages = numUnreadMessages, numUnreadNotifications = numUnreadNotifications, numUnreadMentions = numUnreadMentions, + fullyReadEventId = fullyReadEventId, historyVisibility = historyVisibility, roomVersion = roomVersion, privilegedCreatorRole = privilegedCreatorRole, diff --git a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/room/RoomSummaryFixture.kt b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/room/RoomSummaryFixture.kt index 32635a7eea4..57f89101b7f 100644 --- a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/room/RoomSummaryFixture.kt +++ b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/room/RoomSummaryFixture.kt @@ -115,6 +115,7 @@ fun aRoomSummary( numUnreadMessages = numUnreadMessages, numUnreadNotifications = numUnreadNotifications, numUnreadMentions = numUnreadMentions, + fullyReadEventId = null, historyVisibility = historyVisibility, roomVersion = roomVersion, privilegedCreatorRole = privilegedCreatorRole, From 0399dab1da02b729fe0a5a2130b11b99fa95baba Mon Sep 17 00:00:00 2001 From: Jenna Vassar <5023996+jennaharris7@users.noreply.github.com> Date: Thu, 21 May 2026 08:48:38 -0700 Subject: [PATCH 18/20] Dismiss mark as unread button eagerly --- .../impl/timeline/TimelinePresenter.kt | 16 +++++++- .../impl/timeline/TimelinePresenterTest.kt | 38 +++++++++++++++++++ 2 files changed, 52 insertions(+), 2 deletions(-) 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 bfc951f6ee8..30117b90fde 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 @@ -138,6 +138,12 @@ class TimelinePresenter( val newEventState = remember { mutableStateOf(NewEventState.None) } val messageShieldDialogData: MutableState = remember { mutableStateOf(null) } + // Forces [JumpToUnreadState.Hidden] until the next RoomInfo push. Set after a + // [TimelineEvent.MarkAllAsRead] await completes so the FAB hides without waiting for + // the SDK to push a refreshed fully-read marker; the after-await ordering means any + // RoomInfo update racing the mark-as-read call has already landed and can't undo this. + val suppressJumpToUnread = remember { mutableStateOf(false) } + val resolveVerifiedUserSendFailureState = resolveVerifiedUserSendFailurePresenter.present() val isSendPublicReadReceiptsEnabled by remember { sessionPreferencesStore.isSendPublicReadReceiptsEnabled() @@ -234,6 +240,7 @@ class TimelinePresenter( null } ?: return@launch markAsFullyRead(room.roomId, latestEventId) + suppressJumpToUnread.value = true } is TimelineEvent.ShowShieldDialog -> messageShieldDialogData.value = event.messageShieldData is TimelineEvent.ComputeVerifiedUserSendFailure -> { @@ -300,8 +307,13 @@ class TimelinePresenter( // - Hidden: feature flag off, no marker, caught-up (marker loaded but no virtual item), // or initial load (no items yet). val jumpToUnread = remember { mutableStateOf(JumpToUnreadState.Hidden) } - LaunchedEffect(timelineItems, displayJumpToUnread, roomInfo.fullyReadEventId) { - if (!displayJumpToUnread) { + // The SDK is authoritative again once it pushes a new fully-read marker, so drop the + // post-mark-as-read suppression and let the recompute below pick up the new value. + LaunchedEffect(roomInfo.fullyReadEventId) { + suppressJumpToUnread.value = false + } + LaunchedEffect(timelineItems, displayJumpToUnread, roomInfo.fullyReadEventId, suppressJumpToUnread.value) { + if (!displayJumpToUnread || suppressJumpToUnread.value) { jumpToUnread.value = JumpToUnreadState.Hidden return@LaunchedEffect } 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 7cd0f1f1266..bd40fccf71c 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 @@ -754,6 +754,44 @@ class TimelinePresenterTest { } } + @Test + fun `present - jumpToUnread hides eagerly after MarkAllAsRead even before a new RoomInfo arrives`() = runTest { + val timelineItems = MutableStateFlow(emptyList()) + val timeline = FakeTimeline( + timelineItems = timelineItems, + getLatestEventIdResult = { Result.success(AN_EVENT_ID_2) }, + ) + val fullyReadEventId = EventId("\$older-than-loaded-window") + val room = FakeJoinedRoom( + liveTimeline = timeline, + baseRoom = FakeBaseRoom( + roomPermissions = roomPermissions(), + initialRoomInfo = aRoomInfo(fullyReadEventId = fullyReadEventId), + ), + ) + val presenter = createTimelinePresenter( + timeline = timeline, + room = room, + featureFlagService = FakeFeatureFlagService(initialState = mapOf(FeatureFlags.JumpToUnread.key to true)), + ) + presenter.test { + awaitFirstItem() + timelineItems.emit( + listOf( + MatrixTimelineItem.Event(UniqueId("1"), anEventTimelineItem(eventId = AN_EVENT_ID, content = aMessageContent())), + ) + ) + val outOfWindow = consumeItemsUntilPredicate { it.jumpToUnread is JumpToUnreadState.OutOfWindow }.last() + assertThat(outOfWindow.jumpToUnread).isEqualTo(JumpToUnreadState.OutOfWindow(eventId = fullyReadEventId)) + + outOfWindow.eventSink(TimelineEvent.MarkAllAsRead) + // RoomInfo is intentionally NOT updated — eager hide must fire on the await alone. + val afterMark = consumeItemsUntilPredicate { it.jumpToUnread == JumpToUnreadState.Hidden }.last() + assertThat(afterMark.jumpToUnread).isEqualTo(JumpToUnreadState.Hidden) + cancelAndIgnoreRemainingEvents() + } + } + @Test fun `present - reaction ordering`() = runTest { val timelineItems = MutableStateFlow(emptyList()) From 40f8b48256beec447d66d836c68c27beb7d71c3e Mon Sep 17 00:00:00 2001 From: Jenna Vassar <5023996+jennaharris7@users.noreply.github.com> Date: Thu, 28 May 2026 09:06:01 -0700 Subject: [PATCH 19/20] Reuse UnreadAtom composable --- .../messages/impl/timeline/TimelineView.kt | 31 +++++++------------ .../atomic/atoms/UnreadIndicatorAtom.kt | 4 +++ 2 files changed, 15 insertions(+), 20 deletions(-) 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 cca3c1f96c5..aaf06bfd87a 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 @@ -16,6 +16,7 @@ import androidx.compose.animation.fadeIn import androidx.compose.animation.fadeOut import androidx.compose.animation.scaleIn import androidx.compose.animation.scaleOut +import androidx.compose.foundation.BorderStroke import androidx.compose.foundation.background import androidx.compose.foundation.border import androidx.compose.foundation.clickable @@ -87,6 +88,7 @@ import io.element.android.features.messages.impl.timeline.model.event.TimelineIt import io.element.android.features.messages.impl.timeline.protection.TimelineProtectionState import io.element.android.features.messages.impl.timeline.protection.aTimelineProtectionState import io.element.android.libraries.androidutils.system.copyToClipboard +import io.element.android.libraries.designsystem.atomic.atoms.UnreadIndicatorAtom import io.element.android.libraries.designsystem.components.dialogs.AlertDialog import io.element.android.libraries.designsystem.preview.ElementPreview import io.element.android.libraries.designsystem.preview.PreviewsDayNight @@ -561,30 +563,19 @@ private fun JumpToPositionButton( } } val dotYOffset = if (dotAlignment == Alignment.BottomCenter) 4.dp else (-4).dp - TimelineUnreadIndicator( - isVisible = hasUnread, - modifier = Modifier - .align(dotAlignment) - .offset { IntOffset(x = 0, y = dotYOffset.roundToPx()) }, - ) + if (hasUnread) { + UnreadIndicatorAtom( + modifier = Modifier + .align(dotAlignment) + .offset { IntOffset(x = 0, y = dotYOffset.roundToPx()) }, + color = ElementTheme.colors.iconSuccessPrimary, + border = BorderStroke(2.dp, ElementTheme.colors.bgCanvasDefault), + ) + } } } } -@Composable -private fun TimelineUnreadIndicator( - isVisible: Boolean, - modifier: Modifier = Modifier, -) { - 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 @Composable internal fun TimelineViewPreview( diff --git a/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/atomic/atoms/UnreadIndicatorAtom.kt b/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/atomic/atoms/UnreadIndicatorAtom.kt index d2db3aec8e3..f162677e492 100644 --- a/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/atomic/atoms/UnreadIndicatorAtom.kt +++ b/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/atomic/atoms/UnreadIndicatorAtom.kt @@ -8,7 +8,9 @@ package io.element.android.libraries.designsystem.atomic.atoms +import androidx.compose.foundation.BorderStroke import androidx.compose.foundation.background +import androidx.compose.foundation.border import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.size import androidx.compose.foundation.shape.CircleShape @@ -32,6 +34,7 @@ fun UnreadIndicatorAtom( color: Color = ElementTheme.colors.unreadIndicator, isVisible: Boolean = true, contentDescription: String? = null, + border: BorderStroke? = null, ) { Box( modifier = modifier @@ -41,6 +44,7 @@ fun UnreadIndicatorAtom( .size(size) .clip(CircleShape) .background(if (isVisible) color else Color.Transparent) + .then(if (border != null) Modifier.border(border, CircleShape) else Modifier) ) } From 2c2d803e5828ef3f3e116296acc560d439a8300c Mon Sep 17 00:00:00 2001 From: Jenna Vassar <5023996+jennaharris7@users.noreply.github.com> Date: Thu, 28 May 2026 09:06:31 -0700 Subject: [PATCH 20/20] Update a11y string --- libraries/ui-strings/src/main/res/values/temporary.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/ui-strings/src/main/res/values/temporary.xml b/libraries/ui-strings/src/main/res/values/temporary.xml index 29dadb0418e..fd1edb07595 100644 --- a/libraries/ui-strings/src/main/res/values/temporary.xml +++ b/libraries/ui-strings/src/main/res/values/temporary.xml @@ -7,5 +7,5 @@ "Mark as read" - "Jump to unread messages" + "Jump to first unread message"