Merge pull request #6684 from thundernest/message_details_appearance

Small changes to message details appearance
2023-02-20 14:58:37 +01:00 · 2023-02-20 14:58:37 +01:00 · 7c928c4e8d
commit 7c928c4e8d
parent cdd69cc685 19ac95c9b2
8 changed files with 101 additions and 76 deletions
--- a/app/ui/legacy/src/main/java/com/fsck/k9/ui/messagedetails/KoinModule.kt
+++ b/app/ui/legacy/src/main/java/com/fsck/k9/ui/messagedetails/KoinModule.kt
@ -20,5 +20,5 @@ val messageDetailsUiModule = module {
    factory { ContactSettingsProvider() }
    factory { AddToContactsLauncher() }
    factory { ShowContactLauncher() }
-    factory { createMessageDetailsParticipantFormatter(contactNameProvider = get()) }
+    factory { createMessageDetailsParticipantFormatter(contactNameProvider = get(), resources = get()) }
 }
--- a/app/ui/legacy/src/main/java/com/fsck/k9/ui/messagedetails/MessageDetailsFragment.kt
+++ b/app/ui/legacy/src/main/java/com/fsck/k9/ui/messagedetails/MessageDetailsFragment.kt
@ -13,6 +13,7 @@ import androidx.annotation.StringRes
 import androidx.appcompat.widget.PopupMenu
 import androidx.core.content.ContextCompat
 import androidx.core.os.bundleOf
+import androidx.core.view.MenuCompat
 import androidx.core.view.isVisible
 import androidx.fragment.app.setFragmentResult
 import androidx.recyclerview.widget.RecyclerView
@ -153,7 +154,6 @@ class MessageDetailsFragment : ToolbarBottomSheetDialogFragment() {
            addEventHook(cryptoStatusClickEventHook)
            addEventHook(participantClickEventHook)
            addEventHook(addToContactsClickEventHook)
-            addEventHook(composeClickEventHook)
            addEventHook(overflowClickEventHook)
        }

@ -242,34 +242,6 @@ class MessageDetailsFragment : ToolbarBottomSheetDialogFragment() {
        addToContactsLauncher.launch(context = requireContext(), name = address.personal, email = address.address)
    }

