From 78f0ce0d27fd6ab2931e20dfd669b8e6b3b4aea8 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 26 Dec 2023 14:45:08 +0100 Subject: [PATCH] Refacto: move `displayEmojiReactions` to `ActionListState.Target.Success` --- .../impl/actionlist/ActionListPresenter.kt | 18 ++-- .../impl/actionlist/ActionListState.kt | 2 +- .../actionlist/ActionListStateProvider.kt | 12 ++- .../impl/actionlist/ActionListView.kt | 2 +- .../model/event/TimelineItemEventContent.kt | 1 + .../actionlist/ActionListPresenterTest.kt | 82 +++++++++++-------- 6 files changed, 65 insertions(+), 52 deletions(-) diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/actionlist/ActionListPresenter.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/actionlist/ActionListPresenter.kt index d936ea9af8..645c903158 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/actionlist/ActionListPresenter.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/actionlist/ActionListPresenter.kt @@ -19,7 +19,6 @@ package io.element.android.features.messages.impl.actionlist import androidx.compose.runtime.Composable import androidx.compose.runtime.MutableState import androidx.compose.runtime.collectAsState -import androidx.compose.runtime.derivedStateOf import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember @@ -53,13 +52,6 @@ class ActionListPresenter @Inject constructor( val isDeveloperModeEnabled by preferencesStore.isDeveloperModeEnabledFlow().collectAsState(initial = false) - val displayEmojiReactions by remember { - derivedStateOf { - val event = (target.value as? ActionListState.Target.Success)?.event - event?.isRemote == true && event.content.canReact() - } - } - fun handleEvents(event: ActionListEvents) { when (event) { ActionListEvents.Clear -> target.value = ActionListState.Target.None @@ -75,7 +67,6 @@ class ActionListPresenter @Inject constructor( return ActionListState( target = target.value, - displayEmojiReactions = displayEmojiReactions, eventSink = { handleEvents(it) } ) } @@ -178,8 +169,13 @@ class ActionListPresenter @Inject constructor( } } } - if (actions.isNotEmpty()) { - target.value = ActionListState.Target.Success(timelineItem, actions.toImmutableList()) + val displayEmojiReactions = timelineItem.isRemote && timelineItem.content.canReact() + if (actions.isNotEmpty() || displayEmojiReactions) { + target.value = ActionListState.Target.Success( + event = timelineItem, + displayEmojiReactions = displayEmojiReactions, + actions = actions.toImmutableList() + ) } else { target.value = ActionListState.Target.None } diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/actionlist/ActionListState.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/actionlist/ActionListState.kt index a8fbf81486..5566f949d0 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/actionlist/ActionListState.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/actionlist/ActionListState.kt @@ -24,7 +24,6 @@ import kotlinx.collections.immutable.ImmutableList @Immutable data class ActionListState( val target: Target, - val displayEmojiReactions: Boolean, val eventSink: (ActionListEvents) -> Unit, ) { sealed interface Target { @@ -32,6 +31,7 @@ data class ActionListState( data class Loading(val event: TimelineItem.Event) : Target data class Success( val event: TimelineItem.Event, + val displayEmojiReactions: Boolean, val actions: ImmutableList, ) : Target } diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/actionlist/ActionListStateProvider.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/actionlist/ActionListStateProvider.kt index 3ea99aaede..e33f54de3e 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/actionlist/ActionListStateProvider.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/actionlist/ActionListStateProvider.kt @@ -42,6 +42,7 @@ open class ActionListStateProvider : PreviewParameterProvider { event = aTimelineItemEvent().copy( reactionsState = reactionsState ), + displayEmojiReactions = true, actions = aTimelineItemActionList(), ) ), @@ -50,6 +51,7 @@ open class ActionListStateProvider : PreviewParameterProvider { event = aTimelineItemEvent(content = aTimelineItemImageContent()).copy( reactionsState = reactionsState ), + displayEmojiReactions = true, actions = aTimelineItemActionList(), ) ), @@ -58,6 +60,7 @@ open class ActionListStateProvider : PreviewParameterProvider { event = aTimelineItemEvent(content = aTimelineItemVideoContent()).copy( reactionsState = reactionsState ), + displayEmojiReactions = true, actions = aTimelineItemActionList(), ) ), @@ -66,6 +69,7 @@ open class ActionListStateProvider : PreviewParameterProvider { event = aTimelineItemEvent(content = aTimelineItemFileContent()).copy( reactionsState = reactionsState ), + displayEmojiReactions = true, actions = aTimelineItemActionList(), ) ), @@ -74,6 +78,7 @@ open class ActionListStateProvider : PreviewParameterProvider { event = aTimelineItemEvent(content = aTimelineItemAudioContent()).copy( reactionsState = reactionsState ), + displayEmojiReactions = true, actions = aTimelineItemActionList(), ) ), @@ -82,6 +87,7 @@ open class ActionListStateProvider : PreviewParameterProvider { event = aTimelineItemEvent(content = aTimelineItemVoiceContent()).copy( reactionsState = reactionsState ), + displayEmojiReactions = true, actions = aTimelineItemActionList(), ) ), @@ -90,6 +96,7 @@ open class ActionListStateProvider : PreviewParameterProvider { event = aTimelineItemEvent(content = aTimelineItemLocationContent()).copy( reactionsState = reactionsState ), + displayEmojiReactions = true, actions = aTimelineItemActionList(), ) ), @@ -98,18 +105,18 @@ open class ActionListStateProvider : PreviewParameterProvider { event = aTimelineItemEvent(content = aTimelineItemLocationContent()).copy( reactionsState = reactionsState ), + displayEmojiReactions = false, actions = aTimelineItemActionList(), ), - displayEmojiReactions = false, ), anActionListState().copy( target = ActionListState.Target.Success( event = aTimelineItemEvent(content = aTimelineItemPollContent()).copy( reactionsState = reactionsState ), + displayEmojiReactions = false, actions = aTimelineItemPollActionList(), ), - displayEmojiReactions = false, ), ) } @@ -117,7 +124,6 @@ open class ActionListStateProvider : PreviewParameterProvider { fun anActionListState() = ActionListState( target = ActionListState.Target.None, - displayEmojiReactions = true, eventSink = {} ) diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/actionlist/ActionListView.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/actionlist/ActionListView.kt index c7b72ce527..93736933fd 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/actionlist/ActionListView.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/actionlist/ActionListView.kt @@ -179,7 +179,7 @@ private fun SheetContent( HorizontalDivider() } } - if (state.displayEmojiReactions) { + if (target.displayEmojiReactions) { item { EmojiReactionsRow( highlightedEmojis = target.event.reactionsState.highlightedKeys, diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/event/TimelineItemEventContent.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/event/TimelineItemEventContent.kt index feb3246e54..f459e27d88 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/event/TimelineItemEventContent.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/event/TimelineItemEventContent.kt @@ -47,6 +47,7 @@ fun TimelineItemEventContent.canBeRepliedTo(): Boolean = /** * Return true if user can react (i.e. send a reaction) on the event content. + * This does not take into account the power level of the user. */ fun TimelineItemEventContent.canReact(): Boolean = when (this) { diff --git a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/actionlist/ActionListPresenterTest.kt b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/actionlist/ActionListPresenterTest.kt index 3c8dcf6e85..1fcd873d5a 100644 --- a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/actionlist/ActionListPresenterTest.kt +++ b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/actionlist/ActionListPresenterTest.kt @@ -68,8 +68,9 @@ class ActionListPresenterTest { val successState = awaitItem() assertThat(successState.target).isEqualTo( ActionListState.Target.Success( - messageEvent, - persistentListOf( + event = messageEvent, + displayEmojiReactions = false, + actions = persistentListOf( TimelineItemAction.ViewSource, ) ) @@ -93,8 +94,9 @@ class ActionListPresenterTest { val successState = awaitItem() assertThat(successState.target).isEqualTo( ActionListState.Target.Success( - messageEvent, - persistentListOf( + event = messageEvent, + displayEmojiReactions = false, + actions = persistentListOf( TimelineItemAction.ViewSource, ) ) @@ -121,8 +123,9 @@ class ActionListPresenterTest { val successState = awaitItem() assertThat(successState.target).isEqualTo( ActionListState.Target.Success( - messageEvent, - persistentListOf( + event = messageEvent, + displayEmojiReactions = true, + actions = persistentListOf( TimelineItemAction.Reply, TimelineItemAction.Forward, TimelineItemAction.Copy, @@ -153,8 +156,9 @@ class ActionListPresenterTest { val successState = awaitItem() assertThat(successState.target).isEqualTo( ActionListState.Target.Success( - messageEvent, - persistentListOf( + event = messageEvent, + displayEmojiReactions = true, + actions = persistentListOf( TimelineItemAction.Forward, TimelineItemAction.Copy, TimelineItemAction.ViewSource, @@ -182,8 +186,9 @@ class ActionListPresenterTest { val successState = awaitItem() assertThat(successState.target).isEqualTo( ActionListState.Target.Success( - messageEvent, - persistentListOf( + event = messageEvent, + displayEmojiReactions = true, + actions = persistentListOf( TimelineItemAction.Reply, TimelineItemAction.Forward, TimelineItemAction.Copy, @@ -215,8 +220,9 @@ class ActionListPresenterTest { val successState = awaitItem() assertThat(successState.target).isEqualTo( ActionListState.Target.Success( - messageEvent, - persistentListOf( + event = messageEvent, + displayEmojiReactions = true, + actions = persistentListOf( TimelineItemAction.Reply, TimelineItemAction.Forward, TimelineItemAction.Edit, @@ -248,8 +254,9 @@ class ActionListPresenterTest { val successState = awaitItem() assertThat(successState.target).isEqualTo( ActionListState.Target.Success( - messageEvent, - persistentListOf( + event = messageEvent, + displayEmojiReactions = true, + actions = persistentListOf( TimelineItemAction.Reply, TimelineItemAction.Forward, TimelineItemAction.ViewSource, @@ -279,8 +286,9 @@ class ActionListPresenterTest { val successState = awaitItem() assertThat(successState.target).isEqualTo( ActionListState.Target.Success( - stateEvent, - persistentListOf( + event = stateEvent, + displayEmojiReactions = false, + actions = persistentListOf( TimelineItemAction.Copy, TimelineItemAction.ViewSource, ) @@ -308,8 +316,9 @@ class ActionListPresenterTest { val successState = awaitItem() assertThat(successState.target).isEqualTo( ActionListState.Target.Success( - stateEvent, - persistentListOf( + event = stateEvent, + displayEmojiReactions = false, + actions = persistentListOf( TimelineItemAction.Copy, ) ) @@ -336,8 +345,9 @@ class ActionListPresenterTest { val successState = awaitItem() assertThat(successState.target).isEqualTo( ActionListState.Target.Success( - messageEvent, - persistentListOf( + event = messageEvent, + displayEmojiReactions = true, + actions = persistentListOf( TimelineItemAction.Reply, TimelineItemAction.Forward, TimelineItemAction.Edit, @@ -373,7 +383,6 @@ class ActionListPresenterTest { initialState.eventSink.invoke(ActionListEvents.ComputeForMessage(redactedEvent, canRedact = false, canSendMessage = true)) awaitItem().run { assertThat(target).isEqualTo(ActionListState.Target.None) - assertThat(displayEmojiReactions).isFalse() } } } @@ -395,15 +404,15 @@ class ActionListPresenterTest { val successState = awaitItem() assertThat(successState.target).isEqualTo( ActionListState.Target.Success( - messageEvent, - persistentListOf( + event = messageEvent, + displayEmojiReactions = false, + actions = persistentListOf( TimelineItemAction.Edit, TimelineItemAction.Copy, TimelineItemAction.Redact, ) ) ) - assertThat(successState.displayEmojiReactions).isFalse() } } @@ -423,8 +432,9 @@ class ActionListPresenterTest { val successState = awaitItem() assertThat(successState.target).isEqualTo( ActionListState.Target.Success( - messageEvent, - persistentListOf( + event = messageEvent, + displayEmojiReactions = true, + actions = persistentListOf( TimelineItemAction.Reply, TimelineItemAction.Edit, TimelineItemAction.EndPoll, @@ -432,9 +442,9 @@ class ActionListPresenterTest { ) ) ) - assertThat(successState.displayEmojiReactions).isTrue() } } + @Test fun `present - compute for non-editable poll message`() = runTest { val presenter = createActionListPresenter(isDeveloperModeEnabled = false) @@ -451,15 +461,15 @@ class ActionListPresenterTest { val successState = awaitItem() assertThat(successState.target).isEqualTo( ActionListState.Target.Success( - messageEvent, - persistentListOf( + event = messageEvent, + displayEmojiReactions = true, + actions = persistentListOf( TimelineItemAction.Reply, TimelineItemAction.EndPoll, TimelineItemAction.Redact, ) ) ) - assertThat(successState.displayEmojiReactions).isTrue() } } @@ -479,14 +489,14 @@ class ActionListPresenterTest { val successState = awaitItem() assertThat(successState.target).isEqualTo( ActionListState.Target.Success( - messageEvent, - persistentListOf( + event = messageEvent, + displayEmojiReactions = true, + actions = persistentListOf( TimelineItemAction.Reply, TimelineItemAction.Redact, ) ) ) - assertThat(successState.displayEmojiReactions).isTrue() } } @@ -505,15 +515,15 @@ class ActionListPresenterTest { val successState = awaitItem() assertThat(successState.target).isEqualTo( ActionListState.Target.Success( - messageEvent, - persistentListOf( + event = messageEvent, + displayEmojiReactions = true, + actions = persistentListOf( TimelineItemAction.Reply, TimelineItemAction.Forward, TimelineItemAction.Redact, ) ) ) - assertThat(successState.displayEmojiReactions).isTrue() } } }