From 11e3cb9b19f59a661d48ef60da58ef0cac2694a1 Mon Sep 17 00:00:00 2001 From: hayaksi1 <193020925+hayaksi1@users.noreply.github.com> Date: Mon, 17 Aug 2026 16:20:43 +0300 Subject: [PATCH 1/2] Keep a focused timeline focused until the event has been rendered Opening a room from a notification asks the timeline to focus the notified event. When that event is outside the loaded live window the controller creates a detached timeline for it, and a detached timeline always carries a forward loading indicator as its last chronological item. The items list is reversed for display, so that indicator is the bottom-most row and is composed on first layout, which unconditionally emits a forward pagination request nobody asked for. Because a notified event sits close to the live end, that one page reaches the end, and the controller responds by closing the detached timeline. The live window replaces the item list while it is still anchored at the bottom, so the user lands on the newest message and the focus request is discarded. Defer automatic forward pagination while a focus request is still pending. Once the focused event has been rendered the viewport has moved off the indicator row, so only a real scroll back to the live end switches to the live timeline, which is what the controller's existing behaviour is for. Backward pagination and the explicit jump-to-live action are untouched. --- .../impl/timeline/TimelinePresenter.kt | 21 ++++- .../impl/timeline/TimelinePresenterTest.kt | 88 +++++++++++++++++++ 2 files changed, 106 insertions(+), 3 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 7cd0d018d2..3f98d2a885 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 @@ -166,9 +166,14 @@ class TimelinePresenter( fun handleEvent(event: TimelineEvent) { when (event) { is TimelineEvent.LoadMore -> { - if (event.direction == Timeline.PaginationDirection.FORWARDS && timelineMode is Timeline.Mode.Thread) { - // Do not paginate forwards in thread mode, as it's not supported - return + if (event.direction == Timeline.PaginationDirection.FORWARDS) { + if (timelineMode is Timeline.Mode.Thread) { + // Do not paginate forwards in thread mode, as it's not supported + return + } + if (focusRequestState.value.isPending()) { + return + } } localScope.launch { timelineController.paginate(direction = event.direction) @@ -537,6 +542,16 @@ private fun FocusRequestState.onFocusEventRender(): FocusRequestState { } } +private fun FocusRequestState.isPending(): Boolean { + return when (this) { + is FocusRequestState.Requested, + is FocusRequestState.Loading -> true + is FocusRequestState.Success -> !rendered + FocusRequestState.None, + is FocusRequestState.Failure -> false + } +} + // 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/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 3b0794305f..3b90196974 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 @@ -1059,6 +1059,69 @@ class TimelinePresenterTest { } } + @Test + fun `present - a forward pagination while the focused event is not rendered yet is ignored`() = runTest { + val paginateLambda = lambdaRecorder(ensureNeverCalled = true) { _: Timeline.PaginationDirection -> + Result.success(true) + } + val presenter = createTimelinePresenter( + room = aRoomWithDetachedTimeline(paginateLambda = paginateLambda), + ) + presenter.test { + val initialState = awaitFirstItem() + initialState.eventSink.invoke(TimelineEvent.FocusOnEvent(AN_EVENT_ID)) + val focusedState = consumeItemsUntilPredicate { it.focusRequestState == FocusRequestState.Success(AN_EVENT_ID) }.last() + assertThat(focusedState.isLive).isFalse() + initialState.eventSink.invoke(TimelineEvent.LoadMore(Timeline.PaginationDirection.FORWARDS)) + advanceUntilIdle() + assert(paginateLambda).isNeverCalled() + assertThat(expectMostRecentItem().isLive).isFalse() + } + } + + @Test + fun `present - a forward pagination is performed once the focused event has been rendered`() = runTest { + val paginateLambda = lambdaRecorder { _: Timeline.PaginationDirection -> + Result.success(true) + } + val presenter = createTimelinePresenter( + room = aRoomWithDetachedTimeline(paginateLambda = paginateLambda), + ) + presenter.test { + val initialState = awaitFirstItem() + initialState.eventSink.invoke(TimelineEvent.FocusOnEvent(AN_EVENT_ID)) + consumeItemsUntilPredicate { it.focusRequestState == FocusRequestState.Success(AN_EVENT_ID) } + initialState.eventSink.invoke(TimelineEvent.OnFocusEventRender) + initialState.eventSink.invoke(TimelineEvent.LoadMore(Timeline.PaginationDirection.FORWARDS)) + advanceUntilIdle() + assert(paginateLambda) + .isCalledOnce() + .with(value(Timeline.PaginationDirection.FORWARDS)) + assertThat(expectMostRecentItem().isLive).isTrue() + } + } + + @Test + fun `present - a backward pagination while the focused event is not rendered yet is performed`() = runTest { + val paginateLambda = lambdaRecorder { _: Timeline.PaginationDirection -> + Result.success(false) + } + val presenter = createTimelinePresenter( + room = aRoomWithDetachedTimeline(paginateLambda = paginateLambda), + ) + presenter.test { + val initialState = awaitFirstItem() + initialState.eventSink.invoke(TimelineEvent.FocusOnEvent(AN_EVENT_ID)) + consumeItemsUntilPredicate { it.focusRequestState == FocusRequestState.Success(AN_EVENT_ID) } + initialState.eventSink.invoke(TimelineEvent.LoadMore(Timeline.PaginationDirection.BACKWARDS)) + advanceUntilIdle() + assert(paginateLambda) + .isCalledOnce() + .with(value(Timeline.PaginationDirection.BACKWARDS)) + cancelAndIgnoreRemainingEvents() + } + } + @Test fun `present - focus on known event retrieves the event from cache`() = runTest { val timelineItemIndexer = TimelineItemIndexer() @@ -1552,6 +1615,31 @@ class TimelinePresenterTest { return awaitItem() } + private fun aRoomWithDetachedTimeline( + paginateLambda: (Timeline.PaginationDirection) -> Result, + ): FakeJoinedRoom { + val detachedTimeline = FakeTimeline( + timelineItems = flowOf( + listOf( + MatrixTimelineItem.Event( + uniqueId = A_UNIQUE_ID, + event = anEventTimelineItem(eventId = AN_EVENT_ID), + ) + ) + ) + ).apply { + this.paginateLambda = paginateLambda + } + return FakeJoinedRoom( + liveTimeline = FakeTimeline(timelineItems = flowOf(emptyList())), + createTimelineResult = { Result.success(detachedTimeline) }, + baseRoom = FakeBaseRoom( + roomPermissions = roomPermissions(), + threadRootIdForEventResult = { _ -> Result.success(null) }, + ), + ) + } + private fun roomPermissions( canRedactOther: Boolean = false, canRedactOwn: Boolean = true, From 0aa283729c9bef281e883e172115ece686db19ff Mon Sep 17 00:00:00 2001 From: hayaksi1 <193020925+hayaksi1@users.noreply.github.com> Date: Tue, 18 Aug 2026 00:49:03 +0300 Subject: [PATCH 2/2] Cover every branch of the forward pagination gate Codecov reported 55.55% patch coverage on the previous commit: four of the nine measured lines were only partially covered. No test paginates forwards in a thread timeline, and the focus states Requested, Loading and Failure were never live when a forward pagination arrived, so only the Success branch of isPending() was ever evaluated both ways. Add a test per uncovered state: a forward pagination is deferred while the focus request is still being debounced and while the focused timeline is being created, it is performed again once the request has failed, and it is dropped in a thread timeline while a backward one still goes through. The pagination recorder now also sits on the live timeline, and the presenter is built on that same timeline. Before the detached timeline exists, a pagination that escaped the gate would go to the live one, so without this the new guards would pass against the unfixed presenter. The deferred cases end on the emitted state rather than on cancelAndIgnoreRemainingEvents for the same reason: the recorder throws from inside the presenter coroutine and cancelling swallows it. Verified by reverting the gate: the three deferred-pagination tests then fail with "This lambda should never be called." --- .../impl/timeline/TimelinePresenterTest.kt | 140 +++++++++++++++--- 1 file changed, 123 insertions(+), 17 deletions(-) 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 3b90196974..4847c81f28 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 @@ -1064,8 +1064,10 @@ class TimelinePresenterTest { val paginateLambda = lambdaRecorder(ensureNeverCalled = true) { _: Timeline.PaginationDirection -> Result.success(true) } + val room = aRoomWithDetachedTimeline(paginateLambda = paginateLambda) val presenter = createTimelinePresenter( - room = aRoomWithDetachedTimeline(paginateLambda = paginateLambda), + room = room, + timeline = room.liveTimeline, ) presenter.test { val initialState = awaitFirstItem() @@ -1084,8 +1086,10 @@ class TimelinePresenterTest { val paginateLambda = lambdaRecorder { _: Timeline.PaginationDirection -> Result.success(true) } + val room = aRoomWithDetachedTimeline(paginateLambda = paginateLambda) val presenter = createTimelinePresenter( - room = aRoomWithDetachedTimeline(paginateLambda = paginateLambda), + room = room, + timeline = room.liveTimeline, ) presenter.test { val initialState = awaitFirstItem() @@ -1106,8 +1110,10 @@ class TimelinePresenterTest { val paginateLambda = lambdaRecorder { _: Timeline.PaginationDirection -> Result.success(false) } + val room = aRoomWithDetachedTimeline(paginateLambda = paginateLambda) val presenter = createTimelinePresenter( - room = aRoomWithDetachedTimeline(paginateLambda = paginateLambda), + room = room, + timeline = room.liveTimeline, ) presenter.test { val initialState = awaitFirstItem() @@ -1122,6 +1128,100 @@ class TimelinePresenterTest { } } + @Test + fun `present - a forward pagination while the focus request is debounced is ignored`() = runTest { + val paginateLambda = lambdaRecorder(ensureNeverCalled = true) { _: Timeline.PaginationDirection -> + Result.success(true) + } + val room = aRoomWithDetachedTimeline(paginateLambda = paginateLambda) + val presenter = createTimelinePresenter( + room = room, + timeline = room.liveTimeline, + ) + presenter.test { + val initialState = awaitFirstItem() + initialState.eventSink.invoke(TimelineEvent.FocusOnEvent(AN_EVENT_ID, debounce = 1.seconds)) + // The focus request has started and is now waiting for the debounce to elapse + consumeItemsUntilPredicate { it.focusRequestState is FocusRequestState.Requested } + initialState.eventSink.invoke(TimelineEvent.LoadMore(Timeline.PaginationDirection.FORWARDS)) + advanceUntilIdle() + assert(paginateLambda).isNeverCalled() + assertThat(expectMostRecentItem().isLive).isFalse() + } + } + + @Test + fun `present - a forward pagination while the focused timeline is loading is ignored`() = runTest { + val paginateLambda = lambdaRecorder(ensureNeverCalled = true) { _: Timeline.PaginationDirection -> + Result.success(true) + } + val room = aRoomWithDetachedTimeline(paginateLambda = paginateLambda) + val presenter = createTimelinePresenter( + room = room, + timeline = room.liveTimeline, + ) + presenter.test { + val initialState = awaitFirstItem() + initialState.eventSink.invoke(TimelineEvent.FocusOnEvent(AN_EVENT_ID)) + // The focused timeline is being created, the focus request has not resolved yet + consumeItemsUntilPredicate { it.focusRequestState is FocusRequestState.Loading } + initialState.eventSink.invoke(TimelineEvent.LoadMore(Timeline.PaginationDirection.FORWARDS)) + advanceUntilIdle() + assert(paginateLambda).isNeverCalled() + assertThat(expectMostRecentItem().isLive).isFalse() + } + } + + @Test + fun `present - a forward pagination after a failed focus request is performed`() = runTest { + val paginateLambda = lambdaRecorder { _: Timeline.PaginationDirection -> + Result.success(false) + } + val room = aRoomWithDetachedTimeline( + paginateLambda = paginateLambda, + createTimelineResult = { Result.failure(RuntimeException("An error")) }, + ) + val presenter = createTimelinePresenter( + room = room, + timeline = room.liveTimeline, + ) + presenter.test { + val initialState = awaitFirstItem() + initialState.eventSink.invoke(TimelineEvent.FocusOnEvent(AN_EVENT_ID)) + consumeItemsUntilPredicate { it.focusRequestState is FocusRequestState.Failure } + initialState.eventSink.invoke(TimelineEvent.LoadMore(Timeline.PaginationDirection.FORWARDS)) + advanceUntilIdle() + assert(paginateLambda) + .isCalledOnce() + .with(value(Timeline.PaginationDirection.FORWARDS)) + cancelAndIgnoreRemainingEvents() + } + } + + @Test + fun `present - a forward pagination in a thread timeline is ignored`() = runTest { + val paginateLambda = lambdaRecorder { _: Timeline.PaginationDirection -> + Result.success(true) + } + val timeline = FakeTimeline(mode = Timeline.Mode.Thread(A_THREAD_ID)).apply { + this.paginateLambda = paginateLambda + } + val presenter = createTimelinePresenter(timeline = timeline) + presenter.test { + val initialState = awaitFirstItem() + initialState.eventSink.invoke(TimelineEvent.LoadMore(Timeline.PaginationDirection.FORWARDS)) + advanceUntilIdle() + assert(paginateLambda).isNeverCalled() + // A backward pagination is still performed in a thread timeline + initialState.eventSink.invoke(TimelineEvent.LoadMore(Timeline.PaginationDirection.BACKWARDS)) + advanceUntilIdle() + assert(paginateLambda) + .isCalledOnce() + .with(value(Timeline.PaginationDirection.BACKWARDS)) + cancelAndIgnoreRemainingEvents() + } + } + @Test fun `present - focus on known event retrieves the event from cache`() = runTest { val timelineItemIndexer = TimelineItemIndexer() @@ -1617,22 +1717,13 @@ class TimelinePresenterTest { private fun aRoomWithDetachedTimeline( paginateLambda: (Timeline.PaginationDirection) -> Result, + createTimelineResult: () -> Result = { Result.success(aDetachedTimeline(paginateLambda)) }, ): FakeJoinedRoom { - val detachedTimeline = FakeTimeline( - timelineItems = flowOf( - listOf( - MatrixTimelineItem.Event( - uniqueId = A_UNIQUE_ID, - event = anEventTimelineItem(eventId = AN_EVENT_ID), - ) - ) - ) - ).apply { - this.paginateLambda = paginateLambda - } return FakeJoinedRoom( - liveTimeline = FakeTimeline(timelineItems = flowOf(emptyList())), - createTimelineResult = { Result.success(detachedTimeline) }, + liveTimeline = FakeTimeline(timelineItems = flowOf(emptyList())).apply { + this.paginateLambda = paginateLambda + }, + createTimelineResult = { createTimelineResult() }, baseRoom = FakeBaseRoom( roomPermissions = roomPermissions(), threadRootIdForEventResult = { _ -> Result.success(null) }, @@ -1640,6 +1731,21 @@ class TimelinePresenterTest { ) } + private fun aDetachedTimeline( + paginateLambda: (Timeline.PaginationDirection) -> Result, + ) = FakeTimeline( + timelineItems = flowOf( + listOf( + MatrixTimelineItem.Event( + uniqueId = A_UNIQUE_ID, + event = anEventTimelineItem(eventId = AN_EVENT_ID), + ) + ) + ) + ).apply { + this.paginateLambda = paginateLambda + } + private fun roomPermissions( canRedactOther: Boolean = false, canRedactOwn: Boolean = true,