-    private val composeClickEventHook = object : ClickEventHook<ParticipantItem>() {
-        override fun onBind(viewHolder: RecyclerView.ViewHolder): View? {
-            return if (viewHolder is ParticipantItem.ViewHolder) {
-                viewHolder.menuCompose
-            } else {
-                null
-            }
-        }
-
-        override fun onClick(v: View, position: Int, fastAdapter: FastAdapter<ParticipantItem>, item: ParticipantItem) {
-            val address = item.participant.address
-            composeMessageToAddress(address)
-        }
-    }
-
-    private fun composeMessageToAddress(address: Address) {
-        // TODO: Use the identity this message was sent to as sender identity
-
-        val intent = Intent(context, MessageCompose::class.java).apply {
-            action = Intent.ACTION_SEND
-            putExtra(Intent.EXTRA_EMAIL, arrayOf(address.toString()))
-            putExtra(MessageCompose.EXTRA_ACCOUNT, messageReference.accountUuid)
-        }
-
-        dismiss()
-        requireContext().startActivity(intent)
-    }
-
    private val overflowClickEventHook = object : ClickEventHook<ParticipantItem>() {
        override fun onBind(viewHolder: RecyclerView.ViewHolder): View? {
            return if (viewHolder is ParticipantItem.ViewHolder) {
@ -289,8 +261,11 @@ class MessageDetailsFragment : ToolbarBottomSheetDialogFragment() {
            inflate(R.menu.participant_overflow_menu)
        }

+        val menu = popupMenu.menu
+        MenuCompat.setGroupDividerEnabled(menu, true)
+
        if (participant.address.personal == null) {
-            popupMenu.menu.findItem(R.id.copy_name_and_email_address).isVisible = false
+            menu.findItem(R.id.copy_name_and_email_address).isVisible = false
        }

        popupMenu.setOnMenuItemClickListener { item: MenuItem ->
@ -303,11 +278,25 @@ class MessageDetailsFragment : ToolbarBottomSheetDialogFragment() {

    private fun onOverflowMenuItemClick(itemId: Int, participant: Participant) {
        when (itemId) {
+            R.id.compose_to -> composeMessageToAddress(participant.address)
            R.id.copy_email_address -> viewModel.onCopyEmailAddressToClipboard(participant)
            R.id.copy_name_and_email_address -> viewModel.onCopyNameAndEmailAddressToClipboard(participant)
        }
    }

+    private fun composeMessageToAddress(address: Address) {
+        // TODO: Use the identity this message was sent to as sender identity
+
+        val intent = Intent(context, MessageCompose::class.java).apply {
+            action = Intent.ACTION_SEND
+            putExtra(Intent.EXTRA_EMAIL, arrayOf(address.toString()))
+            putExtra(MessageCompose.EXTRA_ACCOUNT, messageReference.accountUuid)
+        }
+
+        dismiss()
+        requireContext().startActivity(intent)
+    }
+
    private fun showCryptoKeys(pendingIntent: PendingIntent) {
        requireActivity().startIntentSender(pendingIntent.intentSender, null, 0, 0, 0)
    }
--- a/app/ui/legacy/src/main/java/com/fsck/k9/ui/messagedetails/MessageDetailsParticipantFormatter.kt
+++ b/app/ui/legacy/src/main/java/com/fsck/k9/ui/messagedetails/MessageDetailsParticipantFormatter.kt
@ -1,12 +1,15 @@
 package com.fsck.k9.ui.messagedetails

+import android.content.res.Resources
 import android.text.Spannable.SPAN_EXCLUSIVE_EXCLUSIVE
 import android.text.SpannableString
 import android.text.style.ForegroundColorSpan
 import com.fsck.k9.Account
+import com.fsck.k9.Identity
 import com.fsck.k9.K9
 import com.fsck.k9.helper.ContactNameProvider
 import com.fsck.k9.mail.Address
+import com.fsck.k9.ui.R

 /**
 * Get the display name for a participant to be shown in the message details screen.
@ -19,11 +22,12 @@ internal class RealMessageDetailsParticipantFormatter(
    private val contactNameProvider: ContactNameProvider,
    private val showContactNames: Boolean,
    private val contactNameColor: Int?,
+    private val meText: String,
 ) : MessageDetailsParticipantFormatter {
    override fun getDisplayName(address: Address, account: Account): CharSequence? {
-        val identityDisplayName = account.findIdentity(address)?.name
-        if (identityDisplayName != null) {
-            return identityDisplayName
+        val identity = account.findIdentity(address)
+        if (identity != null) {
+            return getIdentityName(identity, account)
        }

        return if (showContactNames) {
@ -33,6 +37,14 @@ internal class RealMessageDetailsParticipantFormatter(
        }
    }

+    private fun getIdentityName(identity: Identity, account: Account): String {
+        return if (account.identities.size == 1) {
+            meText
+        } else {
+            identity.description ?: identity.name ?: meText
+        }
+    }
+
    private fun getContactNameOrNull(address: Address): CharSequence? {
        val contactName = contactNameProvider.getNameForAddress(address.address) ?: return null

@ -48,10 +60,12 @@ internal class RealMessageDetailsParticipantFormatter(

 internal fun createMessageDetailsParticipantFormatter(
    contactNameProvider: ContactNameProvider,
+    resources: Resources,
 ): MessageDetailsParticipantFormatter {
    return RealMessageDetailsParticipantFormatter(
        contactNameProvider = contactNameProvider,
        showContactNames = K9.isShowContactName,
        contactNameColor = if (K9.isChangeContactNameColor) K9.contactNameColor else null,
+        meText = resources.getString(R.string.message_view_me_text),
    )
 }
--- a/app/ui/legacy/src/main/java/com/fsck/k9/ui/messagedetails/ParticipantItem.kt
+++ b/app/ui/legacy/src/main/java/com/fsck/k9/ui/messagedetails/ParticipantItem.kt
@ -22,7 +22,6 @@ internal class ParticipantItem(

    class ViewHolder(view: View) : FastAdapter.ViewHolder<ParticipantItem>(view) {
        val menuAddContact: View = view.findViewById(R.id.menu_add_contact)
-        val menuCompose: View = view.findViewById(R.id.menu_compose)
        val menuOverflow: View = view.findViewById(R.id.menu_overflow)

        private val contactPicture: ImageView = view.findViewById(R.id.contact_picture)
@ -32,7 +31,6 @@ internal class ParticipantItem(

        init {
            TooltipCompat.setTooltipText(menuAddContact, menuAddContact.contentDescription)
-            TooltipCompat.setTooltipText(menuCompose, menuCompose.contentDescription)
            TooltipCompat.setTooltipText(menuOverflow, menuOverflow.contentDescription)
        }

@ -41,11 +39,11 @@ internal class ParticipantItem(

            if (participant.displayName != null) {
                name.text = participant.displayName
-                email.text = participant.emailAddress
            } else {
-                name.text = participant.emailAddress
-                email.isVisible = false
+                name.isVisible = false
            }
+            email.text = participant.emailAddress
+
            menuAddContact.isVisible = !participant.isInContacts

            if (item.showContactsPicture) {
@ -62,8 +60,8 @@ internal class ParticipantItem(

        override fun unbindView(item: ParticipantItem) {
            name.text = null
+            name.isVisible = true
            email.text = null
-            email.isVisible = true
            contactPicture.isVisible = true
            itemView.background = originalBackground
            itemView.isClickable = true
--- a/app/ui/legacy/src/main/res/layout/message_details_participant_item.xml
+++ b/app/ui/legacy/src/main/res/layout/message_details_participant_item.xml
@ -14,19 +14,14 @@
        android:layout_width="40dp"
        android:layout_height="40dp"
        android:layout_marginStart="16dp"
-        android:layout_marginTop="16dp"
        android:src="@drawable/ic_contact_picture"
        app:layout_constraintStart_toStartOf="parent"
-        app:layout_constraintTop_toTopOf="parent" />
+        app:layout_constraintTop_toTopOf="@+id/top_guideline" />

    <TextView
        android:id="@+id/name"
        android:layout_width="wrap_content"
        android:layout_height="wrap_content"
-        android:layout_marginStart="16dp"
-        android:layout_marginTop="16dp"
-        android:ellipsize="end"
-        android:gravity="center_vertical"
        android:textAppearance="@style/TextAppearance.MaterialComponents.Body1"
        android:textColor="?android:attr/textColorPrimary"
        app:layout_constrainedWidth="true"
@ -34,8 +29,8 @@
        app:layout_constraintEnd_toStartOf="@+id/menu_add_contact"
        app:layout_constraintHorizontal_bias="0.0"
        app:layout_constraintHorizontal_chainStyle="packed"
-        app:layout_constraintStart_toEndOf="@id/contact_picture"
-        app:layout_constraintTop_toTopOf="parent"
+        app:layout_constraintStart_toEndOf="@id/start_guideline"
+        app:layout_constraintTop_toTopOf="@+id/top_guideline"
        app:layout_constraintVertical_chainStyle="packed"
        app:layout_goneMarginBottom="12dp"
        tools:text="Alice" />
@ -45,17 +40,14 @@
        android:layout_width="wrap_content"
        android:layout_height="wrap_content"
        android:layout_marginEnd="4dp"
-        android:layout_marginBottom="16dp"
-        android:ellipsize="end"
-        android:gravity="center_vertical"
        android:textAppearance="@style/TextAppearance.MaterialComponents.Body2"
        android:textColor="?android:attr/textColorSecondary"
        app:layout_constrainedWidth="true"
-        app:layout_constraintBottom_toBottomOf="parent"
+        app:layout_constraintBottom_toTopOf="@+id/bottom_guideline"
        app:layout_constraintEnd_toStartOf="@+id/menu_add_contact"
        app:layout_constraintHorizontal_bias="0.0"
        app:layout_constraintHorizontal_chainStyle="packed"
-        app:layout_constraintStart_toStartOf="@+id/name"
+        app:layout_constraintStart_toEndOf="@id/start_guideline"
        app:layout_constraintTop_toBottomOf="@+id/name"
        tools:text="alice@domain.example" />

@ -68,22 +60,9 @@
        android:contentDescription="@string/action_add_to_contacts"
        android:focusable="true"
        android:paddingHorizontal="12dp"
-        app:layout_constraintEnd_toStartOf="@+id/menu_compose"
-        app:layout_constraintTop_toTopOf="@+id/menu_compose"
-        app:srcCompat="?attr/messageDetailsAddContactIcon" />
-
-    <ImageView
-        android:id="@+id/menu_compose"
-        android:layout_width="48dp"
-        android:layout_height="72dp"
-        android:background="?attr/controlBackground"
-        android:clickable="true"
-        android:contentDescription="@string/compose_action"
-        android:focusable="true"
-        android:paddingHorizontal="12dp"
        app:layout_constraintEnd_toStartOf="@id/menu_overflow"
        app:layout_constraintTop_toTopOf="@+id/menu_overflow"
-        app:srcCompat="@drawable/ic_envelope" />
+        app:srcCompat="?attr/messageDetailsAddContactIcon" />

    <ImageView
        android:id="@+id/menu_overflow"
@ -101,4 +80,25 @@
        app:layout_constraintVertical_bias="0.0"
        app:srcCompat="@drawable/dots_vertical" />

+    <androidx.constraintlayout.widget.Guideline
+        android:id="@+id/start_guideline"
+        android:layout_width="wrap_content"
+        android:layout_height="wrap_content"
+        android:orientation="vertical"
+        app:layout_constraintGuide_begin="72dp" />
+
+    <androidx.constraintlayout.widget.Guideline
+        android:id="@+id/top_guideline"
+        android:layout_width="wrap_content"
+        android:layout_height="wrap_content"
+        android:orientation="horizontal"
+        app:layout_constraintGuide_begin="16dp" />
+
+    <androidx.constraintlayout.widget.Guideline
+        android:id="@+id/bottom_guideline"
+        android:layout_width="wrap_content"
+        android:layout_height="wrap_content"
+        android:orientation="horizontal"
+        app:layout_constraintGuide_end="16dp" />
+
 </androidx.constraintlayout.widget.ConstraintLayout>
--- a/app/ui/legacy/src/main/res/menu/participant_overflow_menu.xml
+++ b/app/ui/legacy/src/main/res/menu/participant_overflow_menu.xml
@ -1,9 +1,16 @@
 <?xml version="1.0" encoding="utf-8"?>
 <menu xmlns:android="http://schemas.android.com/apk/res/android">
+    <group android:id="@+id/group_main">
+        <item
+            android:id="@+id/compose_to"
+            android:title="@string/action_compose_message_to" />
+    </group>
+    <group android:id="@+id/group_other">
        <item
            android:id="@+id/copy_email_address"
            android:title="@string/action_copy_email_address" />
        <item
            android:id="@+id/copy_name_and_email_address"
            android:title="@string/action_copy_name_and_email_address" />
+    </group>
 </menu>
--- a/app/ui/legacy/src/main/res/values/strings.xml
+++ b/app/ui/legacy/src/main/res/values/strings.xml
@ -1322,6 +1322,8 @@ You can keep this message and use it as a backup for your secret key. If you wan

    <!-- Name of the action to add a person to contacts. Usually displayed in a menu or in a tooltip when long-pressing the associated icon. -->
    <string name="action_add_to_contacts">Add to contacts</string>
+    <!-- Name of the action to compose a new message to a particular address. Usually displayed in a menu or in a tooltip when long-pressing the associated icon. -->
+    <string name="action_compose_message_to">Compose message to</string>
    <!-- Name of the action to copy an email address to the clipboard. Usually displayed in a menu or in a tooltip when long-pressing the associated icon. -->
    <string name="action_copy_email_address">Copy email address</string>
    <!-- Name of the action to copy a name and email address to the clipboard. Usually displayed in a menu or in a tooltip when long-pressing the associated icon. -->
--- a/app/ui/legacy/src/test/java/com/fsck/k9/ui/messagedetails/MessageDetailsParticipantFormatterTest.kt
+++ b/app/ui/legacy/src/test/java/com/fsck/k9/ui/messagedetails/MessageDetailsParticipantFormatterTest.kt
@ -14,6 +14,7 @@ import org.junit.Test

 private const val IDENTITY_NAME = "Alice"
 private const val IDENTITY_ADDRESS = "me@domain.example"
+private const val ME_TEXT = "me"

 class MessageDetailsParticipantFormatterTest : RobolectricTest() {
    private val contactNameProvider = object : ContactNameProvider {
@ -33,7 +34,19 @@ class MessageDetailsParticipantFormatterTest : RobolectricTest() {
    private val participantFormatter = createParticipantFormatter()

    @Test
-    fun `identity address`() {
+    fun `identity address with single identity`() {
+        val displayName = participantFormatter.getDisplayName(Address(IDENTITY_ADDRESS, "irrelevant"), account)
+
+        assertThat(displayName).isEqualTo(ME_TEXT)
+    }
+
+    @Test
+    fun `identity address with multiple identities`() {
+        val account = Account("uuid").apply {
+            identities += Identity(name = IDENTITY_NAME, email = IDENTITY_ADDRESS)
+            identities += Identity(name = "Another identity", email = "irrelevant@domain.example")
+        }
+
        val displayName = participantFormatter.getDisplayName(Address(IDENTITY_ADDRESS, "irrelevant"), account)

        assertThat(displayName).isEqualTo(IDENTITY_NAME)
@ -47,18 +60,19 @@ class MessageDetailsParticipantFormatterTest : RobolectricTest() {

        val displayName = participantFormatter.getDisplayName(Address(IDENTITY_ADDRESS, "Bob"), account)

-        assertThat(displayName).isEqualTo("Bob")
+        assertThat(displayName).isEqualTo(ME_TEXT)
    }

    @Test
    fun `identity and address without a display name`() {
        val account = Account("uuid").apply {
            identities += Identity(name = null, email = IDENTITY_ADDRESS)
+            identities += Identity(name = "Another identity", email = "irrelevant@domain.example")
        }

        val displayName = participantFormatter.getDisplayName(Address(IDENTITY_ADDRESS), account)

-        assertThat(displayName).isNull()
+        assertThat(displayName).isEqualTo(ME_TEXT)
    }

    @Test
@ -118,6 +132,7 @@ class MessageDetailsParticipantFormatterTest : RobolectricTest() {
            contactNameProvider = contactNameProvider,
            showContactNames = showContactNames,
            contactNameColor = contactNameColor,
+            meText = ME_TEXT,
        )
    }
 }