Code review

Author: tevincent

app/src/main/java/com/infomaniak/mail/ui/bottomSheetDialogs/AccountBottomSheetDialog.kt

@@ -36,6 +36,7 @@ import com.infomaniak.mail.ui.main.user.SwitchUserAdapter
 import com.infomaniak.mail.ui.main.user.SwitchUserViewModel
 import com.infomaniak.mail.utils.AccountUtils
 import com.infomaniak.mail.utils.LogoutUser
+import com.infomaniak.mail.utils.extensions.bindAlertToViewLifecycle
 import com.infomaniak.mail.utils.extensions.launchLoginActivity
 import dagger.hilt.android.AndroidEntryPoint
 import kotlinx.coroutines.CoroutineDispatcher
@@ -84,8 +85,9 @@ class AccountBottomSheetDialog : BottomSheetDialogFragment() {
             context.trackAccountEvent("add")
             context.launchLoginActivity()
         }
-
         observeAccounts()
+
+        bindAlertToViewLifecycle(descriptionDialog)
     }
 
     private fun logoutCurrentUser() = lifecycleScope.launch(ioDispatcher) {

app/src/main/java/com/infomaniak/mail/ui/main/menuDrawer/MenuDrawerAdapter.kt

@@ -90,7 +90,7 @@ class MenuDrawerAdapter @Inject constructor() : ListAdapter<Any, MenuDrawerViewH
         add(MailboxesHeader(currentMailbox, otherMailboxes.isNotEmpty(), areMailboxesExpanded))
         if (areMailboxesExpanded) {
             addAll(otherMailboxes)
-            add(ItemType.ADD_MAILBOX)
+            add(ADD_MAILBOX_ACTION)
         }
     }
 
@@ -169,7 +169,6 @@ class MenuDrawerAdapter @Inject constructor() : ListAdapter<Any, MenuDrawerViewH
             ItemType.DIVIDER -> ItemType.DIVIDER.ordinal
             is MailboxesHeader -> ItemType.MAILBOXES_HEADER.ordinal
             is Mailbox -> if (item.isValid) ItemType.MAILBOX.ordinal else ItemType.INVALID_MAILBOX.ordinal
-            ItemType.ADD_MAILBOX -> ItemType.ADD_MAILBOX.ordinal
             ItemType.FOLDERS_HEADER -> ItemType.FOLDERS_HEADER.ordinal
             is Folder -> ItemType.FOLDER.ordinal
             ItemType.EMPTY_FOLDERS -> ItemType.EMPTY_FOLDERS.ordinal
@@ -188,7 +187,6 @@ class MenuDrawerAdapter @Inject constructor() : ListAdapter<Any, MenuDrawerViewH
             ItemType.MAILBOXES_HEADER.ordinal -> MailboxesHeaderViewHolder(inflater, parent)
             ItemType.MAILBOX.ordinal -> MailboxViewHolder(inflater, parent)
             ItemType.INVALID_MAILBOX.ordinal -> InvalidMailboxViewHolder(inflater, parent)
-            ItemType.ADD_MAILBOX.ordinal -> AddMailboxViewHolder(inflater, parent)
             ItemType.FOLDERS_HEADER.ordinal -> FoldersHeaderViewHolder(inflater, parent)
             ItemType.FOLDER.ordinal -> FolderViewHolder(inflater, parent)
             ItemType.EMPTY_FOLDERS.ordinal -> EmptyFoldersViewHolder(inflater, parent)
@@ -224,7 +222,6 @@ class MenuDrawerAdapter @Inject constructor() : ListAdapter<Any, MenuDrawerViewH
                 onLockedMailboxClicked = callbacks.onLockedMailboxClicked,
                 onInvalidPasswordMailboxClicked = callbacks.onInvalidPasswordMailboxClicked,
             )
-            is AddMailboxViewHolder -> holder.setOnClickListener(callbacks.onAddMailBoxClicked)
             is FoldersHeaderViewHolder -> holder.displayFoldersHeader(
                 onFoldersHeaderClicked = callbacks.onFoldersHeaderClicked,
                 onCreateFolderClicked = callbacks.onCreateFolderClicked,
@@ -259,7 +256,6 @@ class MenuDrawerAdapter @Inject constructor() : ListAdapter<Any, MenuDrawerViewH
         MAILBOXES_HEADER,
         MAILBOX,
         INVALID_MAILBOX,
-        ADD_MAILBOX,
         FOLDERS_HEADER,
         FOLDER,
         EMPTY_FOLDERS,
@@ -279,7 +275,6 @@ class MenuDrawerAdapter @Inject constructor() : ListAdapter<Any, MenuDrawerViewH
                 ItemType.DIVIDER -> newItem == ItemType.DIVIDER
                 is MailboxesHeader -> newItem is MailboxesHeader && newItem.mailbox?.objectId == oldItem.mailbox?.objectId
                 is Mailbox -> newItem is Mailbox && newItem.objectId == oldItem.objectId
-                ItemType.ADD_MAILBOX -> newItem == ItemType.ADD_MAILBOX
                 ItemType.FOLDERS_HEADER -> newItem == ItemType.FOLDERS_HEADER
                 is Folder -> newItem is Folder && newItem.id == oldItem.id
                 ItemType.EMPTY_FOLDERS -> newItem == ItemType.EMPTY_FOLDERS
@@ -309,7 +304,6 @@ class MenuDrawerAdapter @Inject constructor() : ListAdapter<Any, MenuDrawerViewH
                 ItemType.FOLDERS_HEADER,
                 ItemType.EMPTY_FOLDERS,
                 ItemType.ACTIONS_HEADER,
-                ItemType.ADD_MAILBOX,
                 is MenuDrawerAction -> true
                 else -> error("oldItem wasn't any known item type (in MenuDrawer `areContentsTheSame`)")
             }
