From f95a959ed18f1f4ada4ad8e7138e50dcc82d3596 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20Mart=C3=ADn?= Date: Mon, 17 Mar 2025 18:22:03 +0100 Subject: [PATCH] Fix tests and lint issues --- .../messages/impl/timeline/TimelineView.kt | 18 +++++++--- .../messages/impl/MessagesViewTest.kt | 27 ++++++++++++-- .../impl/timeline/TimelineViewTest.kt | 35 +++++++++++++++++-- .../android/libraries/testtags/TestTags.kt | 5 +++ 4 files changed, 75 insertions(+), 10 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 8172617d86..afd0834ad8 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 @@ -72,6 +72,8 @@ import io.element.android.libraries.designsystem.utils.animateScrollToItemCenter import io.element.android.libraries.matrix.api.core.EventId import io.element.android.libraries.matrix.api.core.UserId import io.element.android.libraries.matrix.api.timeline.Timeline +import io.element.android.libraries.testtags.TestTags +import io.element.android.libraries.testtags.testTag import io.element.android.libraries.ui.strings.CommonStrings import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.collectLatest @@ -143,7 +145,8 @@ fun TimelineView( LazyColumn( modifier = Modifier .fillMaxSize() - .nestedScroll(nestedScrollConnection), + .nestedScroll(nestedScrollConnection) + .testTag(TestTags.timeline), state = lazyListState, reverseLayout = useReverseLayout, contentPadding = PaddingValues(vertical = 8.dp), @@ -220,6 +223,8 @@ private fun TimelinePrefetchingHelper( lazyListState: LazyListState, prefetch: () -> Unit, ) { + val latestPrefetch by rememberUpdatedState(prefetch) + // We're using snapshot flows for these because using `LaunchedEffect` with `derivedState` doesn't seem to be responsive enough val firstVisibleItemIndexFlow = snapshotFlow { lazyListState.firstVisibleItemIndex @@ -231,19 +236,22 @@ private fun TimelinePrefetchingHelper( lazyListState.isScrollInProgress } - LaunchedEffect(Unit) { + LaunchedEffect(latestPrefetch) { val isCloseToStartOfLoadedTimelineFlow = combine(layoutInfoFlow, firstVisibleItemIndexFlow) { layoutInfo, firstVisibleItemIndex -> firstVisibleItemIndex + layoutInfo.visibleItemsInfo.size >= layoutInfo.totalItemsCount - 40 } - combine(isCloseToStartOfLoadedTimelineFlow, isScrollingFlow) { needsPrefetch, isScrolling -> + combine( + isCloseToStartOfLoadedTimelineFlow, + isScrollingFlow, + ) { needsPrefetch, isScrolling -> needsPrefetch && isScrolling } .distinctUntilChanged() .collectLatest { needsPrefetch -> if (needsPrefetch) { Timber.d("Prefetching pagination with ${lazyListState.layoutInfo.totalItemsCount} items") - prefetch() + latestPrefetch() } } } @@ -274,7 +282,7 @@ private fun BoxScope.TimelineScrollHelper( coroutineScope.launch { if (lazyListState.firstVisibleItemIndex > 10) { lazyListState.scrollToItem(0) - } else { + } else if (lazyListState.firstVisibleItemIndex != 0) { lazyListState.animateScrollToItem(0) } } diff --git a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/MessagesViewTest.kt b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/MessagesViewTest.kt index a7c76d73e2..9f46637948 100644 --- a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/MessagesViewTest.kt +++ b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/MessagesViewTest.kt @@ -41,6 +41,7 @@ import io.element.android.features.messages.impl.pinned.banner.aLoadedPinnedMess import io.element.android.features.messages.impl.timeline.FOCUS_ON_PINNED_EVENT_DEBOUNCE_DURATION_IN_MILLIS import io.element.android.features.messages.impl.timeline.TimelineEvents import io.element.android.features.messages.impl.timeline.aTimelineItemEvent +import io.element.android.features.messages.impl.timeline.aTimelineItemList import io.element.android.features.messages.impl.timeline.aTimelineItemReadReceipts import io.element.android.features.messages.impl.timeline.aTimelineRoomInfo import io.element.android.features.messages.impl.timeline.aTimelineState @@ -50,6 +51,7 @@ import io.element.android.features.messages.impl.timeline.components.reactionsum import io.element.android.features.messages.impl.timeline.components.receipt.aReadReceiptData import io.element.android.features.messages.impl.timeline.components.receipt.bottomsheet.ReadReceiptBottomSheetEvents import io.element.android.features.messages.impl.timeline.model.TimelineItem +import io.element.android.features.messages.impl.timeline.model.event.aTimelineItemTextContent import io.element.android.libraries.matrix.api.core.UserId import io.element.android.libraries.matrix.test.AN_EVENT_ID import io.element.android.libraries.testtags.TestTags @@ -126,6 +128,9 @@ class MessagesViewTest { fun `clicking on an Event invoke expected callback`() { val eventsRecorder = EventsRecorder(expectEvents = false) val state = aMessagesState( + timelineState = aTimelineState( + timelineItems = aTimelineItemList(aTimelineItemTextContent()), + ), eventSink = eventsRecorder ) val timelineItem = state.timelineState.timelineItems.first() @@ -182,6 +187,9 @@ class MessagesViewTest { canSendReaction = userHasPermissionToSendReaction, canPinUnpin = userCanPinEvent, ), + timelineState = aTimelineState( + timelineItems = aTimelineItemList(aTimelineItemTextContent()), + ), ) val timelineItem = state.timelineState.timelineItems.first() as TimelineItem.Event rule.setMessagesView( @@ -349,7 +357,10 @@ class MessagesViewTest { fun `clicking on a reaction emits the expected Event`() { val eventsRecorder = EventsRecorder() val state = aMessagesState( - eventSink = eventsRecorder + timelineState = aTimelineState( + timelineItems = aTimelineItemList(aTimelineItemTextContent()), + ), + eventSink = eventsRecorder, ) val timelineItem = state.timelineState.timelineItems.first() as TimelineItem.Event rule.setMessagesView( @@ -363,6 +374,9 @@ class MessagesViewTest { fun `long clicking on a reaction emits the expected Event`() { val eventsRecorder = EventsRecorder() val state = aMessagesState( + timelineState = aTimelineState( + timelineItems = aTimelineItemList(aTimelineItemTextContent()), + ), reactionSummaryState = aReactionSummaryState( target = null, eventSink = eventsRecorder, @@ -380,6 +394,9 @@ class MessagesViewTest { fun `clicking on more reaction emits the expected Event`() { val eventsRecorder = EventsRecorder() val state = aMessagesState( + timelineState = aTimelineState( + timelineItems = aTimelineItemList(aTimelineItemTextContent()), + ), customReactionState = aCustomReactionState( eventSink = eventsRecorder, ), @@ -396,7 +413,11 @@ class MessagesViewTest { @Test fun `clicking on more reaction from action list emits the expected Event`() { val eventsRecorder = EventsRecorder() - val state = aMessagesState() + val state = aMessagesState( + timelineState = aTimelineState( + timelineItems = aTimelineItemList(aTimelineItemTextContent()), + ), + ) val timelineItem = state.timelineState.timelineItems.first() as TimelineItem.Event val stateWithActionListState = state.copy( actionListState = anActionListState( @@ -538,7 +559,7 @@ private fun AndroidComposeTestRule.setMessa onCreatePollClick = onCreatePollClick, onJoinCallClick = onJoinCallClick, onViewAllPinnedMessagesClick = onViewAllPinnedMessagesClick, - knockRequestsBannerView = {} + knockRequestsBannerView = {}, ) } } diff --git a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/timeline/TimelineViewTest.kt b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/timeline/TimelineViewTest.kt index 61e742f872..0c434aef15 100644 --- a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/timeline/TimelineViewTest.kt +++ b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/timeline/TimelineViewTest.kt @@ -11,14 +11,18 @@ import androidx.activity.ComponentActivity import androidx.compose.ui.test.junit4.AndroidComposeTestRule import androidx.compose.ui.test.junit4.createAndroidComposeRule import androidx.compose.ui.test.onNodeWithContentDescription +import androidx.compose.ui.test.onNodeWithTag import androidx.compose.ui.test.performClick +import androidx.compose.ui.test.performScrollToIndex import androidx.test.ext.junit.runners.AndroidJUnit4 import io.element.android.features.messages.impl.timeline.components.aCriticalShield import io.element.android.features.messages.impl.timeline.model.TimelineItem import io.element.android.features.messages.impl.timeline.model.event.aTimelineItemImageContent +import io.element.android.features.messages.impl.timeline.model.event.aTimelineItemUnknownContent import io.element.android.features.messages.impl.timeline.model.virtual.TimelineItemLoadingIndicatorModel 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.matrix.api.core.EventId import io.element.android.libraries.matrix.api.core.UniqueId import io.element.android.libraries.matrix.api.core.UserId import io.element.android.libraries.matrix.api.timeline.Timeline @@ -31,6 +35,7 @@ import io.element.android.tests.testutils.EventsRecorder import io.element.android.tests.testutils.clickOn import io.element.android.tests.testutils.setSafeContent import kotlinx.collections.immutable.persistentListOf +import kotlinx.collections.immutable.toPersistentList import org.junit.Rule import org.junit.Test import org.junit.rules.TestRule @@ -114,8 +119,6 @@ class TimelineViewTest { rule.onNodeWithContentDescription(contentDescription).performClick() eventsRecorder.assertList( listOf( - TimelineEvents.OnScrollFinished(0), - TimelineEvents.OnScrollFinished(0), TimelineEvents.OnScrollFinished(0), TimelineEvents.ShowShieldDialog(MessageShield.UnverifiedIdentity(true)), ) @@ -135,6 +138,34 @@ class TimelineViewTest { rule.clickOn(CommonStrings.action_ok) eventsRecorder.assertSingle(TimelineEvents.HideShieldDialog) } + + @Test + fun `scrolling near to the start of the loaded items triggers a pre-fetch`() { + val eventsRecorder = EventsRecorder() + val items = List(20) { + aTimelineItemEvent( + eventId = EventId("\$event_$it"), + content = aTimelineItemUnknownContent(), + ) + }.toPersistentList() + + rule.setTimelineView( + state = aTimelineState( + timelineItems = items, + eventSink = eventsRecorder, + focusedEventIndex = -1, + isLive = false, + ), + ) + + rule.onNodeWithTag("timeline").performScrollToIndex(10) + eventsRecorder.assertList( + listOf( + TimelineEvents.OnScrollFinished(firstIndex = 0), + TimelineEvents.LoadMore(Timeline.PaginationDirection.BACKWARDS), + ) + ) + } } private fun AndroidComposeTestRule.setTimelineView( 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 ddc720ed40..8ee03dd5ad 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 @@ -97,6 +97,11 @@ object TestTags { */ val floatingActionButton = TestTag("floating-action-button") + /** + * Timeline. + */ + val timeline = TestTag("timeline") + /** * Timeline item. */