From 0cde971c7f64e31e2ce35e9306694a7d3ded672c Mon Sep 17 00:00:00 2001 From: David Perez Date: Thu, 9 Nov 2023 09:42:18 -0600 Subject: [PATCH] BIT-974, BIT-978: Add confirmation dialogs when launching an external link. (#229) --- .../components/BitwardenExternalLinkRow.kt | 42 ++++++++++++++++-- .../feature/settings/about/AboutScreen.kt | 27 +++++++++--- .../accountsecurity/AccountSecurityScreen.kt | 10 ++++- .../feature/settings/about/AboutScreenTest.kt | 44 +++++++++++++++++-- .../AccountSecurityScreenTest.kt | 20 ++++++++- 5 files changed, 125 insertions(+), 18 deletions(-) diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/BitwardenExternalLinkRow.kt b/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/BitwardenExternalLinkRow.kt index b34b18cd2..239abc642 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/BitwardenExternalLinkRow.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/BitwardenExternalLinkRow.kt @@ -3,31 +3,48 @@ package com.x8bit.bitwarden.ui.platform.components import androidx.compose.material3.Icon import androidx.compose.material3.MaterialTheme import androidx.compose.runtime.Composable +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.remember +import androidx.compose.runtime.setValue import androidx.compose.ui.Modifier import androidx.compose.ui.res.painterResource +import androidx.compose.ui.res.stringResource import androidx.compose.ui.tooling.preview.Preview import com.x8bit.bitwarden.R import com.x8bit.bitwarden.ui.platform.theme.BitwardenTheme /** * Represents a row of text that can be clicked on and contains an external link. + * A confirmation dialog will always be displayed before [onConfirmClick] is invoked. * * @param text The label for the row as a [String]. - * @param onClick The callback when the row is clicked. + * @param onConfirmClick The callback when the confirm button of the dialog is clicked. * @param modifier The modifier to be applied to the layout. * @param withDivider Indicates if a divider should be drawn on the bottom of the row, defaults * to `true`. + * @param dialogTitle The title of the dialog displayed when the user clicks this item. + * @param dialogMessage The message of the dialog displayed when the user clicks this item. + * @param dialogConfirmButtonText The text on the confirm button of the dialog displayed when the + * user clicks this item. + * @param dialogDismissButtonText The text on the dismiss button of the dialog displayed when the + * user clicks this item. */ @Composable fun BitwardenExternalLinkRow( text: String, - onClick: () -> Unit, + onConfirmClick: () -> Unit, modifier: Modifier = Modifier, withDivider: Boolean = true, + dialogTitle: String, + dialogMessage: String, + dialogConfirmButtonText: String = stringResource(id = R.string.continue_text), + dialogDismissButtonText: String = stringResource(id = R.string.cancel), ) { + var shouldShowDialog by remember { mutableStateOf(false) } BitwardenTextRow( text = text, - onClick = onClick, + onClick = { shouldShowDialog = true }, modifier = modifier, withDivider = withDivider, ) { @@ -37,6 +54,21 @@ fun BitwardenExternalLinkRow( tint = MaterialTheme.colorScheme.onSurface, ) } + + if (shouldShowDialog) { + BitwardenTwoButtonDialog( + title = dialogTitle, + message = dialogMessage, + confirmButtonText = dialogConfirmButtonText, + dismissButtonText = dialogDismissButtonText, + onConfirmClick = { + shouldShowDialog = false + onConfirmClick() + }, + onDismissClick = { shouldShowDialog = false }, + onDismissRequest = { shouldShowDialog = false }, + ) + } } @Preview @@ -45,7 +77,9 @@ private fun BitwardenExternalLinkRow_preview() { BitwardenTheme { BitwardenExternalLinkRow( text = "Linked Text", - onClick = { }, + onConfirmClick = { }, + dialogTitle = "", + dialogMessage = "", ) } } diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/about/AboutScreen.kt b/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/about/AboutScreen.kt index b09911dbc..3c2eb6bdc 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/about/AboutScreen.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/about/AboutScreen.kt @@ -111,7 +111,7 @@ fun AboutScreen( ) }, ) { innerPadding -> - ContentColum( + ContentColumn( state = state, modifier = Modifier .padding(innerPadding) @@ -138,8 +138,9 @@ fun AboutScreen( } } +@Suppress("LongMethod") @Composable -private fun ContentColum( +private fun ContentColumn( state: AboutState, onHelpCenterClick: () -> Unit, onLearnAboutOrgsClick: () -> Unit, @@ -166,19 +167,33 @@ private fun ContentColum( Spacer(modifier = Modifier.height(16.dp)) BitwardenExternalLinkRow( text = stringResource(id = R.string.bitwarden_help_center), - onClick = onHelpCenterClick, + onConfirmClick = onHelpCenterClick, + dialogTitle = stringResource(id = R.string.continue_to_help_center), + dialogMessage = stringResource( + id = R.string.learn_more_about_how_to_use_bitwarden_on_the_help_center, + ), ) BitwardenExternalLinkRow( text = stringResource(id = R.string.web_vault), - onClick = onWebVaultClick, + onConfirmClick = onWebVaultClick, + dialogTitle = stringResource(id = R.string.continue_to_web_app), + dialogMessage = stringResource( + id = R.string.explore_more_features_of_your_bitwarden_account_on_the_web_app, + ), ) BitwardenExternalLinkRow( text = stringResource(id = R.string.learn_org), - onClick = onLearnAboutOrgsClick, + onConfirmClick = onLearnAboutOrgsClick, + dialogTitle = stringResource(id = R.string.continue_to_web_app), + dialogMessage = stringResource( + id = R.string.learn_about_organizations_description_long, + ), ) BitwardenExternalLinkRow( text = stringResource(id = R.string.rate_the_app), - onClick = onRateTheAppClick, + onConfirmClick = onRateTheAppClick, + dialogTitle = stringResource(id = R.string.continue_to_app_store), + dialogMessage = stringResource(id = R.string.rate_app_description_long), ) CopyRow( text = state.version, diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/AccountSecurityScreen.kt b/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/AccountSecurityScreen.kt index 4d1abb904..cd6c8a2c2 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/AccountSecurityScreen.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/AccountSecurityScreen.kt @@ -238,18 +238,24 @@ fun AccountSecurityScreen( ) BitwardenExternalLinkRow( text = stringResource(id = R.string.two_step_login), - onClick = remember(viewModel) { + onConfirmClick = remember(viewModel) { { viewModel.trySendAction(AccountSecurityAction.TwoStepLoginClick) } }, withDivider = false, + dialogTitle = stringResource(id = R.string.continue_to_web_app), + dialogMessage = stringResource(id = R.string.two_step_login_description_long), modifier = Modifier.fillMaxWidth(), ) BitwardenExternalLinkRow( text = stringResource(id = R.string.change_master_password), - onClick = remember(viewModel) { + onConfirmClick = remember(viewModel) { { viewModel.trySendAction(AccountSecurityAction.ChangeMasterPasswordClick) } }, withDivider = false, + dialogTitle = stringResource(id = R.string.continue_to_web_app), + dialogMessage = stringResource( + id = R.string.change_master_password_description_long, + ), modifier = Modifier.fillMaxWidth(), ) BitwardenTextRow( diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/about/AboutScreenTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/about/AboutScreenTest.kt index f0d7ceef6..95636a17a 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/about/AboutScreenTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/about/AboutScreenTest.kt @@ -4,6 +4,10 @@ import androidx.compose.ui.platform.ClipboardManager import androidx.compose.ui.test.assertIsDisplayed import androidx.compose.ui.test.assertIsOff import androidx.compose.ui.test.assertIsOn +import androidx.compose.ui.test.filterToOne +import androidx.compose.ui.test.hasAnyAncestor +import androidx.compose.ui.test.isDialog +import androidx.compose.ui.test.onAllNodesWithText import androidx.compose.ui.test.onNodeWithContentDescription import androidx.compose.ui.test.onNodeWithText import androidx.compose.ui.test.performClick @@ -50,8 +54,9 @@ class AboutScreenTest : BaseComposeTest() { verify { viewModel.trySendAction(AboutAction.BackClick) } } + @Suppress("MaxLineLength") @Test - fun `on bitwarden help center click should send HelpCenterClick`() { + fun `on bitwarden help center click should display confirmation dialog and confirm click should emit HelpCenterClick`() { val viewModel: AboutViewModel = mockk { every { stateFlow } returns mutableStateFlow every { eventFlow } returns emptyFlow() @@ -63,14 +68,22 @@ class AboutScreenTest : BaseComposeTest() { onNavigateBack = { }, ) } + composeTestRule.onNode(isDialog()).assertDoesNotExist() composeTestRule.onNodeWithText("Bitwarden Help Center").performClick() + composeTestRule.onNode(isDialog()).assertExists() + composeTestRule + .onAllNodesWithText("Continue") + .filterToOne(hasAnyAncestor(isDialog())) + .performClick() + composeTestRule.onNode(isDialog()).assertDoesNotExist() verify { viewModel.trySendAction(AboutAction.HelpCenterClick) } } + @Suppress("MaxLineLength") @Test - fun `on bitwarden web vault click should send WebVaultClick`() { + fun `on bitwarden web vault click should display confirmation dialog and confirm click should emit WebVaultClick`() { val viewModel: AboutViewModel = mockk { every { stateFlow } returns mutableStateFlow every { eventFlow } returns emptyFlow() @@ -82,7 +95,14 @@ class AboutScreenTest : BaseComposeTest() { onNavigateBack = { }, ) } + composeTestRule.onNode(isDialog()).assertDoesNotExist() composeTestRule.onNodeWithText("Bitwarden web vault").performClick() + composeTestRule.onNode(isDialog()).assertExists() + composeTestRule + .onAllNodesWithText("Continue") + .filterToOne(hasAnyAncestor(isDialog())) + .performClick() + composeTestRule.onNode(isDialog()).assertDoesNotExist() verify { viewModel.trySendAction(AboutAction.WebVaultClick) } @@ -110,8 +130,9 @@ class AboutScreenTest : BaseComposeTest() { } } + @Suppress("MaxLineLength") @Test - fun `on learn about organizations click should send LearnAboutOrganizationsClick`() { + fun `on learn about organizations click should display confirmation dialog and confirm click should emit LearnAboutOrganizationsClick`() { val viewModel: AboutViewModel = mockk { every { stateFlow } returns mutableStateFlow every { eventFlow } returns emptyFlow() @@ -123,7 +144,14 @@ class AboutScreenTest : BaseComposeTest() { onNavigateBack = { }, ) } + composeTestRule.onNode(isDialog()).assertDoesNotExist() composeTestRule.onNodeWithText("Learn about organizations").performClick() + composeTestRule.onNode(isDialog()).assertExists() + composeTestRule + .onAllNodesWithText("Continue") + .filterToOne(hasAnyAncestor(isDialog())) + .performClick() + composeTestRule.onNode(isDialog()).assertDoesNotExist() verify { viewModel.trySendAction(AboutAction.LearnAboutOrganizationsClick) } @@ -208,8 +236,9 @@ class AboutScreenTest : BaseComposeTest() { } } + @Suppress("MaxLineLength") @Test - fun `on rate the app click should send RateAppClick`() { + fun `on rate the app click should display confirmation dialog and confirm click should emit RateAppClick`() { val viewModel: AboutViewModel = mockk { every { stateFlow } returns mutableStateFlow every { eventFlow } returns emptyFlow() @@ -221,7 +250,14 @@ class AboutScreenTest : BaseComposeTest() { onNavigateBack = { }, ) } + composeTestRule.onNode(isDialog()).assertDoesNotExist() composeTestRule.onNodeWithText("Rate the app").performClick() + composeTestRule.onNode(isDialog()).assertExists() + composeTestRule + .onAllNodesWithText("Continue") + .filterToOne(hasAnyAncestor(isDialog())) + .performClick() + composeTestRule.onNode(isDialog()).assertDoesNotExist() verify { viewModel.trySendAction(AboutAction.RateAppClick) } diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/AccountSecurityScreenTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/AccountSecurityScreenTest.kt index ce165a90b..79f993d43 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/AccountSecurityScreenTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/AccountSecurityScreenTest.kt @@ -178,21 +178,37 @@ class AccountSecurityScreenTest : BaseComposeTest() { composeTestRule.onNodeWithText("Vault timeout action").assertIsDisplayed() } + @Suppress("MaxLineLength") @Test - fun `on two-step login click should send TwoStepLoginClick`() { + fun `on two-step login click should display confirmation dialog and confirm click should send TwoStepLoginClick`() { + composeTestRule.onNode(isDialog()).assertDoesNotExist() composeTestRule .onNodeWithText("Two-step login") .performScrollTo() .performClick() + composeTestRule.onNode(isDialog()).assertExists() + composeTestRule + .onAllNodesWithText("Continue") + .filterToOne(hasAnyAncestor(isDialog())) + .performClick() + composeTestRule.onNode(isDialog()).assertDoesNotExist() verify { viewModel.trySendAction(AccountSecurityAction.TwoStepLoginClick) } } + @Suppress("MaxLineLength") @Test - fun `on change master password click should send ChangeMasterPasswordClick`() { + fun `on change master password click should display confirmation dialog and confirm should send ChangeMasterPasswordClick`() { + composeTestRule.onNode(isDialog()).assertDoesNotExist() composeTestRule .onNodeWithText("Change master password") .performScrollTo() .performClick() + composeTestRule.onNode(isDialog()).assertExists() + composeTestRule + .onAllNodesWithText("Continue") + .filterToOne(hasAnyAncestor(isDialog())) + .performClick() + composeTestRule.onNode(isDialog()).assertDoesNotExist() verify { viewModel.trySendAction(AccountSecurityAction.ChangeMasterPasswordClick) } }