refractor: Apply suggestion from code review

Infomaniak · Nov 22, 2024 · e5322cb · e5322cb
1 parent c707760
commit e5322cb
Show file tree

Hide file tree

Showing 9 changed files with 21 additions and 17 deletions.
diff --git a/app/src/main/java/com/infomaniak/mail/ui/MainViewModel.kt b/app/src/main/java/com/infomaniak/mail/ui/MainViewModel.kt
@@ -1204,8 +1204,8 @@ class MainViewModel @Inject constructor(
         return messageUids
     }
 
-    fun getSubject(threadUuid: String): String? {
-        return threadController.getThread(threadUuid)?.subject
+    fun getSubject(threadUuid: String): String {
+        return threadController.getThread(threadUuid)?.subject ?: ""
     }
 
     companion object {

diff --git a/.../main/java/com/infomaniak/mail/ui/main/thread/actions/DownloadAttachmentProgressDialog.kt b/.../main/java/com/infomaniak/mail/ui/main/thread/actions/DownloadAttachmentProgressDialog.kt
@@ -17,6 +17,12 @@
  */
 package com.infomaniak.mail.ui.main.thread.actions
 
+import android.os.Bundle
+import android.view.LayoutInflater
+import android.view.View
+import android.view.ViewGroup
+import androidx.appcompat.content.res.AppCompatResources
+import androidx.core.view.isVisible
 import androidx.fragment.app.viewModels
 import androidx.navigation.fragment.findNavController
 import androidx.navigation.fragment.navArgs
@@ -31,7 +37,12 @@ class DownloadAttachmentProgressDialog : DownloadProgressDialog() {
     private val downloadAttachmentViewModel: DownloadAttachmentViewModel by viewModels()
 
     override val dialogTitle: String? by lazy { navigationArgs.attachmentName }
-    override val dialogIconDrawableRes: Int? by lazy { navigationArgs.attachmentType.icon }
+
+    override fun onCreateView(inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle?): View? {
+        binding.icon.isVisible = true
+        binding.icon.setImageDrawable(AppCompatResources.getDrawable(requireContext(), navigationArgs.attachmentType.icon))
+        return super.onCreateView(inflater, container, savedInstanceState)
+    }
 
     override fun download() {
         downloadAttachmentViewModel.downloadAttachment().observe(this) { cachedAttachment ->

diff --git a/...rc/main/java/com/infomaniak/mail/ui/main/thread/actions/DownloadMessagesProgressDialog.kt b/...rc/main/java/com/infomaniak/mail/ui/main/thread/actions/DownloadMessagesProgressDialog.kt
@@ -37,7 +37,6 @@ class DownloadMessagesProgressDialog : DownloadProgressDialog() {
     private val downloadThreadsViewModel: DownloadMessagesViewModel by viewModels()
     private val navigationArgs: DownloadMessagesProgressDialogArgs by navArgs()
     override val dialogTitle: String? by lazy { getDialogTitleFromArgs() }
-    override val dialogIconDrawableRes: Int? by lazy { null }
 
     override fun download() {
         downloadThreadsViewModel.downloadThreads(mainViewModel.currentMailbox.value).observe(this) { threadUris ->

diff --git a/app/src/main/java/com/infomaniak/mail/ui/main/thread/actions/DownloadProgressDialog.kt b/app/src/main/java/com/infomaniak/mail/ui/main/thread/actions/DownloadProgressDialog.kt
@@ -20,9 +20,6 @@ package com.infomaniak.mail.ui.main.thread.actions
 import android.app.Dialog
 import android.os.Bundle
 import android.view.KeyEvent
-import androidx.annotation.DrawableRes
-import androidx.appcompat.content.res.AppCompatResources
-import androidx.core.view.isVisible
 import androidx.fragment.app.DialogFragment
 import androidx.fragment.app.activityViewModels
 import androidx.lifecycle.lifecycleScope
@@ -38,10 +35,11 @@ import kotlinx.coroutines.launch
 
 @AndroidEntryPoint
 abstract class DownloadProgressDialog : DialogFragment() {
+
     protected val binding by lazy { DialogDownloadProgressBinding.inflate(layoutInflater) }
     protected val mainViewModel: MainViewModel by activityViewModels()
+
     abstract val dialogTitle: String?
-    @get:DrawableRes abstract val dialogIconDrawableRes: Int?
 
     override fun onStart() {
         download()
@@ -50,10 +48,6 @@ abstract class DownloadProgressDialog : DialogFragment() {
 
     override fun onCreateDialog(savedInstanceState: Bundle?): Dialog {
         val context = requireContext()
-
-        dialogIconDrawableRes?.let { binding.icon.setImageDrawable(AppCompatResources.getDrawable(requireContext(), it)) }
-        binding.icon.isVisible = dialogIconDrawableRes == null
-
         isCancelable = false
 
         return MaterialAlertDialogBuilder(context)

diff --git a/app/src/main/java/com/infomaniak/mail/ui/main/thread/actions/MultiSelectBottomSheetDialog.kt b/app/src/main/java/com/infomaniak/mail/ui/main/thread/actions/MultiSelectBottomSheetDialog.kt
@@ -122,7 +122,7 @@ class MultiSelectBottomSheetDialog : ActionsBottomSheetDialog() {
             trackMultiSelectActionEvent(ACTION_SAVE_KDRIVE_NAME, selectedThreadsCount, isFromBottomSheet = true)
             navigateToDownloadMessagesProgressDialog(
                 messageUids = mainViewModel.getMessagesUidsFromThreadUids(selectedThreadsUids),
-                nameFirstMessage = selectedThreads.firstOrNull()?.subject,
+                nameFirstMessage = mainViewModel.getSubject(selectedThreadsUids.first()),
                 MultiSelectBottomSheetDialog::class.java.name,
             )
             isMultiSelectOn = false

diff --git a/...rc/main/java/com/infomaniak/mail/ui/main/thread/actions/ThreadActionsBottomSheetDialog.kt b/...rc/main/java/com/infomaniak/mail/ui/main/thread/actions/ThreadActionsBottomSheetDialog.kt
@@ -200,7 +200,7 @@ class ThreadActionsBottomSheetDialog : MailActionsBottomSheetDialog() {
                     trackBottomSheetThreadActionsEvent(ACTION_SAVE_KDRIVE_NAME)
                     navigateToDownloadMessagesProgressDialog(
                         messageUids = thread.messages.map { it.uid },
-                        nameFirstMessage = thread.subject,
+                        nameFirstMessage = thread.subject ?: "",
                         ThreadActionsBottomSheetDialog::class.java.name
                     )
                 }

diff --git a/app/src/main/java/com/infomaniak/mail/utils/extensions/NavigationExtensions.kt b/app/src/main/java/com/infomaniak/mail/utils/extensions/NavigationExtensions.kt
@@ -93,7 +93,7 @@ fun Fragment.navigateToDownloadProgressDialog(
 
 fun Fragment.navigateToDownloadMessagesProgressDialog(
     messageUids: List<String>,
-    nameFirstMessage: String?,
+    nameFirstMessage: String,
     currentClassName: String
 ) {
     safeNavigate(

diff --git a/app/src/main/res/layout/dialog_download_progress.xml b/app/src/main/res/layout/dialog_download_progress.xml
@@ -29,6 +29,7 @@
         android:layout_marginStart="@dimen/marginStandardMedium"
         android:importantForAccessibility="no"
         android:src="@drawable/ic_file_unknown"
+        android:visibility="gone"
         app:layout_constraintBottom_toBottomOf="parent"
         app:layout_constraintStart_toStartOf="parent"
         app:layout_constraintTop_toTopOf="parent" />

diff --git a/app/src/main/res/navigation/main_navigation.xml b/app/src/main/res/navigation/main_navigation.xml
@@ -583,8 +583,7 @@
             app:argType="string[]" />
         <argument
             android:name="nameFirstMessage"
-            app:argType="string"
-            app:nullable="true" />
+            app:argType="string" />
     </dialog>
 
     <dialog