diff --git a/appnav/src/main/kotlin/io/element/android/appnav/RoomFlowNode.kt b/appnav/src/main/kotlin/io/element/android/appnav/RoomFlowNode.kt index 4083ed72fc..a0867facf2 100644 --- a/appnav/src/main/kotlin/io/element/android/appnav/RoomFlowNode.kt +++ b/appnav/src/main/kotlin/io/element/android/appnav/RoomFlowNode.kt @@ -18,6 +18,7 @@ package io.element.android.appnav import android.os.Parcelable import androidx.compose.runtime.Composable +import androidx.compose.runtime.DisposableEffect import androidx.compose.ui.Modifier import androidx.lifecycle.lifecycleScope import com.bumble.appyx.core.composable.Children @@ -88,14 +89,12 @@ class RoomFlowNode @AssistedInject constructor( lifecycle.subscribe( onCreate = { Timber.v("OnCreate") - inputs.room.open() plugins().forEach { it.onFlowCreated(id, inputs.room) } appNavigationStateService.onNavigateToRoom(id, inputs.room.roomId) fetchRoomMembers() }, onDestroy = { Timber.v("OnDestroy") - inputs.room.close() plugins().forEach { it.onFlowReleased(id, inputs.room) } appNavigationStateService.onLeavingRoom(id) } @@ -161,6 +160,15 @@ class RoomFlowNode @AssistedInject constructor( @Composable override fun View(modifier: Modifier) { + // Rely on the View Lifecycle instead of the Node Lifecycle, + // because this node enters 'onDestroy' before his children, so it can leads to + // using the room in a child node where it's already closed. + DisposableEffect(Unit) { + inputs.room.open() + onDispose { + inputs.room.close() + } + } Children( navModel = backstack, modifier = modifier, 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 8513d8e605..071dc84116 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 @@ -59,21 +59,16 @@ class TimelinePresenter @Inject constructor( var lastReadMarkerIndex by rememberSaveable { mutableStateOf(Int.MAX_VALUE) } var lastReadMarkerId by rememberSaveable { mutableStateOf(null) } - val timelineItems = timelineItemsFactory - .flow() - .collectAsState() - - val paginationState = timeline - .paginationState() - .collectAsState() + val timelineItems by timelineItemsFactory.collectItemsAsState() + val paginationState by timeline.paginationState.collectAsState() fun handleEvents(event: TimelineEvents) { when (event) { - TimelineEvents.LoadMore -> localCoroutineScope.loadMore(paginationState.value) + TimelineEvents.LoadMore -> localCoroutineScope.loadMore(paginationState) is TimelineEvents.SetHighlightedEvent -> highlightedEventId.value = event.eventId is TimelineEvents.OnScrollFinished -> { // Get last valid EventId seen by the user, as the first index might refer to a Virtual item - val eventId = getLastEventIdBeforeOrAt(event.firstIndex, timelineItems.value) ?: return + val eventId = getLastEventIdBeforeOrAt(event.firstIndex, timelineItems) ?: return if (event.firstIndex <= lastReadMarkerIndex && eventId != lastReadMarkerId) { lastReadMarkerIndex = event.firstIndex lastReadMarkerId = eventId @@ -85,11 +80,11 @@ class TimelinePresenter @Inject constructor( LaunchedEffect(Unit) { timeline - .timelineItems() + .timelineItems .onEach(timelineItemsFactory::replaceWith) .onEach { timelineItems -> if (timelineItems.isEmpty()) { - loadMore(paginationState.value) + loadMore(paginationState) } } .launchIn(this) @@ -97,8 +92,8 @@ class TimelinePresenter @Inject constructor( return TimelineState( highlightedEventId = highlightedEventId.value, - paginationState = paginationState.value, - timelineItems = timelineItems.value, + paginationState = paginationState, + timelineItems = timelineItems, eventSink = ::handleEvents ) } 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 4d7f2b9978..74bb593f11 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 @@ -39,7 +39,6 @@ import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.runtime.saveable.rememberSaveable -import androidx.compose.runtime.snapshotFlow import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.res.pluralStringResource @@ -62,7 +61,6 @@ import io.element.android.libraries.designsystem.theme.components.Icon import io.element.android.libraries.matrix.api.core.EventId import io.element.android.libraries.matrix.api.core.UserId import kotlinx.collections.immutable.ImmutableList -import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.launch @Composable @@ -123,8 +121,7 @@ fun TimelineView( TimelineScrollHelper( lazyListState = lazyListState, - timelineItems = state.timelineItems, - onLoadMore = ::onReachedLoadMore + timelineItems = state.timelineItems ) } } @@ -222,7 +219,6 @@ fun TimelineItemRow( internal fun BoxScope.TimelineScrollHelper( lazyListState: LazyListState, timelineItems: ImmutableList, - onLoadMore: () -> Unit = {}, ) { val coroutineScope = rememberCoroutineScope() val firstVisibleItemIndex by remember { derivedStateOf { lazyListState.firstVisibleItemIndex } } @@ -236,24 +232,6 @@ internal fun BoxScope.TimelineScrollHelper( } } - // Handle load more preloading - val loadMore by remember { - derivedStateOf { - val layoutInfo = lazyListState.layoutInfo - val totalItemsNumber = layoutInfo.totalItemsCount - val lastVisibleItemIndex = (layoutInfo.visibleItemsInfo.lastOrNull()?.index ?: 0) + 1 - lastVisibleItemIndex > (totalItemsNumber - 30) - } - } - - LaunchedEffect(loadMore) { - snapshotFlow { loadMore } - .distinctUntilChanged() - .collect { - onLoadMore() - } - } - // Jump to bottom button if (firstVisibleItemIndex > 2) { FloatingActionButton( diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/factories/TimelineItemsFactory.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/factories/TimelineItemsFactory.kt index c08c8d65a3..5061e9a9e7 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/factories/TimelineItemsFactory.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/factories/TimelineItemsFactory.kt @@ -16,6 +16,9 @@ package io.element.android.features.messages.impl.timeline.factories +import androidx.compose.runtime.Composable +import androidx.compose.runtime.State +import androidx.compose.runtime.collectAsState import androidx.recyclerview.widget.DiffUtil import io.element.android.features.messages.impl.timeline.diff.CacheInvalidator import io.element.android.features.messages.impl.timeline.diff.MatrixTimelineItemsDiffCallback @@ -27,11 +30,8 @@ import io.element.android.libraries.core.coroutine.CoroutineDispatchers import io.element.android.libraries.matrix.api.timeline.MatrixTimelineItem import kotlinx.collections.immutable.ImmutableList import kotlinx.collections.immutable.persistentListOf -import kotlinx.collections.immutable.toImmutableList import kotlinx.collections.immutable.toPersistentList import kotlinx.coroutines.flow.MutableStateFlow -import kotlinx.coroutines.flow.StateFlow -import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock import kotlinx.coroutines.withContext @@ -55,7 +55,10 @@ class TimelineItemsFactory @Inject constructor( private val lock = Mutex() private val cacheInvalidator = CacheInvalidator(timelineItemsCache) - fun flow(): StateFlow> = timelineItems.asStateFlow() + @Composable + fun collectItemsAsState(): State> { + return timelineItems.collectAsState() + } suspend fun replaceWith( timelineItems: List, diff --git a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/timeline/MatrixTimeline.kt b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/timeline/MatrixTimeline.kt index ff7b3e1cad..b74a34bd01 100644 --- a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/timeline/MatrixTimeline.kt +++ b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/timeline/MatrixTimeline.kt @@ -27,8 +27,8 @@ interface MatrixTimeline { val canBackPaginate: Boolean ) - fun paginationState(): StateFlow - fun timelineItems(): Flow> + val paginationState: StateFlow + val timelineItems: Flow> suspend fun paginateBackwards(requestSize: Int, untilNumberOfItems: Int): Result suspend fun fetchDetailsForEvent(eventId: EventId): Result diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/RustMatrixTimeline.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/RustMatrixTimeline.kt index 8c96e48fd7..767a3ce444 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/RustMatrixTimeline.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/RustMatrixTimeline.kt @@ -30,6 +30,7 @@ import kotlinx.coroutines.FlowPreview import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.sample import kotlinx.coroutines.withContext import org.matrix.rustcomponents.sdk.PaginationOptions @@ -45,10 +46,10 @@ class RustMatrixTimeline( private val coroutineDispatchers: CoroutineDispatchers, ) : MatrixTimeline { - private val timelineItems: MutableStateFlow> = + private val _timelineItems: MutableStateFlow> = MutableStateFlow(emptyList()) - private val paginationState = MutableStateFlow( + private val _paginationState = MutableStateFlow( MatrixTimeline.PaginationState(canBackPaginate = true, isBackPaginating = false) ) @@ -64,19 +65,15 @@ class RustMatrixTimeline( ) private val timelineDiffProcessor = MatrixTimelineDiffProcessor( - paginationState = paginationState, - timelineItems = timelineItems, + paginationState = _paginationState, + timelineItems = _timelineItems, timelineItemFactory = timelineItemFactory, ) - override fun paginationState(): StateFlow { - return paginationState - } + override val paginationState: StateFlow = _paginationState.asStateFlow() @OptIn(FlowPreview::class) - override fun timelineItems(): Flow> { - return timelineItems.sample(50) - } + override val timelineItems: Flow> = _timelineItems.sample(50) internal suspend fun postItems(items: List) { timelineDiffProcessor.postItems(items) diff --git a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/timeline/FakeMatrixTimeline.kt b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/timeline/FakeMatrixTimeline.kt index 6bad63b441..db31fbd8f0 100644 --- a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/timeline/FakeMatrixTimeline.kt +++ b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/timeline/FakeMatrixTimeline.kt @@ -23,33 +23,30 @@ import kotlinx.coroutines.delay import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.getAndUpdate class FakeMatrixTimeline( initialTimelineItems: List = emptyList(), initialPaginationState: MatrixTimeline.PaginationState = MatrixTimeline.PaginationState(canBackPaginate = true, isBackPaginating = false) ) : MatrixTimeline { - private val paginationState: MutableStateFlow = MutableStateFlow(initialPaginationState) - private val timelineItems: MutableStateFlow> = MutableStateFlow(initialTimelineItems) + private val _paginationState: MutableStateFlow = MutableStateFlow(initialPaginationState) + private val _timelineItems: MutableStateFlow> = MutableStateFlow(initialTimelineItems) var sendReadReceiptCount = 0 private set fun updatePaginationState(update: (MatrixTimeline.PaginationState.() -> MatrixTimeline.PaginationState)) { - paginationState.value = update(paginationState.value) + _paginationState.getAndUpdate(update) } fun updateTimelineItems(update: (items: List) -> List) { - timelineItems.value = update(timelineItems.value) + _timelineItems.getAndUpdate(update) } - override fun paginationState(): StateFlow { - return paginationState - } + override val paginationState: StateFlow = _paginationState - override fun timelineItems(): Flow> { - return timelineItems - } + override val timelineItems: Flow> = _timelineItems override suspend fun paginateBackwards(requestSize: Int, untilNumberOfItems: Int): Result { updatePaginationState { diff --git a/samples/minimal/src/main/kotlin/io/element/android/samples/minimal/RoomListScreen.kt b/samples/minimal/src/main/kotlin/io/element/android/samples/minimal/RoomListScreen.kt index c5b3911687..f9cdf0f3fd 100644 --- a/samples/minimal/src/main/kotlin/io/element/android/samples/minimal/RoomListScreen.kt +++ b/samples/minimal/src/main/kotlin/io/element/android/samples/minimal/RoomListScreen.kt @@ -82,11 +82,7 @@ class RoomListScreen( withContext(coroutineDispatchers.io) { matrixClient.getRoom(roomId)!!.use { room -> room.open() - val timeline = room.timeline - timeline.apply { - // TODO This doesn't work reliably as initialize is asynchronous, and the timeline can't be used until it's finished - paginateBackwards(20, 50) - } + room.timeline.paginateBackwards(20, 50) } } }