From 135de25dd9877020aa2be1a6351b6ea482a40d82 Mon Sep 17 00:00:00 2001 From: NicolasBourdin88 Date: Fri, 10 May 2024 08:40:35 +0200 Subject: [PATCH] Apply suggestion from code review --- .../cache/mailboxContent/MessageController.kt | 1 - .../com/infomaniak/mail/ui/MainViewModel.kt | 4 +-- .../mail/ui/main/folder/ThreadListAdapter.kt | 9 ++--- .../mail/ui/main/folder/ThreadListFragment.kt | 10 ++---- .../main/settings/SettingRadioButtonView.kt | 1 - .../mail/ui/main/thread/ThreadFragment.kt | 33 +++++++++---------- .../res/layout/view_setting_radio_button.xml | 12 ++----- 7 files changed, 29 insertions(+), 41 deletions(-) diff --git a/app/src/main/java/com/infomaniak/mail/data/cache/mailboxContent/MessageController.kt b/app/src/main/java/com/infomaniak/mail/data/cache/mailboxContent/MessageController.kt index 15f71979935..5d14d5ce489 100644 --- a/app/src/main/java/com/infomaniak/mail/data/cache/mailboxContent/MessageController.kt +++ b/app/src/main/java/com/infomaniak/mail/data/cache/mailboxContent/MessageController.kt @@ -205,7 +205,6 @@ class MessageController @Inject constructor(private val mailboxContentRealm: Rea fun getNewestMessages(folderId: String, limit: Int, realm: MutableRealm): List { return getOldestOrNewestMessagesQuery(folderId, Sort.DESCENDING, realm, limit).find() } - //endregion //region Edit data diff --git a/app/src/main/java/com/infomaniak/mail/ui/MainViewModel.kt b/app/src/main/java/com/infomaniak/mail/ui/MainViewModel.kt index 8612a7bd3da..7e7a6db02eb 100644 --- a/app/src/main/java/com/infomaniak/mail/ui/MainViewModel.kt +++ b/app/src/main/java/com/infomaniak/mail/ui/MainViewModel.kt @@ -617,11 +617,11 @@ class MainViewModel @Inject constructor( //region Archive fun archiveMessage(threadUid: String, message: Message) { - archiveThreadsOrMessage(listOf(threadUid), message = message) + archiveThreadsOrMessage(threadsUids = listOf(threadUid), message = message) } fun archiveThread(threadUid: String) { - archiveThreadsOrMessage(listOf(threadUid)) + archiveThreadsOrMessage(threadsUids = listOf(threadUid)) } fun archiveThreads(threadsUids: List) { diff --git a/app/src/main/java/com/infomaniak/mail/ui/main/folder/ThreadListAdapter.kt b/app/src/main/java/com/infomaniak/mail/ui/main/folder/ThreadListAdapter.kt index d7edd046889..87abf01aa5d 100644 --- a/app/src/main/java/com/infomaniak/mail/ui/main/folder/ThreadListAdapter.kt +++ b/app/src/main/java/com/infomaniak/mail/ui/main/folder/ThreadListAdapter.kt @@ -293,11 +293,11 @@ class ThreadListAdapter @Inject constructor( if (multiSelection?.isEnabled == true) { toggleMultiSelectedThread(thread) } else { - previousThreadClickedPosition?.let { previousThreadClickedPosition -> - if (position > previousThreadClickedPosition) { - localSettings.autoAdvanceIntelligentMode = AutoAdvanceMode.FOLLOWING_THREAD + previousThreadClickedPosition?.let { + localSettings.autoAdvanceIntelligentMode = if (position > it) { + AutoAdvanceMode.FOLLOWING_THREAD } else { - localSettings.autoAdvanceIntelligentMode = AutoAdvanceMode.PREVIOUS_THREAD + AutoAdvanceMode.PREVIOUS_THREAD } } @@ -311,6 +311,7 @@ class ThreadListAdapter @Inject constructor( } fun selectNewThread(newPosition: Int?, threadUid: String?) { + val oldPosition = openedThreadPosition openedThreadPosition = newPosition diff --git a/app/src/main/java/com/infomaniak/mail/ui/main/folder/ThreadListFragment.kt b/app/src/main/java/com/infomaniak/mail/ui/main/folder/ThreadListFragment.kt index baaac402847..b03db9d7bce 100644 --- a/app/src/main/java/com/infomaniak/mail/ui/main/folder/ThreadListFragment.kt +++ b/app/src/main/java/com/infomaniak/mail/ui/main/folder/ThreadListFragment.kt @@ -173,16 +173,13 @@ class ThreadListFragment : TwoPaneFragment(), SwipeRefreshLayout.OnRefreshListen override fun getRightPane(): FragmentContainerView? = _binding?.threadHostFragment override fun getAnchor(): View? { - return if (isSelectedThreadAtLastPositionOrStart() || !isOnlyRightShown()) { - _binding?.newMessageFab - } else { + return if (isOnlyRightShown()) { _binding?.threadHostFragment?.getFragment()?.getAnchor() + } else { + _binding?.newMessageFab } } - private fun isSelectedThreadAtLastPositionOrStart() = - threadListAdapter.openedThreadPosition == 1 || threadListAdapter.openedThreadPosition == threadListAdapter.dataSet.size - override fun doAfterFolderChanged() { navigateFromNotificationToThread() } @@ -632,7 +629,6 @@ class ThreadListFragment : TwoPaneFragment(), SwipeRefreshLayout.OnRefreshListen } } - private fun checkLastUpdateDay() { if (lastUpdatedDate?.isToday() == false) mainViewModel.forceTriggerCurrentFolder() } diff --git a/app/src/main/java/com/infomaniak/mail/ui/main/settings/SettingRadioButtonView.kt b/app/src/main/java/com/infomaniak/mail/ui/main/settings/SettingRadioButtonView.kt index 86f61213120..bfe1725c3c5 100644 --- a/app/src/main/java/com/infomaniak/mail/ui/main/settings/SettingRadioButtonView.kt +++ b/app/src/main/java/com/infomaniak/mail/ui/main/settings/SettingRadioButtonView.kt @@ -55,7 +55,6 @@ class SettingRadioButtonView @JvmOverloads constructor( associatedValue = getString(R.styleable.SettingRadioButtonView_value) setIcon(iconDrawable) - text.text = textString checkMark.setColorFilter(checkMarkColor) diff --git a/app/src/main/java/com/infomaniak/mail/ui/main/thread/ThreadFragment.kt b/app/src/main/java/com/infomaniak/mail/ui/main/thread/ThreadFragment.kt index 421e962779e..3e2f27af611 100644 --- a/app/src/main/java/com/infomaniak/mail/ui/main/thread/ThreadFragment.kt +++ b/app/src/main/java/com/infomaniak/mail/ui/main/thread/ThreadFragment.kt @@ -467,7 +467,6 @@ class ThreadFragment : Fragment() { private fun observeClosedThread() { mainViewModel.autoAdvanceTrigger.observe(viewLifecycleOwner) { listThreadsUids -> - println("called") tryToAutoAdvance(localSettings.autoAdvanceMode, listThreadsUids) } } @@ -631,22 +630,6 @@ class ThreadFragment : Fragment() { twoPaneViewModel.navArgs.value = NavData(resId, args) } - private fun getNextThreadToOpenByPosition(startingThreadIndex: Int, autoAdvanceMode: AutoAdvanceMode): Pair? = - with(twoPaneFragment.threadListAdapter) { - return when (autoAdvanceMode) { - AutoAdvanceMode.PREVIOUS_THREAD -> getNextThread(startingThreadIndex, direction = PREVIOUS_CHRONOLOGICAL_THREAD) - AutoAdvanceMode.FOLLOWING_THREAD -> getNextThread(startingThreadIndex, direction = NEXT_CHRONOLOGICAL_THREAD) - AutoAdvanceMode.LIST_THREAD -> null - AutoAdvanceMode.NATURAL_THREAD -> { - if (localSettings.autoAdvanceIntelligentMode == AutoAdvanceMode.PREVIOUS_THREAD) { - getNextThread(startingThreadIndex, direction = PREVIOUS_CHRONOLOGICAL_THREAD) - } else { - getNextThread(startingThreadIndex, direction = NEXT_CHRONOLOGICAL_THREAD) - } - } - } - } - private fun tryToAutoAdvance(autoAdvanceMode: AutoAdvanceMode, listThreadUids: List) = with(twoPaneFragment.threadListAdapter) { if (!listThreadUids.contains(openedThreadUid)) return@with @@ -665,6 +648,22 @@ class ThreadFragment : Fragment() { } } + private fun getNextThreadToOpenByPosition(startingThreadIndex: Int, autoAdvanceMode: AutoAdvanceMode): Pair? = + with(twoPaneFragment.threadListAdapter) { + return when (autoAdvanceMode) { + AutoAdvanceMode.PREVIOUS_THREAD -> getNextThread(startingThreadIndex, direction = PREVIOUS_CHRONOLOGICAL_THREAD) + AutoAdvanceMode.FOLLOWING_THREAD -> getNextThread(startingThreadIndex, direction = NEXT_CHRONOLOGICAL_THREAD) + AutoAdvanceMode.LIST_THREAD -> null + AutoAdvanceMode.NATURAL_THREAD -> { + if (localSettings.autoAdvanceIntelligentMode == AutoAdvanceMode.PREVIOUS_THREAD) { + getNextThread(startingThreadIndex, direction = PREVIOUS_CHRONOLOGICAL_THREAD) + } else { + getNextThread(startingThreadIndex, direction = NEXT_CHRONOLOGICAL_THREAD) + } + } + } + } + enum class HeaderState { ELEVATED, LOWERED, diff --git a/app/src/main/res/layout/view_setting_radio_button.xml b/app/src/main/res/layout/view_setting_radio_button.xml index 921e6a0a85f..530af1bdc9c 100644 --- a/app/src/main/res/layout/view_setting_radio_button.xml +++ b/app/src/main/res/layout/view_setting_radio_button.xml @@ -33,18 +33,12 @@ android:importantForAccessibility="no" android:visibility="gone" /> - - - - + tools:text="Simple messages" />