Simplify login dialogs under single state property (#1109)

app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/login/LoginScreen.kt

@@ -45,8 +45,10 @@ import com.x8bit.bitwarden.ui.platform.components.appbar.action.BitwardenOverflo
 import com.x8bit.bitwarden.ui.platform.components.appbar.action.OverflowMenuItemData
 import com.x8bit.bitwarden.ui.platform.components.button.BitwardenFilledButton
 import com.x8bit.bitwarden.ui.platform.components.button.BitwardenOutlinedButtonWithIcon
+import com.x8bit.bitwarden.ui.platform.components.dialog.BasicDialogState
 import com.x8bit.bitwarden.ui.platform.components.dialog.BitwardenBasicDialog
 import com.x8bit.bitwarden.ui.platform.components.dialog.BitwardenLoadingDialog
+import com.x8bit.bitwarden.ui.platform.components.dialog.LoadingDialogState
 import com.x8bit.bitwarden.ui.platform.components.field.BitwardenPasswordField
 import com.x8bit.bitwarden.ui.platform.components.scaffold.BitwardenScaffold
 import com.x8bit.bitwarden.ui.platform.components.text.BitwardenClickableText
@@ -60,7 +62,7 @@ import kotlinx.collections.immutable.toImmutableList
  */
 @OptIn(ExperimentalMaterial3Api::class)
 @Composable
-@Suppress("LongMethod", "LongParameterList")
+@Suppress("LongMethod")
 fun LoginScreen(
     onNavigateBack: () -> Unit,
     onNavigateToMasterPasswordHint: (String) -> Unit,
@@ -102,6 +104,13 @@ fun LoginScreen(
         }
     }
 
+    LoginDialogs(
+        dialogState = state.dialogState,
+        onDismissRequest = remember(viewModel) {
+            { viewModel.trySendAction(LoginAction.ErrorDialogDismiss) }
+        },
+    )
+
     val isAccountButtonVisible = state.accountSummaries.isNotEmpty()
     var isAccountMenuVisible by rememberSaveable { mutableStateOf(false) }
     val scrollBehavior = TopAppBarDefaults.pinnedScrollBehavior(rememberTopAppBarState())
@@ -140,9 +149,6 @@ fun LoginScreen(
     ) { innerPadding ->
         LoginScreenContent(
             state = state,
-            onErrorDialogDismiss = remember(viewModel) {
-                { viewModel.trySendAction(LoginAction.ErrorDialogDismiss) }
-            },
             onPasswordInputChanged = remember(viewModel) {
                 { viewModel.trySendAction(LoginAction.PasswordInputChanged(it)) }
             },
@@ -193,11 +199,32 @@ fun LoginScreen(
     }
 }
 
-@Suppress("LongMethod", "LongParameterList")
+@Composable
+private fun LoginDialogs(
+    dialogState: LoginState.DialogState?,
+    onDismissRequest: () -> Unit,
+) {
+    when (dialogState) {
+        is LoginState.DialogState.Error -> BitwardenBasicDialog(
+            visibilityState = BasicDialogState.Shown(
+                title = dialogState.title,
+                message = dialogState.message,
+            ),
+            onDismissRequest = onDismissRequest,
+        )
+
+        is LoginState.DialogState.Loading -> BitwardenLoadingDialog(
+            visibilityState = LoadingDialogState.Shown(text = dialogState.message),
+        )
+
+        null -> Unit
+    }
+}
+
+@Suppress("LongMethod")
 @Composable
 private fun LoginScreenContent(
     state: LoginState,
-    onErrorDialogDismiss: () -> Unit,
     onPasswordInputChanged: (String) -> Unit,
     onMasterPasswordClick: () -> Unit,
     onLoginButtonClick: () -> Unit,
@@ -211,14 +238,6 @@ private fun LoginScreenContent(
             .imePadding()
             .verticalScroll(rememberScrollState()),
     ) {
-        BitwardenLoadingDialog(
-            visibilityState = state.loadingDialogState,
-        )
-        BitwardenBasicDialog(
-            visibilityState = state.errorDialogState,
-            onDismissRequest = onErrorDialogDismiss,
-        )
-
         BitwardenPasswordField(
             modifier = Modifier
                 .semantics { testTag = "MasterPasswordEntry" }

app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/login/LoginViewModel.kt

@@ -15,9 +15,8 @@ import com.x8bit.bitwarden.data.auth.repository.util.generateUriForCaptcha
 import com.x8bit.bitwarden.data.platform.repository.EnvironmentRepository
 import com.x8bit.bitwarden.data.vault.repository.VaultRepository
 import com.x8bit.bitwarden.ui.platform.base.BaseViewModel
+import com.x8bit.bitwarden.ui.platform.base.util.Text
 import com.x8bit.bitwarden.ui.platform.base.util.asText
-import com.x8bit.bitwarden.ui.platform.components.dialog.BasicDialogState
-import com.x8bit.bitwarden.ui.platform.components.dialog.LoadingDialogState
 import com.x8bit.bitwarden.ui.platform.components.model.AccountSummary
 import com.x8bit.bitwarden.ui.vault.feature.vault.util.toAccountSummaries
 import dagger.hilt.android.lifecycle.HiltViewModel
@@ -46,8 +45,7 @@ class LoginViewModel @Inject constructor(
             isLoginButtonEnabled = false,
             passwordInput = "",
             environmentLabel = environmentRepository.environment.label,
-            loadingDialogState = LoadingDialogState.Shown(R.string.loading.asText()),
+            dialogState = LoginState.DialogState.Loading(R.string.loading.asText()),
-            errorDialogState = BasicDialogState.Hidden,
             captchaToken = LoginArgs(savedStateHandle).captchaToken,
             accountSummaries = authRepository.userStateFlow.value?.toAccountSummaries().orEmpty(),
             shouldShowLoginWithDevice = false,
@@ -130,7 +128,7 @@ class LoginViewModel @Inject constructor(
             is KnownDeviceResult.Success -> {
                 mutableStateFlow.update {
                     it.copy(
-                        loadingDialogState = LoadingDialogState.Hidden,
+                        dialogState = null,
                         shouldShowLoginWithDevice = action.knownDeviceResult.isKnownDevice,
                     )
                 }
@@ -139,7 +137,7 @@ class LoginViewModel @Inject constructor(
             is KnownDeviceResult.Error -> {
                 mutableStateFlow.update {
                     it.copy(
-                        loadingDialogState = LoadingDialogState.Hidden,
+                        dialogState = null,
                         shouldShowLoginWithDevice = false,
                     )
                 }
@@ -150,7 +148,7 @@ class LoginViewModel @Inject constructor(
     private fun handleReceiveLoginResult(action: LoginAction.Internal.ReceiveLoginResult) {
         when (val loginResult = action.loginResult) {
             is LoginResult.CaptchaRequired -> {
-                mutableStateFlow.update { it.copy(loadingDialogState = LoadingDialogState.Hidden) }
+                mutableStateFlow.update { it.copy(dialogState = null) }
                 sendEvent(
                     event = LoginEvent.NavigateToCaptcha(
                         uri = generateUriForCaptcha(captchaId = loginResult.captchaId),
@@ -159,7 +157,7 @@ class LoginViewModel @Inject constructor(
             }
 
             is LoginResult.TwoFactorRequired -> {
-                mutableStateFlow.update { it.copy(loadingDialogState = LoadingDialogState.Hidden) }
+                mutableStateFlow.update { it.copy(dialogState = null) }
                 sendEvent(
                     LoginEvent.NavigateToTwoFactorLogin(
                         emailAddress = state.emailAddress,
@@ -171,24 +169,23 @@ class LoginViewModel @Inject constructor(
             is LoginResult.Error -> {
                 mutableStateFlow.update {
                     it.copy(
-                        errorDialogState = BasicDialogState.Shown(
+                        dialogState = LoginState.DialogState.Error(
                             title = R.string.an_error_has_occurred.asText(),
-                            message = (loginResult.errorMessage)?.asText()
+                            message = loginResult.errorMessage?.asText()
                                 ?: R.string.generic_error_message.asText(),
                         ),
-                        loadingDialogState = LoadingDialogState.Hidden,
                     )
                 }
             }
 
             is LoginResult.Success -> {
-                mutableStateFlow.update { it.copy(loadingDialogState = LoadingDialogState.Hidden) }
+                mutableStateFlow.update { it.copy(dialogState = null) }
             }
         }
     }
 
     private fun handleErrorDialogDismiss() {
-        mutableStateFlow.update { it.copy(errorDialogState = BasicDialogState.Hidden) }
+        mutableStateFlow.update { it.copy(dialogState = null) }
     }
 
     private fun handleCaptchaTokenReceived(tokenResult: CaptchaCallbackTokenResult) {
@@ -196,7 +193,7 @@ class LoginViewModel @Inject constructor(
             CaptchaCallbackTokenResult.MissingToken -> {
                 mutableStateFlow.update {
                     it.copy(
-                        errorDialogState = BasicDialogState.Shown(
+                        dialogState = LoginState.DialogState.Error(
                             title = R.string.log_in_denied.asText(),
                             message = R.string.captcha_failed.asText(),
                         ),
@@ -228,8 +225,8 @@ class LoginViewModel @Inject constructor(
     private fun attemptLogin() {
         mutableStateFlow.update {
             it.copy(
-                loadingDialogState = LoadingDialogState.Shown(
+                dialogState = LoginState.DialogState.Loading(
-                    text = R.string.logging_in.asText(),
+                    message = R.string.logging_in.asText(),
                 ),
             )
         }
@@ -281,11 +278,32 @@ data class LoginState(
     val captchaToken: String?,
     val environmentLabel: String,
     val isLoginButtonEnabled: Boolean,
-    val loadingDialogState: LoadingDialogState,
+    val dialogState: DialogState?,
-    val errorDialogState: BasicDialogState,
     val accountSummaries: List<AccountSummary>,
     val shouldShowLoginWithDevice: Boolean,
-) : Parcelable
+) : Parcelable {
+    /**
+     * Represents the current state of any dialogs on the screen.
+     */
+    sealed class DialogState : Parcelable {
+        /**
+         * Represents a dismissible dialog with the given error [message].
+         */
+        @Parcelize
+        data class Error(
+            val title: Text?,
+            val message: Text,
+        ) : DialogState()
+
+        /**
+         * Represents a loading dialog with the given [message].
+         */
+        @Parcelize
+        data class Loading(
+            val message: Text,
+        ) : DialogState()
+    }
+}
 
 /**
  * Models events for the login screen.

app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/login/LoginScreenTest.kt

@@ -16,8 +16,6 @@ import androidx.compose.ui.test.performScrollTo
 import androidx.compose.ui.test.performTextInput
 import com.x8bit.bitwarden.data.platform.repository.util.bufferedMutableSharedFlow
 import com.x8bit.bitwarden.ui.platform.base.BaseComposeTest
-import com.x8bit.bitwarden.ui.platform.components.dialog.BasicDialogState
-import com.x8bit.bitwarden.ui.platform.components.dialog.LoadingDialogState
 import com.x8bit.bitwarden.ui.platform.components.model.AccountSummary
 import com.x8bit.bitwarden.ui.platform.manager.intent.IntentManager
 import com.x8bit.bitwarden.ui.util.assertLockOrLogoutDialogIsDisplayed
@@ -328,8 +326,7 @@ private val DEFAULT_STATE =
         isLoginButtonEnabled = false,
         passwordInput = "",
         environmentLabel = "",
-        loadingDialogState = LoadingDialogState.Hidden,
+        dialogState = null,
-        errorDialogState = BasicDialogState.Hidden,
         accountSummaries = emptyList(),
         shouldShowLoginWithDevice = false,
     )

app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/login/LoginViewModelTest.kt

@@ -17,8 +17,6 @@ import com.x8bit.bitwarden.data.platform.repository.util.bufferedMutableSharedFl
 import com.x8bit.bitwarden.data.vault.repository.VaultRepository
 import com.x8bit.bitwarden.ui.platform.base.BaseViewModelTest
 import com.x8bit.bitwarden.ui.platform.base.util.asText
-import com.x8bit.bitwarden.ui.platform.components.dialog.BasicDialogState
-import com.x8bit.bitwarden.ui.platform.components.dialog.LoadingDialogState
 import com.x8bit.bitwarden.ui.platform.components.model.AccountSummary
 import com.x8bit.bitwarden.ui.vault.feature.vault.util.toAccountSummaries
 import io.mockk.coEvery
@@ -259,19 +257,18 @@ class LoginViewModelTest : BaseViewModelTest() {
             viewModel.trySendAction(LoginAction.LoginButtonClick)
             assertEquals(
                 DEFAULT_STATE.copy(
-                    loadingDialogState = LoadingDialogState.Shown(
+                    dialogState = LoginState.DialogState.Loading(
-                        text = R.string.logging_in.asText(),
+                        message = R.string.logging_in.asText(),
                     ),
                 ),
                 awaitItem(),
             )
             assertEquals(
                 DEFAULT_STATE.copy(
-                    errorDialogState = BasicDialogState.Shown(
+                    dialogState = LoginState.DialogState.Error(
                         title = R.string.an_error_has_occurred.asText(),
                         message = "mock_error".asText(),
                     ),
-                    loadingDialogState = LoadingDialogState.Hidden,
                 ),
                 awaitItem(),
             )
@@ -296,14 +293,14 @@ class LoginViewModelTest : BaseViewModelTest() {
             viewModel.trySendAction(LoginAction.LoginButtonClick)
             assertEquals(
                 DEFAULT_STATE.copy(
-                    loadingDialogState = LoadingDialogState.Shown(
+                    dialogState = LoginState.DialogState.Loading(
-                        text = R.string.logging_in.asText(),
+                        message = R.string.logging_in.asText(),
                     ),
                 ),
                 awaitItem(),
             )
             assertEquals(
-                DEFAULT_STATE.copy(loadingDialogState = LoadingDialogState.Hidden),
+                DEFAULT_STATE.copy(dialogState = null),
                 awaitItem(),
             )
         }
@@ -464,8 +461,7 @@ class LoginViewModelTest : BaseViewModelTest() {
             passwordInput = "",
             isLoginButtonEnabled = false,
             environmentLabel = Environment.Us.label,
-            loadingDialogState = LoadingDialogState.Hidden,
+            dialogState = null,
-            errorDialogState = BasicDialogState.Hidden,
             captchaToken = null,
             accountSummaries = emptyList(),
             shouldShowLoginWithDevice = false,