Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update message action list #4056

Merged
merged 8 commits into from
Dec 18, 2024
Merged

Update message action list #4056

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
df400a5
Replace ic_developer_options.xml by the icon from Compound.
bmarty Dec 17, 2024
9d34275
Remove unused ic_groups.xml
bmarty Dec 17, 2024
dddef79
Change icon of the remove caption action
bmarty Dec 17, 2024
293da98
Reorder items in message action list.
bmarty Dec 17, 2024
57595d6
Make TimelineItemAction an enum class.
bmarty Dec 17, 2024
2221420
Add tests on TimelineItemActionComparator
bmarty Dec 17, 2024
a0e0d25
Add EditPoll action and fix tests.
bmarty Dec 17, 2024
cb7e1b6
Update screenshots
ElementBot Dec 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion ...sages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesPresenter.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 2 additions & 0 deletions ...c/main/kotlin/io/element/android/features/messages/impl/actionlist/ActionListPresenter.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
1 change: 1 addition & 0 deletions ...in/kotlin/io/element/android/features/messages/impl/actionlist/ActionListStateProvider.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,7 @@ fun aTimelineItemActionList(
fun aTimelineItemPollActionList(): ImmutableList<TimelineItemAction> {
return setOf(
TimelineItemAction.EndPoll,
TimelineItemAction.EditPoll,
TimelineItemAction.Reply,
TimelineItemAction.Pin,
TimelineItemAction.CopyLink,
Expand Down
38 changes: 19 additions & 19 deletions ...n/kotlin/io/element/android/features/messages/impl/actionlist/model/TimelineItemAction.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}
14 changes: 9 additions & 5 deletions ...o/element/android/features/messages/impl/actionlist/model/TimelineItemActionComparator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
6 changes: 3 additions & 3 deletions ...droid/features/messages/impl/pinned/list/PinnedMessagesListTimelineActionPostProcessor.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}
2 changes: 1 addition & 1 deletion ...s/impl/src/test/kotlin/io/element/android/features/messages/impl/MessagesPresenterTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
Expand Down
54 changes: 27 additions & 27 deletions ...st/kotlin/io/element/android/features/messages/impl/actionlist/ActionListPresenterTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,8 +179,8 @@ class ActionListPresenterTest {
actions = persistentListOf(
TimelineItemAction.Reply,
TimelineItemAction.Forward,
TimelineItemAction.Pin,
TimelineItemAction.CopyLink,
TimelineItemAction.Pin,
TimelineItemAction.CopyText,
TimelineItemAction.ViewSource,
TimelineItemAction.ReportContent,
Expand Down Expand Up @@ -225,8 +225,8 @@ class ActionListPresenterTest {
actions = persistentListOf(
TimelineItemAction.ReplyInThread,
TimelineItemAction.Forward,
TimelineItemAction.Pin,
TimelineItemAction.CopyLink,
TimelineItemAction.Pin,
TimelineItemAction.CopyText,
TimelineItemAction.ViewSource,
TimelineItemAction.ReportContent,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -320,8 +320,8 @@ class ActionListPresenterTest {
actions = persistentListOf(
TimelineItemAction.Reply,
TimelineItemAction.Forward,
TimelineItemAction.Pin,
TimelineItemAction.CopyLink,
TimelineItemAction.Pin,
TimelineItemAction.CopyText,
TimelineItemAction.ViewSource,
TimelineItemAction.ReportContent,
Expand Down Expand Up @@ -368,8 +368,8 @@ class ActionListPresenterTest {
actions = persistentListOf(
TimelineItemAction.Reply,
TimelineItemAction.Forward,
TimelineItemAction.Pin,
TimelineItemAction.CopyLink,
TimelineItemAction.Pin,
TimelineItemAction.CopyText,
TimelineItemAction.ViewSource,
TimelineItemAction.ReportContent,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
)
Expand Down Expand Up @@ -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,
)
Expand Down Expand Up @@ -612,8 +612,8 @@ class ActionListPresenterTest {
TimelineItemAction.Forward,
// Not here
// TimelineItemAction.AddCaption,
TimelineItemAction.Pin,
TimelineItemAction.CopyLink,
TimelineItemAction.Pin,
TimelineItemAction.ViewSource,
TimelineItemAction.Redact,
)
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -711,8 +711,8 @@ class ActionListPresenterTest {
actions = persistentListOf(
TimelineItemAction.Reply,
TimelineItemAction.Forward,
TimelineItemAction.Pin,
TimelineItemAction.CopyLink,
TimelineItemAction.Pin,
TimelineItemAction.CopyCaption,
TimelineItemAction.ViewSource,
TimelineItemAction.ReportContent,
Expand Down Expand Up @@ -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,
)
Expand Down Expand Up @@ -878,8 +878,8 @@ class ActionListPresenterTest {
actions = persistentListOf(
TimelineItemAction.Reply,
TimelineItemAction.Forward,
TimelineItemAction.CopyLink,
TimelineItemAction.Edit,
TimelineItemAction.CopyLink,
TimelineItemAction.CopyText,
TimelineItemAction.ViewSource,
TimelineItemAction.Redact,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -1072,9 +1072,9 @@ class ActionListPresenterTest {
actions = persistentListOf(
TimelineItemAction.EndPoll,
TimelineItemAction.Reply,
TimelineItemAction.Pin,
TimelineItemAction.EditPoll,
TimelineItemAction.CopyLink,
TimelineItemAction.Edit,
TimelineItemAction.Pin,
TimelineItemAction.Redact,
)
)
Expand Down Expand Up @@ -1116,8 +1116,8 @@ class ActionListPresenterTest {
actions = persistentListOf(
TimelineItemAction.EndPoll,
TimelineItemAction.Reply,
TimelineItemAction.Pin,
TimelineItemAction.CopyLink,
TimelineItemAction.Pin,
TimelineItemAction.Redact,
)
)
Expand Down Expand Up @@ -1158,8 +1158,8 @@ class ActionListPresenterTest {
verifiedUserSendFailure = VerifiedUserSendFailure.None,
actions = persistentListOf(
TimelineItemAction.Reply,
TimelineItemAction.Pin,
TimelineItemAction.CopyLink,
TimelineItemAction.Pin,
TimelineItemAction.Redact,
)
)
Expand Down Expand Up @@ -1203,8 +1203,8 @@ class ActionListPresenterTest {
actions = persistentListOf(
TimelineItemAction.Reply,
TimelineItemAction.Forward,
TimelineItemAction.Pin,
TimelineItemAction.CopyLink,
TimelineItemAction.Pin,
TimelineItemAction.Redact,
)
)
Expand Down
28 changes: 28 additions & 0 deletions ...ement/android/features/messages/impl/actionlist/model/TimelineItemActionComparatorTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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" })
}
}
}
Loading
Loading