@@ -325,6 +319,12 @@ class MenuDrawerAdapter @Inject constructor() : ListAdapter<Any, MenuDrawerViewH
     }
 
     companion object {
+        private val ADD_MAILBOX_ACTION = MenuDrawerAction(
+            type = ActionType.ADD_MAILBOX,
+            icon = R.drawable.ic_add_circle_thin,
+            text = R.string.buttonAddExistingAddress,
+            maxLines = 1,
+        )
         private val SYNC_AUTO_CONFIG_ACTION = MenuDrawerAction(
             type = ActionType.SYNC_AUTO_CONFIG,
             icon = R.drawable.ic_synchronize,

app/src/main/java/com/infomaniak/mail/ui/main/menuDrawer/MenuDrawerAdapterCallbacks.kt

@@ -24,8 +24,6 @@ interface MenuDrawerAdapterCallbacks {
 
     var onMailboxesHeaderClicked: () -> Unit
 
-    var onAddMailBoxClicked: () -> Unit
-
     var onValidMailboxClicked: (Int) -> Unit
     var onLockedMailboxClicked: (String) -> Unit
     var onInvalidPasswordMailboxClicked: (Mailbox) -> Unit

app/src/main/java/com/infomaniak/mail/ui/main/menuDrawer/MenuDrawerFragment.kt

@@ -125,7 +125,6 @@ class MenuDrawerFragment : Fragment() {
             callbacks = object : MenuDrawerAdapterCallbacks {
                 override var onMailboxesHeaderClicked: () -> Unit = ::onMailboxesHeaderClicked
                 override var onValidMailboxClicked: (Int) -> Unit = ::onValidMailboxClicked
-                override var onAddMailBoxClicked: () -> Unit = ::onAddMailboxClicked
                 override var onLockedMailboxClicked: (String) -> Unit = ::onLockedMailboxClicked
                 override var onInvalidPasswordMailboxClicked: (Mailbox) -> Unit = ::onInvalidPasswordMailboxClicked
                 override var onFoldersHeaderClicked: (Boolean) -> Unit = ::onFoldersHeaderClicked
@@ -213,6 +212,7 @@ class MenuDrawerFragment : Fragment() {
 
     private fun onActionClicked(type: ActionType) {
         when (type) {
+            ActionType.ADD_MAILBOX -> onAddMailboxClicked()
             ActionType.SYNC_AUTO_CONFIG -> onSyncAutoConfigClicked()
             ActionType.IMPORT_MAILS -> onImportMailsClicked()
             ActionType.RESTORE_MAILS -> onRestoreMailsClicked()

app/src/main/java/com/infomaniak/mail/ui/main/menuDrawer/items/ActionViewHolder.kt

@@ -56,6 +56,7 @@ class ActionViewHolder(
     ) {
 
         enum class ActionType {
+            ADD_MAILBOX,
             SYNC_AUTO_CONFIG,
             IMPORT_MAILS,
             RESTORE_MAILS,

app/src/main/java/com/infomaniak/mail/ui/main/menuDrawer/items/AddMailboxViewHolder.kt

@@ -38,21 +38,23 @@ class AccountManagementSettingsFragment : Fragment() {
 
     private var binding: FragmentAccountManagementSettingsBinding by safeBinding()
 
-    override fun onCreate(savedInstanceState: Bundle?) {
-        super.onCreate(savedInstanceState)
-        saveFocusWhenNavigatingBack(getLayout = { binding.container }, lifecycle)
-    }
-
     override fun onCreateView(inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle?): View {
         return FragmentAccountManagementSettingsBinding.inflate(inflater, container, false).also { binding = it }.root
     }
 
-    override fun onViewCreated(view: View, savedInstanceState: Bundle?) = with(binding) {
+    override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
         super.onViewCreated(view, savedInstanceState)
         setSystemBarsColors()
 
         setUi()
-        setDeleteAccountClickListener()
+        return setDeleteAccountClickListener()
+    }
+
+    private fun setUi() = with(binding) {
+        AccountUtils.currentUser?.let {
+            username.text = it.displayName
+            email.text = it.email
+        }
     }
 
     private fun setDeleteAccountClickListener() = with(binding) {
@@ -65,13 +67,6 @@ class AccountManagementSettingsFragment : Fragment() {
         }
     }
 
-    private fun setUi() = with(binding) {
-        AccountUtils.currentUser?.let {
-            username.text = it.displayName
-            email.text = it.email
-        }
-    }
-
     companion object {
         private const val TERMINATE_ACCOUNT_FULL_URL = "$AUTOLOG_URL/?url=$TERMINATE_ACCOUNT_URL"
     }

app/src/main/java/com/infomaniak/mail/ui/main/settings/privacy/AccountManagementSettingsFragment.kt

@@ -21,7 +21,7 @@
     android:layout_width="match_parent"
     android:layout_height="match_parent"
     app:title="@string/buttonAddEmailAddress"
-    tools:context="com.infomaniak.mail.ui.main.user.AttachMailboxFragment">
+    tools:context=".ui.main.user.AttachMailboxFragment">
 
     <androidx.core.widget.NestedScrollView
         android:layout_width="match_parent"