From 331413e8b4f856921b0c07f37e92e3e9981e2751 Mon Sep 17 00:00:00 2001 From: ganfra Date: Wed, 7 Aug 2024 19:00:41 +0200 Subject: [PATCH] Pinned events: make sure state is preserved --- .../pinned/IsPinnedMessagesFeatureEnabled.kt | 54 +++++++++++++++++++ .../banner/PinnedMessagesBannerPresenter.kt | 44 ++++++++------- .../PinnedMessagesBannerPresenterTest.kt | 9 +--- 3 files changed, 76 insertions(+), 31 deletions(-) create mode 100644 features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/pinned/IsPinnedMessagesFeatureEnabled.kt diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/pinned/IsPinnedMessagesFeatureEnabled.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/pinned/IsPinnedMessagesFeatureEnabled.kt new file mode 100644 index 0000000000..5ef5e2c793 --- /dev/null +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/pinned/IsPinnedMessagesFeatureEnabled.kt @@ -0,0 +1,54 @@ +/* + * Copyright (c) 2024 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.element.android.features.messages.impl.pinned + +import androidx.compose.runtime.Composable +import androidx.compose.runtime.LaunchedEffect +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.saveable.rememberSaveable +import androidx.compose.runtime.setValue +import com.squareup.anvil.annotations.ContributesBinding +import io.element.android.libraries.di.AppScope +import io.element.android.libraries.featureflag.api.FeatureFlagService +import io.element.android.libraries.featureflag.api.FeatureFlags +import kotlinx.coroutines.flow.launchIn +import kotlinx.coroutines.flow.onEach +import javax.inject.Inject + +fun interface IsPinnedMessagesFeatureEnabled { + @Composable + operator fun invoke(): Boolean +} + +@ContributesBinding(AppScope::class) +class DefaultIsPinnedMessagesFeatureEnabled @Inject constructor( + private val featureFlagService: FeatureFlagService, +) : IsPinnedMessagesFeatureEnabled { + @Composable + override operator fun invoke(): Boolean { + var isFeatureEnabled by rememberSaveable { + mutableStateOf(false) + } + LaunchedEffect(Unit) { + featureFlagService.isFeatureEnabledFlow(FeatureFlags.PinnedEvents) + .onEach { isFeatureEnabled = it } + .launchIn(this) + } + return isFeatureEnabled + } +} diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/pinned/banner/PinnedMessagesBannerPresenter.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/pinned/banner/PinnedMessagesBannerPresenter.kt index ba094dfffe..578e38eaa9 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/pinned/banner/PinnedMessagesBannerPresenter.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/pinned/banner/PinnedMessagesBannerPresenter.kt @@ -26,10 +26,9 @@ import androidx.compose.runtime.remember import androidx.compose.runtime.rememberUpdatedState import androidx.compose.runtime.saveable.rememberSaveable import androidx.compose.runtime.setValue +import io.element.android.features.messages.impl.pinned.IsPinnedMessagesFeatureEnabled import io.element.android.features.networkmonitor.api.NetworkMonitor import io.element.android.libraries.architecture.Presenter -import io.element.android.libraries.featureflag.api.FeatureFlagService -import io.element.android.libraries.featureflag.api.FeatureFlags import io.element.android.libraries.matrix.api.room.MatrixRoom import kotlinx.collections.immutable.ImmutableList import kotlinx.collections.immutable.persistentListOf @@ -46,21 +45,20 @@ import kotlin.time.Duration.Companion.milliseconds class PinnedMessagesBannerPresenter @Inject constructor( private val room: MatrixRoom, private val itemFactory: PinnedMessagesBannerItemFactory, - private val featureFlagService: FeatureFlagService, + private val isFeatureEnabled: IsPinnedMessagesFeatureEnabled, private val networkMonitor: NetworkMonitor, ) : Presenter { + private val pinnedItems = mutableStateOf>(persistentListOf()) + @Composable override fun present(): PinnedMessagesBannerState { - val isFeatureEnabled by featureFlagService.isFeatureEnabledFlow(FeatureFlags.PinnedEvents).collectAsState(initial = false) - var hasTimelineFailedToLoad by rememberSaveable { mutableStateOf(false) } - var currentPinnedMessageIndex by rememberSaveable { mutableIntStateOf(-1) } + val isFeatureEnabled = isFeatureEnabled() val knownPinnedMessagesCount by remember { room.roomInfoFlow.map { roomInfo -> roomInfo.pinnedEventIds.size } }.collectAsState(initial = 0) - var pinnedItems by remember { - mutableStateOf>(persistentListOf()) - } + var hasTimelineFailedToLoad by rememberSaveable { mutableStateOf(false) } + var currentPinnedMessageIndex by rememberSaveable { mutableIntStateOf(-1) } PinnedMessagesBannerItemsEffect( isFeatureEnabled = isFeatureEnabled, @@ -69,7 +67,7 @@ class PinnedMessagesBannerPresenter @Inject constructor( if (currentPinnedMessageIndex >= pinnedMessageCount || currentPinnedMessageIndex < 0) { currentPinnedMessageIndex = pinnedMessageCount - 1 } - pinnedItems = newItems + pinnedItems.value = newItems }, onTimelineFail = { hasTimelineFailed -> hasTimelineFailedToLoad = hasTimelineFailed @@ -82,7 +80,7 @@ class PinnedMessagesBannerPresenter @Inject constructor( if (currentPinnedMessageIndex > 0) { currentPinnedMessageIndex-- } else { - currentPinnedMessageIndex = pinnedItems.size - 1 + currentPinnedMessageIndex = pinnedItems.value.size - 1 } } } @@ -92,7 +90,7 @@ class PinnedMessagesBannerPresenter @Inject constructor( isFeatureEnabled = isFeatureEnabled, hasTimelineFailed = hasTimelineFailedToLoad, realPinnedMessagesCount = knownPinnedMessagesCount, - pinnedItems = pinnedItems, + pinnedItems = pinnedItems.value, currentPinnedMessageIndex = currentPinnedMessageIndex, eventSink = ::handleEvent ) @@ -111,16 +109,14 @@ class PinnedMessagesBannerPresenter @Inject constructor( return when { !isFeatureEnabled -> PinnedMessagesBannerState.Hidden hasTimelineFailed -> PinnedMessagesBannerState.Hidden + currentPinnedMessage != null -> PinnedMessagesBannerState.Loaded( + currentPinnedMessage = currentPinnedMessage, + currentPinnedMessageIndex = currentPinnedMessageIndex, + knownPinnedMessagesCount = pinnedItems.size, + eventSink = eventSink + ) realPinnedMessagesCount == 0 -> PinnedMessagesBannerState.Hidden - currentPinnedMessage == null -> PinnedMessagesBannerState.Loading(realPinnedMessagesCount = realPinnedMessagesCount) - else -> { - PinnedMessagesBannerState.Loaded( - currentPinnedMessage = currentPinnedMessage, - currentPinnedMessageIndex = currentPinnedMessageIndex, - knownPinnedMessagesCount = pinnedItems.size, - eventSink = eventSink - ) - } + else -> PinnedMessagesBannerState.Loading(realPinnedMessagesCount = realPinnedMessagesCount) } } @@ -136,8 +132,10 @@ class PinnedMessagesBannerPresenter @Inject constructor( val networkStatus by networkMonitor.connectivity.collectAsState() LaunchedEffect(isFeatureEnabled, networkStatus) { - if (!isFeatureEnabled) return@LaunchedEffect - + if (!isFeatureEnabled) { + updatedOnItemsChange(persistentListOf()) + return@LaunchedEffect + } val pinnedEventsTimeline = room.pinnedEventsTimeline() .onFailure { updatedOnTimelineFail(true) } .onSuccess { updatedOnTimelineFail(false) } diff --git a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/pinned/banner/PinnedMessagesBannerPresenterTest.kt b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/pinned/banner/PinnedMessagesBannerPresenterTest.kt index 6e78b2c446..fbbf40b7f0 100644 --- a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/pinned/banner/PinnedMessagesBannerPresenterTest.kt +++ b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/pinned/banner/PinnedMessagesBannerPresenterTest.kt @@ -20,8 +20,6 @@ import com.google.common.truth.Truth.assertThat import io.element.android.features.networkmonitor.api.NetworkMonitor import io.element.android.features.networkmonitor.test.FakeNetworkMonitor import io.element.android.libraries.eventformatter.test.FakePinnedMessagesBannerFormatter -import io.element.android.libraries.featureflag.api.FeatureFlags -import io.element.android.libraries.featureflag.test.FakeFeatureFlagService import io.element.android.libraries.matrix.api.room.MatrixRoom import io.element.android.libraries.matrix.api.timeline.MatrixTimelineItem import io.element.android.libraries.matrix.test.AN_EVENT_ID @@ -195,15 +193,10 @@ class PinnedMessagesBannerPresenterTest { networkMonitor: NetworkMonitor = FakeNetworkMonitor(), isFeatureEnabled: Boolean = true, ): PinnedMessagesBannerPresenter { - val featureFlagService = FakeFeatureFlagService( - initialState = mapOf( - FeatureFlags.PinnedEvents.key to isFeatureEnabled - ) - ) return PinnedMessagesBannerPresenter( room = room, itemFactory = itemFactory, - featureFlagService = featureFlagService, + isFeatureEnabled = { isFeatureEnabled }, networkMonitor = networkMonitor, ) }