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 dbb23b774e..6e0145f742 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 12dd01d57a..6f48a8d9fc 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 7fc3386ca7..0ea48c3c35 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 9a324afd2f..b93f9e950d 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 0000000000..8154445015 --- /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 502308cfe7..bec7895c63 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() }