From 71c8fb11d961ec9028e0d18a730e327ec99a6d75 Mon Sep 17 00:00:00 2001 From: cketti Date: Wed, 15 Mar 2023 14:54:08 +0100 Subject: [PATCH 1/4] Message Details: Don't create new adapter every time the list is updated --- .../messagedetails/MessageDetailsFragment.kt | 54 ++++++++++--------- 1 file changed, 28 insertions(+), 26 deletions(-) diff --git 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 index a241585d0..a6f5d20c5 100644 --- 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 @@ -18,6 +18,7 @@ import androidx.core.view.isVisible import androidx.fragment.app.setFragmentResult import androidx.recyclerview.widget.RecyclerView import androidx.recyclerview.widget.RecyclerView.OnScrollListener +import app.k9mail.ui.utils.bottomsheet.ToolbarBottomSheetDialog import app.k9mail.ui.utils.bottomsheet.ToolbarBottomSheetDialogFragment import com.fsck.k9.activity.MessageCompose import com.fsck.k9.contacts.ContactPictureLoader @@ -44,6 +45,7 @@ class MessageDetailsFragment : ToolbarBottomSheetDialogFragment() { private val folderIconProvider: FolderIconProvider by inject { parametersOf(requireContext().theme) } private lateinit var messageReference: MessageReference + private val itemAdapter = ItemAdapter() // FIXME: Replace this with a mechanism that survives process death var cryptoResult: CryptoResultAnnotation? = null @@ -87,15 +89,7 @@ class MessageDetailsFragment : ToolbarBottomSheetDialogFragment() { val errorView = view.findViewById(R.id.message_details_error) val recyclerView = view.findViewById(R.id.message_details_list) - // Don't allow dragging down the bottom sheet (by dragging the toolbar) unless the list is scrolled all the way - // to the top. - recyclerView.addOnScrollListener( - object : OnScrollListener() { - override fun onScrolled(recyclerView: RecyclerView, dx: Int, dy: Int) { - dialog.behavior.isDraggable = !recyclerView.canScrollVertically(-1) - } - }, - ) + initializeRecyclerView(recyclerView, dialog) viewModel.uiEvents.observe(this) { event -> when (event) { @@ -121,18 +115,33 @@ class MessageDetailsFragment : ToolbarBottomSheetDialogFragment() { progressBar.isVisible = false errorView.isVisible = false recyclerView.isVisible = true - setMessageDetails(recyclerView, state.details, state.appearance) + setMessageDetails(state.details, state.appearance) } } } } - private fun setMessageDetails( - recyclerView: RecyclerView, - details: MessageDetailsUi, - appearance: MessageDetailsAppearance, - ) { - val itemAdapter = ItemAdapter().apply { + private fun initializeRecyclerView(recyclerView: RecyclerView, dialog: ToolbarBottomSheetDialog) { + // Don't allow dragging down the bottom sheet (by dragging the toolbar) unless the list is scrolled all the way + // to the top. + recyclerView.addOnScrollListener( + object : OnScrollListener() { + override fun onScrolled(recyclerView: RecyclerView, dx: Int, dy: Int) { + dialog.behavior.isDraggable = !recyclerView.canScrollVertically(-1) + } + }, + ) + + recyclerView.adapter = FastAdapter.with(itemAdapter).apply { + addEventHook(cryptoStatusClickEventHook) + addEventHook(participantClickEventHook) + addEventHook(addToContactsClickEventHook) + addEventHook(overflowClickEventHook) + } + } + + private fun setMessageDetails(details: MessageDetailsUi, appearance: MessageDetailsAppearance) { + val items = buildList { add(MessageDateItem(details.date ?: getString(R.string.message_details_missing_date))) if (details.cryptoDetails != null) { @@ -154,17 +163,10 @@ class MessageDetailsFragment : ToolbarBottomSheetDialogFragment() { } } - val adapter = FastAdapter.with(itemAdapter).apply { - addEventHook(cryptoStatusClickEventHook) - addEventHook(participantClickEventHook) - addEventHook(addToContactsClickEventHook) - addEventHook(overflowClickEventHook) - } - - recyclerView.adapter = adapter + itemAdapter.setNewList(items) } - private fun ItemAdapter.addParticipants( + private fun MutableList.addParticipants( participants: List, @StringRes title: Int, appearance: MessageDetailsAppearance, @@ -186,7 +188,7 @@ class MessageDetailsFragment : ToolbarBottomSheetDialogFragment() { } } - private fun ItemAdapter.addFolderName(folder: FolderInfoUi) { + private fun MutableList.addFolderName(folder: FolderInfoUi) { val folderNameItem = FolderNameItem( displayName = folder.displayName, iconResourceId = folderIconProvider.getFolderIcon(folder.type), From 5fa993392bea12670e8ccaf5ad7ed426d74c0503 Mon Sep 17 00:00:00 2001 From: cketti Date: Wed, 15 Mar 2023 15:20:35 +0100 Subject: [PATCH 2/4] Message Details: Use stable IDs for list items --- .../fsck/k9/ui/messagedetails/EmptyItem.kt | 25 ++++++++++++ .../messagedetails/MessageDetailsFragment.kt | 38 +++++++++++++------ .../main/res/values/message_details_ids.xml | 1 + 3 files changed, 52 insertions(+), 12 deletions(-) create mode 100644 app/ui/legacy/src/main/java/com/fsck/k9/ui/messagedetails/EmptyItem.kt diff --git a/app/ui/legacy/src/main/java/com/fsck/k9/ui/messagedetails/EmptyItem.kt b/app/ui/legacy/src/main/java/com/fsck/k9/ui/messagedetails/EmptyItem.kt new file mode 100644 index 000000000..30bcea9f6 --- /dev/null +++ b/app/ui/legacy/src/main/java/com/fsck/k9/ui/messagedetails/EmptyItem.kt @@ -0,0 +1,25 @@ +package com.fsck.k9.ui.messagedetails + +import android.content.Context +import android.view.View +import android.view.ViewGroup +import com.fsck.k9.ui.R +import com.mikepenz.fastadapter.FastAdapter +import com.mikepenz.fastadapter.items.AbstractItem + +internal class EmptyItem : AbstractItem() { + override val type: Int = R.id.message_details_empty + override val layoutRes = 0 + + override fun createView(ctx: Context, parent: ViewGroup?): View { + return View(ctx) + } + + override fun getViewHolder(v: View) = ViewHolder(v) + + class ViewHolder(view: View) : FastAdapter.ViewHolder(view) { + override fun bindView(item: EmptyItem, payloads: List) = Unit + + override fun unbindView(item: EmptyItem) = Unit + } +} diff --git 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 index a6f5d20c5..98672c2b3 100644 --- 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 @@ -144,9 +144,7 @@ class MessageDetailsFragment : ToolbarBottomSheetDialogFragment() { val items = buildList { add(MessageDateItem(details.date ?: getString(R.string.message_details_missing_date))) - if (details.cryptoDetails != null) { - add(CryptoStatusItem(details.cryptoDetails)) - } + addCryptoStatus(details) addParticipants(details.from, R.string.message_details_from_section_title, appearance) addParticipants(details.sender, R.string.message_details_sender_section_title, appearance) @@ -158,14 +156,26 @@ class MessageDetailsFragment : ToolbarBottomSheetDialogFragment() { addParticipants(details.cc, R.string.message_details_cc_section_title, appearance) addParticipants(details.bcc, R.string.message_details_bcc_section_title, appearance) - if (details.folder != null) { - addFolderName(details.folder) - } + addFolderName(details.folder) + } + + // Use list index as stable identifier. This means changes to the list may only update existing items or add + // new items to the end of the list. Use place holder items (EmptyItem) if necessary. + items.forEachIndexed { index, item -> + item.identifier = index.toLong() } itemAdapter.setNewList(items) } + private fun MutableList.addCryptoStatus(details: MessageDetailsUi) { + if (details.cryptoDetails != null) { + add(CryptoStatusItem(details.cryptoDetails)) + } else { + add(EmptyItem()) + } + } + private fun MutableList.addParticipants( participants: List, @StringRes title: Int, @@ -188,12 +198,16 @@ class MessageDetailsFragment : ToolbarBottomSheetDialogFragment() { } } - private fun MutableList.addFolderName(folder: FolderInfoUi) { - val folderNameItem = FolderNameItem( - displayName = folder.displayName, - iconResourceId = folderIconProvider.getFolderIcon(folder.type), - ) - add(folderNameItem) + private fun MutableList.addFolderName(folder: FolderInfoUi?) { + if (folder != null) { + val folderNameItem = FolderNameItem( + displayName = folder.displayName, + iconResourceId = folderIconProvider.getFolderIcon(folder.type), + ) + add(folderNameItem) + } else { + add(EmptyItem()) + } } private val cryptoStatusClickEventHook = object : ClickEventHook() { diff --git a/app/ui/legacy/src/main/res/values/message_details_ids.xml b/app/ui/legacy/src/main/res/values/message_details_ids.xml index bc2f6e35f..82ef7db6e 100644 --- a/app/ui/legacy/src/main/res/values/message_details_ids.xml +++ b/app/ui/legacy/src/main/res/values/message_details_ids.xml @@ -6,4 +6,5 @@ + From 4e639256e46d1798e5c1d087bf25879adce39288 Mon Sep 17 00:00:00 2001 From: cketti Date: Wed, 15 Mar 2023 15:41:46 +0100 Subject: [PATCH 3/4] Message Details: Update list when the fragment is resumed We want to update the list of participants when the associated contact has changed in the contacts database. Right now we don't listen to changes to the contacts database, but refresh the list when onResume() is called. This should catch the case where a user is adding a contact from the message details screen using one of the "add to contacts" buttons. --- .../messagedetails/MessageDetailsFragment.kt | 10 +++++- .../messagedetails/MessageDetailsViewModel.kt | 33 +++++++++++++++---- 2 files changed, 35 insertions(+), 8 deletions(-) diff --git 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 index 98672c2b3..eb3bbb5c7 100644 --- 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 @@ -99,7 +99,7 @@ class MessageDetailsFragment : ToolbarBottomSheetDialogFragment() { } } - viewModel.loadData(messageReference).observe(this) { state -> + viewModel.uiState.observe(this) { state -> when (state) { MessageDetailsState.Loading -> { progressBar.isVisible = true @@ -119,6 +119,14 @@ class MessageDetailsFragment : ToolbarBottomSheetDialogFragment() { } } } + + viewModel.initialize(messageReference) + } + + override fun onResume() { + super.onResume() + + viewModel.reload() } private fun initializeRecyclerView(recyclerView: RecyclerView, dialog: ToolbarBottomSheetDialog) { diff --git a/app/ui/legacy/src/main/java/com/fsck/k9/ui/messagedetails/MessageDetailsViewModel.kt b/app/ui/legacy/src/main/java/com/fsck/k9/ui/messagedetails/MessageDetailsViewModel.kt index b08bc103f..9b3b5faa4 100644 --- a/app/ui/legacy/src/main/java/com/fsck/k9/ui/messagedetails/MessageDetailsViewModel.kt +++ b/app/ui/legacy/src/main/java/com/fsck/k9/ui/messagedetails/MessageDetailsViewModel.kt @@ -4,6 +4,7 @@ import android.app.PendingIntent import android.content.res.Resources import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope +import app.k9mail.core.android.common.contact.CachingRepository import app.k9mail.core.android.common.contact.ContactPermissionResolver import app.k9mail.core.android.common.contact.ContactRepository import app.k9mail.core.common.mail.EmailAddress @@ -24,8 +25,8 @@ import java.text.DateFormat import java.util.Locale import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.channels.Channel +import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow -import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.receiveAsFlow import kotlinx.coroutines.launch @@ -42,15 +43,35 @@ internal class MessageDetailsViewModel( private val folderNameFormatter: FolderNameFormatter, ) : ViewModel() { private val dateFormat = DateFormat.getDateTimeInstance(DateFormat.LONG, DateFormat.MEDIUM, Locale.getDefault()) - private val uiState = MutableStateFlow(MessageDetailsState.Loading) - private val eventChannel = Channel() + private val internalUiState = MutableStateFlow(MessageDetailsState.Loading) + val uiState: Flow + get() = internalUiState + + private val eventChannel = Channel() val uiEvents = eventChannel.receiveAsFlow() + + private var messageReference: MessageReference? = null var cryptoResult: CryptoResultAnnotation? = null - fun loadData(messageReference: MessageReference): StateFlow { + fun initialize(messageReference: MessageReference) { + this.messageReference = messageReference + loadData(messageReference) + } + + fun reload() { + messageReference?.let { messageReference -> + if (contactRepository is CachingRepository) { + contactRepository.clearCache() + } + + loadData(messageReference) + } + } + + private fun loadData(messageReference: MessageReference) { viewModelScope.launch(Dispatchers.IO) { - uiState.value = try { + internalUiState.value = try { val account = accountManager.getAccount(messageReference.accountUuid) ?: error("Account not found") val messageDetails = messageRepository.getMessageDetails(messageReference) @@ -82,8 +103,6 @@ internal class MessageDetailsViewModel( MessageDetailsState.Error } } - - return uiState } private fun buildDisplayDate(messageDate: MessageDate): String? { From d7d221ebbf261c18fac9fa38530af7d2e0cd5011 Mon Sep 17 00:00:00 2001 From: cketti Date: Wed, 15 Mar 2023 15:51:45 +0100 Subject: [PATCH 4/4] Don't rely on ViewHolder.unbindView() being called when an item is updated --- .../com/fsck/k9/ui/messagedetails/CryptoStatusItem.kt | 5 +++-- .../com/fsck/k9/ui/messagedetails/ParticipantItem.kt | 11 ++++++----- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/app/ui/legacy/src/main/java/com/fsck/k9/ui/messagedetails/CryptoStatusItem.kt b/app/ui/legacy/src/main/java/com/fsck/k9/ui/messagedetails/CryptoStatusItem.kt index 5f4116e06..073e23c1f 100644 --- a/app/ui/legacy/src/main/java/com/fsck/k9/ui/messagedetails/CryptoStatusItem.kt +++ b/app/ui/legacy/src/main/java/com/fsck/k9/ui/messagedetails/CryptoStatusItem.kt @@ -37,7 +37,9 @@ internal class CryptoStatusItem(val cryptoDetails: CryptoDetails) : AbstractItem descriptionTextView.text = context.getString(stringResId) } - if (!cryptoDetails.isClickable) { + if (cryptoDetails.isClickable) { + itemView.background = originalBackground + } else { itemView.background = null } } @@ -46,7 +48,6 @@ internal class CryptoStatusItem(val cryptoDetails: CryptoDetails) : AbstractItem imageView.setImageDrawable(null) titleTextView.text = null descriptionTextView.text = null - itemView.background = originalBackground } } } diff --git 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 index fc8463fa7..efe066d20 100644 --- 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 @@ -40,6 +40,7 @@ internal class ParticipantItem( if (participant.displayName != null) { name.text = participant.displayName + name.isVisible = true } else { name.isVisible = false } @@ -48,12 +49,16 @@ internal class ParticipantItem( menuAddContact.isVisible = !item.alwaysHideAddContactsButton && !participant.isInContacts if (item.showContactsPicture) { + contactPicture.isVisible = true item.contactPictureLoader.setContactPicture(contactPicture, participant.address) } else { contactPicture.isVisible = false } - if (!item.participant.isInContacts) { + if (item.participant.isInContacts) { + itemView.isClickable = true + itemView.background = originalBackground + } else { itemView.isClickable = false itemView.background = null } @@ -61,11 +66,7 @@ internal class ParticipantItem( override fun unbindView(item: ParticipantItem) { name.text = null - name.isVisible = true email.text = null - contactPicture.isVisible = true - itemView.background = originalBackground - itemView.isClickable = true } } }