Merge pull request #4056 from element-hq/feature/bma/messageActionList

Update message action list

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesPresenter.kt

@@ -274,7 +274,8 @@ class MessagesPresenter @AssistedInject constructor(
             TimelineItemAction.CopyCaption -> handleCopyCaption(targetEvent)
             TimelineItemAction.CopyLink -> handleCopyLink(targetEvent)
             TimelineItemAction.Redact -> handleActionRedact(targetEvent)
-            TimelineItemAction.Edit -> handleActionEdit(targetEvent, composerState, enableTextFormatting)
+            TimelineItemAction.Edit,
+            TimelineItemAction.EditPoll -> handleActionEdit(targetEvent, composerState, enableTextFormatting)
             TimelineItemAction.AddCaption -> handleActionAddCaption(targetEvent, composerState)
             TimelineItemAction.EditCaption -> handleActionEditCaption(targetEvent, composerState)
             TimelineItemAction.RemoveCaption -> handleRemoveCaption(targetEvent)

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/actionlist/ActionListPresenter.kt

@@ -178,6 +178,8 @@ class DefaultActionListPresenter @AssistedInject constructor(
                         add(TimelineItemAction.EditCaption)
                         add(TimelineItemAction.RemoveCaption)
                     }
+                } else if (timelineItem.content is TimelineItemPollContent) {
+                    add(TimelineItemAction.EditPoll)
                 } else {
                     add(TimelineItemAction.Edit)
                 }

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/actionlist/ActionListStateProvider.kt

