From ad8573242720913f8a3c082e035f290f780cdea3 Mon Sep 17 00:00:00 2001 From: ganfra Date: Thu, 8 Aug 2024 14:51:49 +0200 Subject: [PATCH] Pinned events : minor code quality changes --- .../banner/PinnedMessagesBannerPresenter.kt | 18 +++++++----------- .../pinned/banner/PinnedMessagesBannerState.kt | 10 +++++----- .../PinnedMessagesBannerStateProvider.kt | 4 ++-- .../PinnedMessagesBannerPresenterTest.kt | 8 ++++---- 4 files changed, 18 insertions(+), 22 deletions(-) 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 578e38eaa9..13c45e0092 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 @@ -53,7 +53,7 @@ class PinnedMessagesBannerPresenter @Inject constructor( @Composable override fun present(): PinnedMessagesBannerState { val isFeatureEnabled = isFeatureEnabled() - val knownPinnedMessagesCount by remember { + val expectedPinnedMessagesCount by remember { room.roomInfoFlow.map { roomInfo -> roomInfo.pinnedEventIds.size } }.collectAsState(initial = 0) @@ -77,11 +77,7 @@ class PinnedMessagesBannerPresenter @Inject constructor( fun handleEvent(event: PinnedMessagesBannerEvents) { when (event) { is PinnedMessagesBannerEvents.MoveToNextPinned -> { - if (currentPinnedMessageIndex > 0) { - currentPinnedMessageIndex-- - } else { - currentPinnedMessageIndex = pinnedItems.value.size - 1 - } + currentPinnedMessageIndex = (currentPinnedMessageIndex - 1).mod(pinnedItems.value.size) } } } @@ -89,7 +85,7 @@ class PinnedMessagesBannerPresenter @Inject constructor( return pinnedMessagesBannerState( isFeatureEnabled = isFeatureEnabled, hasTimelineFailed = hasTimelineFailedToLoad, - realPinnedMessagesCount = knownPinnedMessagesCount, + expectedPinnedMessagesCount = expectedPinnedMessagesCount, pinnedItems = pinnedItems.value, currentPinnedMessageIndex = currentPinnedMessageIndex, eventSink = ::handleEvent @@ -100,7 +96,7 @@ class PinnedMessagesBannerPresenter @Inject constructor( private fun pinnedMessagesBannerState( isFeatureEnabled: Boolean, hasTimelineFailed: Boolean, - realPinnedMessagesCount: Int, + expectedPinnedMessagesCount: Int, pinnedItems: ImmutableList, currentPinnedMessageIndex: Int, eventSink: (PinnedMessagesBannerEvents) -> Unit @@ -112,11 +108,11 @@ class PinnedMessagesBannerPresenter @Inject constructor( currentPinnedMessage != null -> PinnedMessagesBannerState.Loaded( currentPinnedMessage = currentPinnedMessage, currentPinnedMessageIndex = currentPinnedMessageIndex, - knownPinnedMessagesCount = pinnedItems.size, + loadedPinnedMessagesCount = pinnedItems.size, eventSink = eventSink ) - realPinnedMessagesCount == 0 -> PinnedMessagesBannerState.Hidden - else -> PinnedMessagesBannerState.Loading(realPinnedMessagesCount = realPinnedMessagesCount) + expectedPinnedMessagesCount == 0 -> PinnedMessagesBannerState.Hidden + else -> PinnedMessagesBannerState.Loading(expectedPinnedMessagesCount = expectedPinnedMessagesCount) } } diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/pinned/banner/PinnedMessagesBannerState.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/pinned/banner/PinnedMessagesBannerState.kt index c06003cc7c..a686ada33c 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/pinned/banner/PinnedMessagesBannerState.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/pinned/banner/PinnedMessagesBannerState.kt @@ -27,23 +27,23 @@ import io.element.android.libraries.ui.strings.CommonStrings sealed interface PinnedMessagesBannerState { data object Hidden : PinnedMessagesBannerState sealed interface Visible : PinnedMessagesBannerState - data class Loading(val realPinnedMessagesCount: Int) : Visible + data class Loading(val expectedPinnedMessagesCount: Int) : Visible data class Loaded( val currentPinnedMessage: PinnedMessagesBannerItem, val currentPinnedMessageIndex: Int, - val knownPinnedMessagesCount: Int, + val loadedPinnedMessagesCount: Int, val eventSink: (PinnedMessagesBannerEvents) -> Unit ) : Visible fun pinnedMessagesCount() = when (this) { is Hidden -> 0 - is Loading -> realPinnedMessagesCount - is Loaded -> knownPinnedMessagesCount + is Loading -> expectedPinnedMessagesCount + is Loaded -> loadedPinnedMessagesCount } fun currentPinnedMessageIndex() = when (this) { is Hidden -> 0 - is Loading -> realPinnedMessagesCount - 1 + is Loading -> expectedPinnedMessagesCount - 1 is Loaded -> currentPinnedMessageIndex } diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/pinned/banner/PinnedMessagesBannerStateProvider.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/pinned/banner/PinnedMessagesBannerStateProvider.kt index bdcab879fe..e9d0a27c16 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/pinned/banner/PinnedMessagesBannerStateProvider.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/pinned/banner/PinnedMessagesBannerStateProvider.kt @@ -42,7 +42,7 @@ internal fun aHiddenPinnedMessagesBannerState() = PinnedMessagesBannerState.Hidd internal fun aLoadingPinnedMessagesBannerState( knownPinnedMessagesCount: Int = 4 ) = PinnedMessagesBannerState.Loading( - realPinnedMessagesCount = knownPinnedMessagesCount + expectedPinnedMessagesCount = knownPinnedMessagesCount ) internal fun aLoadedPinnedMessagesBannerState( @@ -56,6 +56,6 @@ internal fun aLoadedPinnedMessagesBannerState( ) = PinnedMessagesBannerState.Loaded( currentPinnedMessage = currentPinnedMessage, currentPinnedMessageIndex = currentPinnedMessageIndex, - knownPinnedMessagesCount = knownPinnedMessagesCount, + loadedPinnedMessagesCount = knownPinnedMessagesCount, eventSink = eventSink ) 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 fbbf40b7f0..1ae4729a40 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 @@ -99,7 +99,7 @@ class PinnedMessagesBannerPresenterTest { skipItems(2) val loadedState = awaitItem() as PinnedMessagesBannerState.Loaded assertThat(loadedState.currentPinnedMessageIndex).isEqualTo(0) - assertThat(loadedState.knownPinnedMessagesCount).isEqualTo(1) + assertThat(loadedState.loadedPinnedMessagesCount).isEqualTo(1) assertThat(loadedState.currentPinnedMessage.formatted.text).isEqualTo(messageContent.toString()) } } @@ -139,7 +139,7 @@ class PinnedMessagesBannerPresenterTest { awaitItem().also { loadedState -> loadedState as PinnedMessagesBannerState.Loaded assertThat(loadedState.currentPinnedMessageIndex).isEqualTo(1) - assertThat(loadedState.knownPinnedMessagesCount).isEqualTo(2) + assertThat(loadedState.loadedPinnedMessagesCount).isEqualTo(2) assertThat(loadedState.currentPinnedMessage.formatted.text).isEqualTo(messageContent2.toString()) loadedState.eventSink(PinnedMessagesBannerEvents.MoveToNextPinned) } @@ -147,7 +147,7 @@ class PinnedMessagesBannerPresenterTest { awaitItem().also { loadedState -> loadedState as PinnedMessagesBannerState.Loaded assertThat(loadedState.currentPinnedMessageIndex).isEqualTo(0) - assertThat(loadedState.knownPinnedMessagesCount).isEqualTo(2) + assertThat(loadedState.loadedPinnedMessagesCount).isEqualTo(2) assertThat(loadedState.currentPinnedMessage.formatted.text).isEqualTo(messageContent1.toString()) loadedState.eventSink(PinnedMessagesBannerEvents.MoveToNextPinned) } @@ -155,7 +155,7 @@ class PinnedMessagesBannerPresenterTest { awaitItem().also { loadedState -> loadedState as PinnedMessagesBannerState.Loaded assertThat(loadedState.currentPinnedMessageIndex).isEqualTo(1) - assertThat(loadedState.knownPinnedMessagesCount).isEqualTo(2) + assertThat(loadedState.loadedPinnedMessagesCount).isEqualTo(2) assertThat(loadedState.currentPinnedMessage.formatted.text).isEqualTo(messageContent2.toString()) } }