mirror of
https://github.com/Novaloop-AG/element-x-android.git
synced 2026-11-03 16:10:00 +00:00
Gate out-of-window jump-to-unread on genuinely unread displayable content
The jump-to-unread FAB could surface an OutOfWindow target when the
fully-read marker event was either loaded-but-not-displayed (state/filtered
events) or when there was nothing displayable to jump to. Add two guards to
the recompute:
- Require roomInfo.numUnreadMessages > 0, so the FAB only appears when there
is genuinely unread "interesting" content (never state/hidden events).
- Add Timeline.isEventLoaded to distinguish "in the loaded window but not
rendered" from "genuinely outside the window", falling back to the SDK only
after the cheap display-index check (isKnown) misses.
The RustTimeline implementation reads the replay cache first and releases the
EventTimelineItem native handle via use {} when probing the SDK. Adds presenter
tests for the loaded-but-not-displayed and no-unread-messages branches.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
+25
-9
@@ -312,23 +312,39 @@ class TimelinePresenter(
|
|||||||
LaunchedEffect(roomInfo.fullyReadEventId) {
|
LaunchedEffect(roomInfo.fullyReadEventId) {
|
||||||
suppressJumpToUnread.value = false
|
suppressJumpToUnread.value = false
|
||||||
}
|
}
|
||||||
LaunchedEffect(timelineItems.map { it.identifier() }, displayJumpToUnread, roomInfo.fullyReadEventId, suppressJumpToUnread.value) {
|
LaunchedEffect(
|
||||||
|
timelineItems.map { it.identifier() },
|
||||||
|
displayJumpToUnread,
|
||||||
|
roomInfo.fullyReadEventId,
|
||||||
|
roomInfo.numUnreadMessages,
|
||||||
|
suppressJumpToUnread.value,
|
||||||
|
) {
|
||||||
if (!displayJumpToUnread || suppressJumpToUnread.value) {
|
if (!displayJumpToUnread || suppressJumpToUnread.value) {
|
||||||
jumpToUnread.value = JumpToUnreadState.Hidden
|
jumpToUnread.value = JumpToUnreadState.Hidden
|
||||||
return@LaunchedEffect
|
return@LaunchedEffect
|
||||||
}
|
}
|
||||||
val items = timelineItems
|
val items = timelineItems
|
||||||
val fullyReadEventId = roomInfo.fullyReadEventId
|
val fullyReadEventId = roomInfo.fullyReadEventId
|
||||||
jumpToUnread.value = withContext(dispatchers.computation) {
|
val hasUnreadMessages = roomInfo.numUnreadMessages > 0
|
||||||
val markerIndex = items.indexOfFirst {
|
val markerIndex = withContext(dispatchers.computation) {
|
||||||
|
items.indexOfFirst {
|
||||||
(it as? TimelineItem.Virtual)?.model is TimelineItemReadMarkerModel
|
(it as? TimelineItem.Virtual)?.model is TimelineItemReadMarkerModel
|
||||||
}
|
}
|
||||||
when {
|
}
|
||||||
markerIndex >= 0 -> JumpToUnreadState.InWindow(markerIndex)
|
jumpToUnread.value = when {
|
||||||
fullyReadEventId != null && items.isNotEmpty() && !timelineItemIndexer.isKnown(fullyReadEventId) ->
|
markerIndex >= 0 -> JumpToUnreadState.InWindow(markerIndex)
|
||||||
JumpToUnreadState.OutOfWindow(fullyReadEventId)
|
// Out-of-window only when there is genuinely unread *displayable* content
|
||||||
else -> JumpToUnreadState.Hidden
|
// (numUnreadMessages counts "interesting" messages, never state/hidden events) AND
|
||||||
}
|
// the marker event isn't merely an in-window item we don't render. isKnown is the
|
||||||
|
// cheap display-index check; isEventLoaded falls back to the SDK to tell
|
||||||
|
// "in window but not displayed" apart from "genuinely out of window".
|
||||||
|
fullyReadEventId != null &&
|
||||||
|
hasUnreadMessages &&
|
||||||
|
items.isNotEmpty() &&
|
||||||
|
!timelineItemIndexer.isKnown(fullyReadEventId) &&
|
||||||
|
!timelineController.activeTimelineFlow().value.isEventLoaded(fullyReadEventId) ->
|
||||||
|
JumpToUnreadState.OutOfWindow(fullyReadEventId)
|
||||||
|
else -> JumpToUnreadState.Hidden
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
+75
-2
@@ -608,7 +608,9 @@ class TimelinePresenterTest {
|
|||||||
liveTimeline = timeline,
|
liveTimeline = timeline,
|
||||||
baseRoom = FakeBaseRoom(
|
baseRoom = FakeBaseRoom(
|
||||||
roomPermissions = roomPermissions(),
|
roomPermissions = roomPermissions(),
|
||||||
initialRoomInfo = aRoomInfo(fullyReadEventId = fullyReadEventId),
|
// There is genuinely unread *displayable* content, and the marker event isn't loaded
|
||||||
|
// (isEventLoaded defaults to false), so the FAB targets the out-of-window marker.
|
||||||
|
initialRoomInfo = aRoomInfo(fullyReadEventId = fullyReadEventId, numUnreadMessages = 1),
|
||||||
),
|
),
|
||||||
)
|
)
|
||||||
val presenter = createTimelinePresenter(
|
val presenter = createTimelinePresenter(
|
||||||
@@ -632,6 +634,77 @@ class TimelinePresenterTest {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `present - jumpToUnread is Hidden when the marker event is loaded in the window but not displayed`() = runTest {
|
||||||
|
val timelineItems = MutableStateFlow(emptyList<MatrixTimelineItem>())
|
||||||
|
// The marker event is in the loaded window (e.g. a state/filtered event) but never rendered.
|
||||||
|
val fullyReadEventId = EventId("\$loaded-but-not-displayed")
|
||||||
|
val timeline = FakeTimeline(timelineItems = timelineItems).apply {
|
||||||
|
isEventLoadedLambda = { it == fullyReadEventId }
|
||||||
|
}
|
||||||
|
val room = FakeJoinedRoom(
|
||||||
|
liveTimeline = timeline,
|
||||||
|
baseRoom = FakeBaseRoom(
|
||||||
|
roomPermissions = roomPermissions(),
|
||||||
|
initialRoomInfo = aRoomInfo(fullyReadEventId = fullyReadEventId, numUnreadMessages = 1),
|
||||||
|
),
|
||||||
|
)
|
||||||
|
val presenter = createTimelinePresenter(
|
||||||
|
timeline = timeline,
|
||||||
|
room = room,
|
||||||
|
featureFlagService = FakeFeatureFlagService(initialState = mapOf(FeatureFlags.JumpToUnread.key to true)),
|
||||||
|
)
|
||||||
|
presenter.test {
|
||||||
|
awaitFirstItem()
|
||||||
|
// Displayed items don't contain the marker event, but the SDK reports it as loaded.
|
||||||
|
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())),
|
||||||
|
)
|
||||||
|
)
|
||||||
|
advanceUntilIdle()
|
||||||
|
// It must never be OutOfWindow — there is nothing displayable to jump to.
|
||||||
|
val drained = consumeItemsUntilTimeout()
|
||||||
|
assertThat(drained.any { it.jumpToUnread is JumpToUnreadState.OutOfWindow }).isFalse()
|
||||||
|
assertThat(drained.last().jumpToUnread).isEqualTo(JumpToUnreadState.Hidden)
|
||||||
|
cancelAndIgnoreRemainingEvents()
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `present - jumpToUnread is Hidden when out of window but there are no unread messages`() = runTest {
|
||||||
|
val timelineItems = MutableStateFlow(emptyList<MatrixTimelineItem>())
|
||||||
|
val fullyReadEventId = EventId("\$older-than-loaded-window")
|
||||||
|
// isEventLoaded defaults to false (genuinely out of window), but numUnreadMessages is 0.
|
||||||
|
val timeline = FakeTimeline(timelineItems = timelineItems)
|
||||||
|
val room = FakeJoinedRoom(
|
||||||
|
liveTimeline = timeline,
|
||||||
|
baseRoom = FakeBaseRoom(
|
||||||
|
roomPermissions = roomPermissions(),
|
||||||
|
initialRoomInfo = aRoomInfo(fullyReadEventId = fullyReadEventId, numUnreadMessages = 0),
|
||||||
|
),
|
||||||
|
)
|
||||||
|
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())),
|
||||||
|
MatrixTimelineItem.Event(UniqueId("2"), anEventTimelineItem(eventId = AN_EVENT_ID_2, content = aMessageContent())),
|
||||||
|
)
|
||||||
|
)
|
||||||
|
advanceUntilIdle()
|
||||||
|
val drained = consumeItemsUntilTimeout()
|
||||||
|
assertThat(drained.any { it.jumpToUnread != JumpToUnreadState.Hidden }).isFalse()
|
||||||
|
cancelAndIgnoreRemainingEvents()
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
fun `present - jumpToUnread is Hidden when fullyReadEventId IS in the loaded window`() = runTest {
|
fun `present - jumpToUnread is Hidden when fullyReadEventId IS in the loaded window`() = runTest {
|
||||||
val timelineItems = MutableStateFlow(emptyList<MatrixTimelineItem>())
|
val timelineItems = MutableStateFlow(emptyList<MatrixTimelineItem>())
|
||||||
@@ -766,7 +839,7 @@ class TimelinePresenterTest {
|
|||||||
liveTimeline = timeline,
|
liveTimeline = timeline,
|
||||||
baseRoom = FakeBaseRoom(
|
baseRoom = FakeBaseRoom(
|
||||||
roomPermissions = roomPermissions(),
|
roomPermissions = roomPermissions(),
|
||||||
initialRoomInfo = aRoomInfo(fullyReadEventId = fullyReadEventId),
|
initialRoomInfo = aRoomInfo(fullyReadEventId = fullyReadEventId, numUnreadMessages = 1),
|
||||||
),
|
),
|
||||||
)
|
)
|
||||||
val presenter = createTimelinePresenter(
|
val presenter = createTimelinePresenter(
|
||||||
|
|||||||
+7
@@ -216,6 +216,13 @@ interface Timeline : AutoCloseable {
|
|||||||
|
|
||||||
suspend fun loadReplyDetails(eventId: EventId): InReplyTo
|
suspend fun loadReplyDetails(eventId: EventId): InReplyTo
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Returns true if [eventId] is currently loaded in this timeline's window, even if the display
|
||||||
|
* layer does not render it (e.g. filtered or state events). This distinguishes "in the window
|
||||||
|
* but not displayed" from "genuinely outside the loaded window".
|
||||||
|
*/
|
||||||
|
suspend fun isEventLoaded(eventId: EventId): Boolean
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Adds a new pinned event by sending an updated `m.room.pinned_events`
|
* Adds a new pinned event by sending an updated `m.room.pinned_events`
|
||||||
* event containing the new event id.
|
* event containing the new event id.
|
||||||
|
|||||||
+18
@@ -637,6 +637,24 @@ class RustTimeline(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
override suspend fun isEventLoaded(eventId: EventId): Boolean = withContext(dispatcher) {
|
||||||
|
val isInLoadedItems = _timelineItems.replayCache.firstOrNull().orEmpty().any { timelineItem ->
|
||||||
|
timelineItem is MatrixTimelineItem.Event && timelineItem.eventId == eventId
|
||||||
|
}
|
||||||
|
if (isInLoadedItems) {
|
||||||
|
// Displayed events are caught above. The SDK-level list still holds events the display
|
||||||
|
// layer later drops, so this avoids the FFI call for in-window-but-not-rendered events.
|
||||||
|
true
|
||||||
|
} else {
|
||||||
|
// getEventTimelineItemByEventId throws when the event isn't in the loaded window.
|
||||||
|
// EventTimelineItem is a Disposable wrapping a native handle, so release it once we've
|
||||||
|
// confirmed presence — otherwise the handle lingers until the GC cleaner runs.
|
||||||
|
runCatchingExceptions {
|
||||||
|
inner.getEventTimelineItemByEventId(eventId.value).use { /* presence is all we need */ }
|
||||||
|
}.isSuccess
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
override suspend fun pinEvent(eventId: EventId): Result<Boolean> = withContext(dispatcher) {
|
override suspend fun pinEvent(eventId: EventId): Result<Boolean> = withContext(dispatcher) {
|
||||||
runCatchingExceptions {
|
runCatchingExceptions {
|
||||||
inner.pinEvent(eventId = eventId.value)
|
inner.pinEvent(eventId = eventId.value)
|
||||||
|
|||||||
+3
@@ -436,6 +436,9 @@ class FakeTimeline(
|
|||||||
|
|
||||||
override suspend fun loadReplyDetails(eventId: EventId) = loadReplyDetailsLambda(eventId)
|
override suspend fun loadReplyDetails(eventId: EventId) = loadReplyDetailsLambda(eventId)
|
||||||
|
|
||||||
|
var isEventLoadedLambda: (eventId: EventId) -> Boolean = { false }
|
||||||
|
override suspend fun isEventLoaded(eventId: EventId): Boolean = isEventLoadedLambda(eventId)
|
||||||
|
|
||||||
var pinEventLambda: (eventId: EventId) -> Result<Boolean> = { lambdaError() }
|
var pinEventLambda: (eventId: EventId) -> Result<Boolean> = { lambdaError() }
|
||||||
override suspend fun pinEvent(eventId: EventId): Result<Boolean> {
|
override suspend fun pinEvent(eventId: EventId): Result<Boolean> {
|
||||||
return pinEventLambda(eventId)
|
return pinEventLambda(eventId)
|
||||||
|
|||||||
Reference in New Issue
Block a user