@@ -203,6 +203,7 @@ fun aTimelineItemActionList(
 fun aTimelineItemPollActionList(): ImmutableList<TimelineItemAction> {
     return setOf(
         TimelineItemAction.EndPoll,
+        TimelineItemAction.EditPoll,
         TimelineItemAction.Reply,
         TimelineItemAction.Pin,
         TimelineItemAction.CopyLink,

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/actionlist/model/TimelineItemAction.kt

@@ -11,30 +11,30 @@ import androidx.annotation.DrawableRes
 import androidx.annotation.StringRes
 import androidx.compose.runtime.Immutable
 import io.element.android.libraries.designsystem.icons.CompoundDrawables
-import io.element.android.libraries.designsystem.utils.CommonDrawables
 import io.element.android.libraries.ui.strings.CommonStrings
 
 @Immutable
-sealed class TimelineItemAction(
+enum class TimelineItemAction(
     @StringRes val titleRes: Int,
     @DrawableRes val icon: Int,
     val destructive: Boolean = false
 ) {
-    data object ViewInTimeline : TimelineItemAction(CommonStrings.action_view_in_timeline, CompoundDrawables.ic_compound_visibility_on)
-    data object Forward : TimelineItemAction(CommonStrings.action_forward, CompoundDrawables.ic_compound_forward)
-    data object CopyText : TimelineItemAction(CommonStrings.action_copy_text, CompoundDrawables.ic_compound_copy)
-    data object CopyCaption : TimelineItemAction(CommonStrings.action_copy_caption, CompoundDrawables.ic_compound_copy)
-    data object CopyLink : TimelineItemAction(CommonStrings.action_copy_link_to_message, CompoundDrawables.ic_compound_link)
-    data object Redact : TimelineItemAction(CommonStrings.action_remove, CompoundDrawables.ic_compound_delete, destructive = true)
-    data object Reply : TimelineItemAction(CommonStrings.action_reply, CompoundDrawables.ic_compound_reply)
-    data object ReplyInThread : TimelineItemAction(CommonStrings.action_reply_in_thread, CompoundDrawables.ic_compound_reply)
-    data object Edit : TimelineItemAction(CommonStrings.action_edit, CompoundDrawables.ic_compound_edit)
-    data object EditCaption : TimelineItemAction(CommonStrings.action_edit_caption, CompoundDrawables.ic_compound_edit)
-    data object AddCaption : TimelineItemAction(CommonStrings.action_add_caption, CompoundDrawables.ic_compound_edit)
-    data object RemoveCaption : TimelineItemAction(CommonStrings.action_remove_caption, CompoundDrawables.ic_compound_delete, destructive = true)
-    data object ViewSource : TimelineItemAction(CommonStrings.action_view_source, CommonDrawables.ic_developer_options)
-    data object ReportContent : TimelineItemAction(CommonStrings.action_report_content, CompoundDrawables.ic_compound_chat_problem, destructive = true)
-    data object EndPoll : TimelineItemAction(CommonStrings.action_end_poll, CompoundDrawables.ic_compound_polls_end)
-    data object Pin : TimelineItemAction(CommonStrings.action_pin, CompoundDrawables.ic_compound_pin)
-    data object Unpin : TimelineItemAction(CommonStrings.action_unpin, CompoundDrawables.ic_compound_unpin)
+    ViewInTimeline(CommonStrings.action_view_in_timeline, CompoundDrawables.ic_compound_visibility_on),
+    Forward(CommonStrings.action_forward, CompoundDrawables.ic_compound_forward),
+    CopyText(CommonStrings.action_copy_text, CompoundDrawables.ic_compound_copy),
+    CopyCaption(CommonStrings.action_copy_caption, CompoundDrawables.ic_compound_copy),
+    CopyLink(CommonStrings.action_copy_link_to_message, CompoundDrawables.ic_compound_link),
+    Redact(CommonStrings.action_remove, CompoundDrawables.ic_compound_delete, destructive = true),
+    Reply(CommonStrings.action_reply, CompoundDrawables.ic_compound_reply),
+    ReplyInThread(CommonStrings.action_reply_in_thread, CompoundDrawables.ic_compound_reply),
+    Edit(CommonStrings.action_edit, CompoundDrawables.ic_compound_edit),
+    EditPoll(CommonStrings.action_edit_poll, CompoundDrawables.ic_compound_edit),
+    EditCaption(CommonStrings.action_edit_caption, CompoundDrawables.ic_compound_edit),
+    AddCaption(CommonStrings.action_add_caption, CompoundDrawables.ic_compound_edit),
+    RemoveCaption(CommonStrings.action_remove_caption, CompoundDrawables.ic_compound_close, destructive = true),
+    ViewSource(CommonStrings.action_view_source, CompoundDrawables.ic_compound_code),
+    ReportContent(CommonStrings.action_report_content, CompoundDrawables.ic_compound_chat_problem, destructive = true),
+    EndPoll(CommonStrings.action_end_poll, CompoundDrawables.ic_compound_polls_end),
+    Pin(CommonStrings.action_pin, CompoundDrawables.ic_compound_pin),
+    Unpin(CommonStrings.action_unpin, CompoundDrawables.ic_compound_unpin),
 }

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/actionlist/model/TimelineItemActionComparator.kt

@@ -7,21 +7,25 @@
 
 package io.element.android.features.messages.impl.actionlist.model
 
+import androidx.annotation.VisibleForTesting
+
 class TimelineItemActionComparator : Comparator<TimelineItemAction> {
     // See order in https://www.figma.com/design/ux3tYoZV9WghC7hHT9Fhk0/Compound-iOS-Components?node-id=2946-2392
-    private val orderedList = listOf(
+    @VisibleForTesting
+    val orderedList = listOf(
         TimelineItemAction.EndPoll,
         TimelineItemAction.ViewInTimeline,
         TimelineItemAction.Reply,
         TimelineItemAction.ReplyInThread,
         TimelineItemAction.Forward,
-        TimelineItemAction.Pin,
-        TimelineItemAction.Unpin,
-        TimelineItemAction.CopyLink,
         TimelineItemAction.Edit,
-        TimelineItemAction.CopyText,
+        TimelineItemAction.EditPoll,
         TimelineItemAction.AddCaption,
         TimelineItemAction.EditCaption,
+        TimelineItemAction.CopyLink,
+        TimelineItemAction.Pin,
+        TimelineItemAction.Unpin,
+        TimelineItemAction.CopyText,
         TimelineItemAction.CopyCaption,
         TimelineItemAction.RemoveCaption,
         TimelineItemAction.ViewSource,

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/pinned/list/PinnedMessagesListTimelineActionPostProcessor.kt

@@ -14,9 +14,9 @@ class PinnedMessagesListTimelineActionPostProcessor : TimelineItemActionPostProc
     override fun process(actions: List<TimelineItemAction>): List<TimelineItemAction> {
         return buildList {
             add(TimelineItemAction.ViewInTimeline)
-            actions.firstOrNull { it is TimelineItemAction.Unpin }?.let(::add)
-            actions.firstOrNull { it is TimelineItemAction.Forward }?.let(::add)
-            actions.firstOrNull { it is TimelineItemAction.ViewSource }?.let(::add)
+            actions.firstOrNull { it == TimelineItemAction.Unpin }?.let(::add)
+            actions.firstOrNull { it == TimelineItemAction.Forward }?.let(::add)
+            actions.firstOrNull { it == TimelineItemAction.ViewSource }?.let(::add)
         }
     }
 }

features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/MessagesPresenterTest.kt

@@ -467,7 +467,7 @@ class MessagesPresenterTest {
             presenter.present()
         }.test {
             val initialState = awaitItem()
-            initialState.eventSink(MessagesEvents.HandleAction(TimelineItemAction.Edit, aMessageEvent(content = aTimelineItemPollContent())))
+            initialState.eventSink(MessagesEvents.HandleAction(TimelineItemAction.EditPoll, aMessageEvent(content = aTimelineItemPollContent())))
             awaitItem()
             onEditPollClickLambda.assertions().isCalledOnce().with(value(AN_EVENT_ID))
         }

features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/actionlist/ActionListPresenterTest.kt

@@ -179,8 +179,8 @@ class ActionListPresenterTest {
                     actions = persistentListOf(
                         TimelineItemAction.Reply,
                         TimelineItemAction.Forward,
-                        TimelineItemAction.Pin,
                         TimelineItemAction.CopyLink,
+                        TimelineItemAction.Pin,
                         TimelineItemAction.CopyText,
                         TimelineItemAction.ViewSource,
                         TimelineItemAction.ReportContent,
@@ -225,8 +225,8 @@ class ActionListPresenterTest {
                     actions = persistentListOf(
                         TimelineItemAction.ReplyInThread,
                         TimelineItemAction.Forward,
-                        TimelineItemAction.Pin,
                         TimelineItemAction.CopyLink,
+                        TimelineItemAction.Pin,
                         TimelineItemAction.CopyText,
                         TimelineItemAction.ViewSource,
                         TimelineItemAction.ReportContent,
@@ -273,8 +273,8 @@ class ActionListPresenterTest {
                     verifiedUserSendFailure = VerifiedUserSendFailure.None,
                     actions = persistentListOf(
                         TimelineItemAction.Forward,
-                        TimelineItemAction.Pin,
                         TimelineItemAction.CopyLink,
+                        TimelineItemAction.Pin,
                         TimelineItemAction.CopyText,
                         TimelineItemAction.ViewSource,
                         TimelineItemAction.ReportContent,
@@ -320,8 +320,8 @@ class ActionListPresenterTest {
                     actions = persistentListOf(
                         TimelineItemAction.Reply,
                         TimelineItemAction.Forward,
-                        TimelineItemAction.Pin,
                         TimelineItemAction.CopyLink,
+                        TimelineItemAction.Pin,
                         TimelineItemAction.CopyText,
                         TimelineItemAction.ViewSource,
                         TimelineItemAction.ReportContent,
@@ -368,8 +368,8 @@ class ActionListPresenterTest {
                     actions = persistentListOf(
                         TimelineItemAction.Reply,
                         TimelineItemAction.Forward,
-                        TimelineItemAction.Pin,
                         TimelineItemAction.CopyLink,
+                        TimelineItemAction.Pin,
                         TimelineItemAction.CopyText,
                         TimelineItemAction.ViewSource,
                         TimelineItemAction.ReportContent,
@@ -417,9 +417,9 @@ class ActionListPresenterTest {
                     actions = persistentListOf(
                         TimelineItemAction.Reply,
                         TimelineItemAction.Forward,
-                        TimelineItemAction.Pin,
-                        TimelineItemAction.CopyLink,
                         TimelineItemAction.Edit,
+                        TimelineItemAction.CopyLink,
+                        TimelineItemAction.Pin,
                         TimelineItemAction.CopyText,
                         TimelineItemAction.ViewSource,
                         TimelineItemAction.Redact,
@@ -463,9 +463,9 @@ class ActionListPresenterTest {
                     actions = persistentListOf(
                         TimelineItemAction.ReplyInThread,
                         TimelineItemAction.Forward,
-                        TimelineItemAction.Pin,
-                        TimelineItemAction.CopyLink,
                         TimelineItemAction.Edit,
+                        TimelineItemAction.CopyLink,
+                        TimelineItemAction.Pin,
                         TimelineItemAction.CopyText,
                         TimelineItemAction.ViewSource,
                         TimelineItemAction.Redact,
@@ -512,9 +512,9 @@ class ActionListPresenterTest {
                     actions = persistentListOf(
                         TimelineItemAction.Reply,
                         TimelineItemAction.Forward,
-                        TimelineItemAction.Pin,
-                        TimelineItemAction.CopyLink,
                         TimelineItemAction.Edit,
+                        TimelineItemAction.CopyLink,
+                        TimelineItemAction.Pin,
                         TimelineItemAction.CopyText,
                         TimelineItemAction.ViewSource,
                     )
@@ -559,9 +559,9 @@ class ActionListPresenterTest {
                     actions = persistentListOf(
                         TimelineItemAction.Reply,
                         TimelineItemAction.Forward,
-                        TimelineItemAction.Pin,
-                        TimelineItemAction.CopyLink,
                         TimelineItemAction.AddCaption,
+                        TimelineItemAction.CopyLink,
+                        TimelineItemAction.Pin,
                         TimelineItemAction.ViewSource,
                         TimelineItemAction.Redact,
                     )
@@ -612,8 +612,8 @@ class ActionListPresenterTest {
                         TimelineItemAction.Forward,
                         // Not here
                         // TimelineItemAction.AddCaption,
-                        TimelineItemAction.Pin,
                         TimelineItemAction.CopyLink,
+                        TimelineItemAction.Pin,
                         TimelineItemAction.ViewSource,
                         TimelineItemAction.Redact,
                     )
@@ -660,9 +660,9 @@ class ActionListPresenterTest {
                     actions = persistentListOf(
                         TimelineItemAction.Reply,
                         TimelineItemAction.Forward,
-                        TimelineItemAction.Pin,
-                        TimelineItemAction.CopyLink,
                         TimelineItemAction.EditCaption,
+                        TimelineItemAction.CopyLink,
+                        TimelineItemAction.Pin,
                         TimelineItemAction.CopyCaption,
                         TimelineItemAction.RemoveCaption,
                         TimelineItemAction.ViewSource,
@@ -711,8 +711,8 @@ class ActionListPresenterTest {
                     actions = persistentListOf(
                         TimelineItemAction.Reply,
                         TimelineItemAction.Forward,
-                        TimelineItemAction.Pin,
                         TimelineItemAction.CopyLink,
+                        TimelineItemAction.Pin,
                         TimelineItemAction.CopyCaption,
                         TimelineItemAction.ViewSource,
                         TimelineItemAction.ReportContent,
@@ -830,9 +830,9 @@ class ActionListPresenterTest {
                     actions = persistentListOf(
                         TimelineItemAction.Reply,
                         TimelineItemAction.Forward,
-                        TimelineItemAction.Pin,
-                        TimelineItemAction.CopyLink,
                         TimelineItemAction.Edit,
+                        TimelineItemAction.CopyLink,
+                        TimelineItemAction.Pin,
                         TimelineItemAction.CopyText,
                         TimelineItemAction.Redact,
                     )
@@ -878,8 +878,8 @@ class ActionListPresenterTest {
                     actions = persistentListOf(
                         TimelineItemAction.Reply,
                         TimelineItemAction.Forward,
-                        TimelineItemAction.CopyLink,
                         TimelineItemAction.Edit,
+                        TimelineItemAction.CopyLink,
                         TimelineItemAction.CopyText,
                         TimelineItemAction.ViewSource,
                         TimelineItemAction.Redact,
@@ -933,9 +933,9 @@ class ActionListPresenterTest {
                     actions = persistentListOf(
                         TimelineItemAction.Reply,
                         TimelineItemAction.Forward,
-                        TimelineItemAction.Unpin,
-                        TimelineItemAction.CopyLink,
                         TimelineItemAction.Edit,
+                        TimelineItemAction.CopyLink,
+                        TimelineItemAction.Unpin,
                         TimelineItemAction.CopyText,
                         TimelineItemAction.ViewSource,
                         TimelineItemAction.Redact,
@@ -1072,9 +1072,9 @@ class ActionListPresenterTest {
                     actions = persistentListOf(
                         TimelineItemAction.EndPoll,
                         TimelineItemAction.Reply,
-                        TimelineItemAction.Pin,
+                        TimelineItemAction.EditPoll,
                         TimelineItemAction.CopyLink,
-                        TimelineItemAction.Edit,
+                        TimelineItemAction.Pin,
                         TimelineItemAction.Redact,
                     )
                 )
@@ -1116,8 +1116,8 @@ class ActionListPresenterTest {
                     actions = persistentListOf(
                         TimelineItemAction.EndPoll,
                         TimelineItemAction.Reply,
-                        TimelineItemAction.Pin,
                         TimelineItemAction.CopyLink,
+                        TimelineItemAction.Pin,
                         TimelineItemAction.Redact,
                     )
                 )
@@ -1158,8 +1158,8 @@ class ActionListPresenterTest {
                     verifiedUserSendFailure = VerifiedUserSendFailure.None,
                     actions = persistentListOf(
                         TimelineItemAction.Reply,
-                        TimelineItemAction.Pin,
                         TimelineItemAction.CopyLink,
+                        TimelineItemAction.Pin,
                         TimelineItemAction.Redact,
                     )
                 )
@@ -1203,8 +1203,8 @@ class ActionListPresenterTest {
                     actions = persistentListOf(
                         TimelineItemAction.Reply,
                         TimelineItemAction.Forward,
-                        TimelineItemAction.Pin,
                         TimelineItemAction.CopyLink,
+                        TimelineItemAction.Pin,
                         TimelineItemAction.Redact,
                     )
                 )

features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/actionlist/model/TimelineItemActionComparatorTest.kt

@@ -0,0 +1,28 @@
+/*
+ * Copyright 2024 New Vector Ltd.
+ *
+ * SPDX-License-Identifier: AGPL-3.0-only
+ * Please see LICENSE in the repository root for full details.
+ */
+
+package io.element.android.features.messages.impl.actionlist.model
+
+import org.junit.Test
+
+class TimelineItemActionComparatorTest {
+    @Test
+    fun `check that the list in the comparator only contain each item once`() {
+        val sut = TimelineItemActionComparator()
+        sut.orderedList.forEach {
+            require(sut.orderedList.count { item -> item == it } == 1, { "Duplicate ${it::class.java}.$it" })
+        }
+    }
+
+    @Test
+    fun `check that the list in the comparator contains all the items`() {
+        val sut = TimelineItemActionComparator()
+        TimelineItemAction.entries.forEach {
+            require(it in sut.orderedList, { "Missing ${it::class.simpleName}.$it in orderedList" })
+        }
+    }
+}

features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/pinned/list/PinnedMessagesListTimelineActionPostProcessorTest.kt

@@ -27,24 +27,7 @@ class PinnedMessagesListTimelineActionPostProcessorTest {
     fun `ensure that some actions are kept and some other are filtered out`() {
         val sut = PinnedMessagesListTimelineActionPostProcessor()
         val result = sut.process(
-            listOf(
-                TimelineItemAction.Forward,
-                TimelineItemAction.CopyText,
-                TimelineItemAction.CopyCaption,
-                TimelineItemAction.CopyLink,
-                TimelineItemAction.Redact,
-                TimelineItemAction.Reply,
-                TimelineItemAction.ReplyInThread,
-                TimelineItemAction.Edit,
-                TimelineItemAction.EditCaption,
-                TimelineItemAction.AddCaption,
-                TimelineItemAction.RemoveCaption,
-                TimelineItemAction.ViewSource,
-                TimelineItemAction.ReportContent,
-                TimelineItemAction.EndPoll,
-                TimelineItemAction.Pin,
-                TimelineItemAction.Unpin,
-            )
+            TimelineItemAction.entries.toList()
         )
         assertThat(result).isEqualTo(
             listOf(

features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/root/PreferencesRootView.kt

@@ -25,6 +25,7 @@ import io.element.android.features.preferences.impl.user.UserPreferences
 import io.element.android.libraries.architecture.coverage.ExcludeFromCoverage
 import io.element.android.libraries.designsystem.components.list.ListItemContent
 import io.element.android.libraries.designsystem.components.preferences.PreferencePage
+import io.element.android.libraries.designsystem.icons.CompoundDrawables
 import io.element.android.libraries.designsystem.preview.ElementPreviewDark
 import io.element.android.libraries.designsystem.preview.ElementPreviewLight
 import io.element.android.libraries.designsystem.preview.PreviewWithLargeHeight
@@ -33,7 +34,6 @@ import io.element.android.libraries.designsystem.theme.components.IconSource
 import io.element.android.libraries.designsystem.theme.components.ListItem
 import io.element.android.libraries.designsystem.theme.components.ListItemStyle
 import io.element.android.libraries.designsystem.theme.components.Text
-import io.element.android.libraries.designsystem.utils.CommonDrawables
 import io.element.android.libraries.designsystem.utils.snackbar.SnackbarHost
 import io.element.android.libraries.designsystem.utils.snackbar.rememberSnackbarHostState
 import io.element.android.libraries.matrix.api.core.DeviceId
@@ -270,7 +270,7 @@ private fun ColumnScope.Footer(
 private fun DeveloperPreferencesView(onOpenDeveloperSettings: () -> Unit) {
     ListItem(
         headlineContent = { Text(stringResource(id = CommonStrings.common_developer_options)) },
-        leadingContent = ListItemContent.Icon(IconSource.Resource(CommonDrawables.ic_developer_options)),
+        leadingContent = ListItemContent.Icon(IconSource.Resource(CompoundDrawables.ic_compound_code)),
         onClick = onOpenDeveloperSettings
     )
 }