From 85e750cc08e9acdc2640b8eb4f154cbdd86ec142 Mon Sep 17 00:00:00 2001 From: joshua-livefront <139182194+joshua-livefront@users.noreply.github.com> Date: Thu, 30 Nov 2023 11:51:59 -0500 Subject: [PATCH] BIT-660: Adding ui for catch all email (#303) --- .../components/BitwardenMultiSelectButton.kt | 11 +++ .../feature/generator/GeneratorScreen.kt | 91 ++++++++++++++++--- .../feature/generator/GeneratorViewModel.kt | 65 ++++++++++++- .../feature/generator/GeneratorScreenTest.kt | 35 +++++++ .../generator/GeneratorViewModelTest.kt | 59 ++++++++++++ 5 files changed, 245 insertions(+), 16 deletions(-) diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/BitwardenMultiSelectButton.kt b/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/BitwardenMultiSelectButton.kt index 9abcdf641..c84c7adc5 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/BitwardenMultiSelectButton.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/BitwardenMultiSelectButton.kt @@ -38,6 +38,7 @@ import com.x8bit.bitwarden.ui.platform.theme.BitwardenTheme * @param onOptionSelected A lambda that is invoked when an option * is selected from the dropdown menu. * @param modifier A [Modifier] that you can use to apply custom modifications to the composable. + * @param supportingText A optional supporting text that will appear below the text field. */ @Suppress("LongMethod") @Composable @@ -47,6 +48,7 @@ fun BitwardenMultiSelectButton( selectedOption: String, onOptionSelected: (String) -> Unit, modifier: Modifier = Modifier, + supportingText: String? = null, ) { var shouldShowDialog by remember { mutableStateOf(false) } @@ -93,7 +95,16 @@ fun BitwardenMultiSelectButton( disabledTrailingIconColor = MaterialTheme.colorScheme.onSurfaceVariant, disabledLabelColor = MaterialTheme.colorScheme.onSurfaceVariant, disabledPlaceholderColor = MaterialTheme.colorScheme.onSurfaceVariant, + disabledSupportingTextColor = MaterialTheme.colorScheme.onSurfaceVariant, ), + supportingText = supportingText?.let { + { + Text( + text = supportingText, + style = MaterialTheme.typography.bodySmall, + ) + } + }, ) if (shouldShowDialog) { BitwardenSelectionDialog( diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorScreen.kt b/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorScreen.kt index 3dcebdd36..95b22ae4a 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorScreen.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorScreen.kt @@ -16,7 +16,6 @@ import androidx.compose.foundation.rememberScrollState import androidx.compose.foundation.text.KeyboardOptions import androidx.compose.foundation.verticalScroll import androidx.compose.material3.ExperimentalMaterial3Api -import androidx.compose.material3.MaterialTheme import androidx.compose.material3.OutlinedTextField import androidx.compose.material3.Slider import androidx.compose.material3.SnackbarDuration @@ -146,6 +145,10 @@ fun GeneratorScreen( PlusAddressedEmailHandlers.create(viewModel = viewModel) } + val catchAllEmailHandlers = remember(viewModel) { + CatchAllEmailHandlers.create(viewModel = viewModel) + } + val scrollBehavior = TopAppBarDefaults.exitUntilCollapsedScrollBehavior(rememberTopAppBarState()) @@ -174,6 +177,7 @@ fun GeneratorScreen( passwordHandlers = passwordHandlers, passphraseHandlers = passphraseHandlers, plusAddressedEmailHandlers = plusAddressedEmailHandlers, + catchAllEmailHandlers = catchAllEmailHandlers, modifier = Modifier.padding(innerPadding), ) } @@ -193,6 +197,7 @@ private fun ScrollContent( passwordHandlers: PasswordHandlers, passphraseHandlers: PassphraseHandlers, plusAddressedEmailHandlers: PlusAddressedEmailHandlers, + catchAllEmailHandlers: CatchAllEmailHandlers, modifier: Modifier = Modifier, ) { Column( @@ -241,6 +246,7 @@ private fun ScrollContent( usernameState = selectedType, onSubStateOptionClicked = onUsernameSubStateOptionClicked, plusAddressedEmailHandlers = plusAddressedEmailHandlers, + catchAllEmailHandlers = catchAllEmailHandlers, ) } } @@ -695,6 +701,7 @@ private fun ColumnScope.UsernameTypeItems( usernameState: GeneratorState.MainType.Username, onSubStateOptionClicked: (GeneratorState.MainType.Username.UsernameTypeOption) -> Unit, plusAddressedEmailHandlers: PlusAddressedEmailHandlers, + catchAllEmailHandlers: CatchAllEmailHandlers, ) { UsernameOptionsItem(usernameState, onSubStateOptionClicked) @@ -711,7 +718,10 @@ private fun ColumnScope.UsernameTypeItems( } is GeneratorState.MainType.Username.UsernameType.CatchAllEmail -> { - // TODO: Implement CatchAllEmail BIT-656 + CatchAllEmailTypeContent( + usernameTypeState = selectedType, + catchAllEmailHandlers = catchAllEmailHandlers, + ) } is GeneratorState.MainType.Username.UsernameType.RandomWord -> { @@ -740,6 +750,9 @@ private fun UsernameOptionsItem( modifier = Modifier .padding(horizontal = 16.dp) .fillMaxWidth(), + supportingText = currentSubState.selectedType.supportingStringResId?.let { + stringResource(id = it) + }, ) } @@ -752,17 +765,6 @@ private fun ColumnScope.PlusAddressedEmailTypeContent( usernameTypeState: GeneratorState.MainType.Username.UsernameType.PlusAddressedEmail, plusAddressedEmailHandlers: PlusAddressedEmailHandlers, ) { - Spacer(modifier = Modifier.height(4.dp)) - - Text( - text = stringResource(id = R.string.plus_addressed_email_description), - style = MaterialTheme.typography.bodySmall, - color = MaterialTheme.colorScheme.onSurfaceVariant, - modifier = Modifier - .fillMaxWidth() - .padding(horizontal = 32.dp), - ) - Spacer(modifier = Modifier.height(8.dp)) PlusAddressedEmailTextInputItem( @@ -790,6 +792,40 @@ private fun PlusAddressedEmailTextInputItem( //endregion PlusAddressedEmailType Composables +//region CatchAllEmailType Composables + +@Composable +private fun ColumnScope.CatchAllEmailTypeContent( + usernameTypeState: GeneratorState.MainType.Username.UsernameType.CatchAllEmail, + catchAllEmailHandlers: CatchAllEmailHandlers, +) { + Spacer(modifier = Modifier.height(8.dp)) + + CatchAllEmailTextInputItem( + domain = usernameTypeState.domainName, + onDomainTextChange = catchAllEmailHandlers.onDomainChange, + ) +} + +@Composable +private fun CatchAllEmailTextInputItem( + domain: String, + onDomainTextChange: (domain: String) -> Unit, +) { + BitwardenTextField( + label = stringResource(id = R.string.domain_name_required_parenthesis), + value = domain, + onValueChange = { + onDomainTextChange(it) + }, + modifier = Modifier + .fillMaxWidth() + .padding(horizontal = 16.dp), + ) +} + +//endregion CatchAllEmailType Composables + @Preview(showBackground = true) @Composable private fun GeneratorPreview() { @@ -966,3 +1002,32 @@ private class PlusAddressedEmailHandlers( } } } + +/** + * A class dedicated to handling user interactions related to plus addressed email + * configuration. + * Each lambda corresponds to a specific user action, allowing for easy delegation of + * logic when user input is detected. + */ +private class CatchAllEmailHandlers( + val onDomainChange: (String) -> Unit, +) { + companion object { + fun create(viewModel: GeneratorViewModel): CatchAllEmailHandlers { + return CatchAllEmailHandlers( + onDomainChange = { newDomain -> + viewModel.trySendAction( + GeneratorAction + .MainType + .Username + .UsernameType + .CatchAllEmail + .DomainTextChange( + domain = newDomain, + ), + ) + }, + ) + } + } +} diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorViewModel.kt b/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorViewModel.kt index 6354e6207..dc1a59e89 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorViewModel.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorViewModel.kt @@ -22,6 +22,7 @@ import com.x8bit.bitwarden.ui.tools.feature.generator.GeneratorState.MainType.Pa import com.x8bit.bitwarden.ui.tools.feature.generator.GeneratorState.MainType.Username import com.x8bit.bitwarden.ui.tools.feature.generator.GeneratorState.MainType.Username.UsernameType.ForwardedEmailAlias.ServiceType.AnonAddy import com.x8bit.bitwarden.ui.tools.feature.generator.GeneratorState.MainType.Username.UsernameType.PlusAddressedEmail +import com.x8bit.bitwarden.ui.tools.feature.generator.GeneratorState.MainType.Username.UsernameType.CatchAllEmail import dagger.hilt.android.lifecycle.HiltViewModel import kotlinx.coroutines.Job import kotlinx.coroutines.flow.launchIn @@ -104,6 +105,10 @@ class GeneratorViewModel @Inject constructor( { handlePlusAddressedEmailTextInputChange(action) } + + is GeneratorAction.MainType.Username.UsernameType.CatchAllEmail.DomainTextChange -> { + handleCatchAllEmailTextInputChange(action) + } } } @@ -540,6 +545,19 @@ class GeneratorViewModel @Inject constructor( //endregion Plus Addressed Email Specific Handlers + //region Catch-All Email Specific Handlers + + private fun handleCatchAllEmailTextInputChange( + action: GeneratorAction.MainType.Username.UsernameType.CatchAllEmail.DomainTextChange, + ) { + updateCatchAllEmailType { catchAllEmailType -> + val newDomain = action.domain + catchAllEmailType.copy(domainName = newDomain) + } + } + + //endregion Catch-All Email Specific Handlers + //region Utility Functions private inline fun updateGeneratorMainType( @@ -623,6 +641,18 @@ class GeneratorViewModel @Inject constructor( } } + private inline fun updateCatchAllEmailType( + crossinline block: (CatchAllEmail) -> CatchAllEmail, + ) { + updateGeneratorMainTypeUsername { currentSelectedType -> + val currentUsernameType = currentSelectedType.selectedType + if (currentUsernameType !is CatchAllEmail) { + return@updateGeneratorMainTypeUsername currentSelectedType + } + currentSelectedType.copy(selectedType = block(currentUsernameType)) + } + } + //endregion Utility Functions companion object { @@ -821,12 +851,16 @@ data class GeneratorState( /** * Represents the resource ID for the display string specific to each - * PasscodeType subclass. Every subclass of UsernameType must override - * this property to provide the appropriate string resource ID for - * its display string. + * UsernameType subclass. */ abstract val displayStringResId: Int + /** + * Represents the resource ID for the supporting display string specific to each + * UsernameType subclass. + */ + abstract val supportingStringResId: Int? + /** * Represents a PlusAddressedEmail type. * @@ -838,6 +872,9 @@ data class GeneratorState( ) : UsernameType(), Parcelable { override val displayStringResId: Int get() = UsernameTypeOption.PLUS_ADDRESSED_EMAIL.labelRes + + override val supportingStringResId: Int + get() = R.string.plus_addressed_email_description } /** @@ -852,6 +889,9 @@ data class GeneratorState( ) : UsernameType(), Parcelable { override val displayStringResId: Int get() = UsernameTypeOption.CATCH_ALL_EMAIL.labelRes + + override val supportingStringResId: Int + get() = R.string.catch_all_email_description } /** @@ -868,6 +908,9 @@ data class GeneratorState( ) : UsernameType(), Parcelable { override val displayStringResId: Int get() = UsernameTypeOption.RANDOM_WORD.labelRes + + override val supportingStringResId: Int? + get() = null } /** @@ -883,6 +926,9 @@ data class GeneratorState( override val displayStringResId: Int get() = UsernameTypeOption.FORWARDED_EMAIL_ALIAS.labelRes + override val supportingStringResId: Int + get() = R.string.forwarded_email_description + /** * Enum representing the types of services, * allowing for different service configurations. @@ -1203,6 +1249,19 @@ sealed class GeneratorAction { */ data class EmailTextChange(val email: String) : PlusAddressedEmail() } + + /** + * Represents actions specifically related to Catch-All Email. + */ + sealed class CatchAllEmail : UsernameType() { + + /** + * Fired when the domain text input is changed. + * + * @property domain The new domain text. + */ + data class DomainTextChange(val domain: String) : CatchAllEmail() + } } } } diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorScreenTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorScreenTest.kt index b25062ae6..753d6fc71 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorScreenTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorScreenTest.kt @@ -993,6 +993,41 @@ class GeneratorScreenTest : BaseComposeTest() { } } + @Suppress("MaxLineLength") + @Test + fun `in Username_CatchAllEmail state, updating text in email field should send EmailTextChange action`() { + updateState( + GeneratorState( + generatedText = "Placeholder", + selectedType = GeneratorState.MainType.Username( + GeneratorState.MainType.Username.UsernameType.CatchAllEmail( + domainName = "", + ), + ), + ), + ) + + composeTestRule.setContent { + GeneratorScreen(viewModel = viewModel) + } + + val newDomain = "test.com" + + // Find the text field for Catch-All Email and input text + composeTestRule + .onNodeWithText("Domain name (required)") + .performScrollTo() + .performTextInput(newDomain) + + verify { + viewModel.trySendAction( + GeneratorAction.MainType.Username.UsernameType.CatchAllEmail.DomainTextChange( + domain = newDomain, + ), + ) + } + } + //endregion Username Plus Addressed Email Tests private fun updateState(state: GeneratorState) { diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorViewModelTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorViewModelTest.kt index 853b076a3..20dcd6ca9 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorViewModelTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorViewModelTest.kt @@ -27,6 +27,10 @@ class GeneratorViewModelTest : BaseViewModelTest() { private val initialUsernameState = createPlusAddressedEmailState() private val usernameSavedStateHandle = createSavedStateHandleWithState(initialUsernameState) + private val initialCatchAllEmailState = createCatchAllEmailState() + private val catchAllEmailSavedStateHandle = + createSavedStateHandleWithState(initialCatchAllEmailState) + private val fakeGeneratorRepository = FakeGeneratorRepository().apply { setMockGeneratePasswordResult( GeneratedPasswordResult.Success("defaultPassword"), @@ -826,6 +830,48 @@ class GeneratorViewModelTest : BaseViewModelTest() { assertEquals(expectedState, viewModel.stateFlow.value) } } + + @Nested + inner class CatchAllEmailActions { + private val defaultCatchAllEmailState = createCatchAllEmailState() + private lateinit var viewModel: GeneratorViewModel + + @BeforeEach + fun setup() { + viewModel = GeneratorViewModel(catchAllEmailSavedStateHandle, fakeGeneratorRepository) + } + + @Suppress("MaxLineLength") + @Test + fun `DomainTextChange should update domain correctly`() = + runTest { + val newDomain = "test.com" + viewModel.actionChannel.trySend( + GeneratorAction + .MainType + .Username + .UsernameType + .CatchAllEmail + .DomainTextChange( + domain = newDomain, + ), + ) + + val expectedState = defaultCatchAllEmailState.copy( + selectedType = GeneratorState.MainType.Username( + selectedType = GeneratorState + .MainType + .Username + .UsernameType + .CatchAllEmail( + domainName = newDomain, + ), + ), + ) + + assertEquals(expectedState, viewModel.stateFlow.value) + } + } //region Helper Functions @Suppress("LongParameterList") @@ -888,6 +934,19 @@ class GeneratorViewModelTest : BaseViewModelTest() { ), ) + private fun createCatchAllEmailState( + generatedText: String = "defaultCatchAllEmail", + domain: String = "defaultDomain", + ): GeneratorState = + GeneratorState( + generatedText = generatedText, + selectedType = GeneratorState.MainType.Username( + GeneratorState.MainType.Username.UsernameType.CatchAllEmail( + domainName = domain, + ), + ), + ) + private fun createSavedStateHandleWithState(state: GeneratorState) = SavedStateHandle().apply { set("state", state)