From 83c057af292439d964f42f3761730af8e1209b02 Mon Sep 17 00:00:00 2001 From: joshua-livefront <139182194+joshua-livefront@users.noreply.github.com> Date: Tue, 17 Oct 2023 10:27:06 -0400 Subject: [PATCH] BIT-870: Updating the BitwardenWideSwitch ripple (#119) --- .../components/BitwardenMultiSelectButton.kt | 6 +- .../BitwardenTextFieldWithTwoIcons.kt | 12 ++-- .../components/BitwardenWideSwitch.kt | 18 +++-- .../feature/generator/GeneratorScreen.kt | 24 ++++++- .../feature/generator/GeneratorScreenTest.kt | 68 ++++++++++--------- 5 files changed, 78 insertions(+), 50 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 5bec201e1..57d696116 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 @@ -67,11 +67,11 @@ fun BitwardenMultiSelectButton( Box( modifier = modifier - .semantics(mergeDescendants = true) {} - .fillMaxWidth()) { + .semantics(mergeDescendants = true) {}, + ) { OutlinedTextField( // TODO: Update with final accessibility reading (BIT-752) - modifier = modifier + modifier = Modifier .clearAndSetSemantics { this.role = Role.DropdownList contentDescription = "$label, $selectedOption" diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/BitwardenTextFieldWithTwoIcons.kt b/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/BitwardenTextFieldWithTwoIcons.kt index aa2aafeda..3cc3fb7ef 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/BitwardenTextFieldWithTwoIcons.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/BitwardenTextFieldWithTwoIcons.kt @@ -47,13 +47,13 @@ fun BitwardenTextFieldWithTwoIcons( onSecondIconClick: () -> Unit, modifier: Modifier = Modifier, ) { - Box(modifier = Modifier.semantics(mergeDescendants = true) {}) { + Box(modifier = modifier.semantics(mergeDescendants = true) {}) { Row( - modifier = modifier.fillMaxWidth(), + modifier = Modifier.fillMaxWidth(), verticalAlignment = Alignment.CenterVertically, ) { OutlinedTextField( - modifier = modifier + modifier = Modifier .weight(1f) .clearAndSetSemantics { contentDescription = "$label, $value" @@ -70,7 +70,6 @@ fun BitwardenTextFieldWithTwoIcons( }, ) RowOfIconButtons( - modifier = modifier, firstIconResource = firstIconResource, onFirstIconClick = onFirstIconClick, secondIconResource = secondIconResource, @@ -83,7 +82,6 @@ fun BitwardenTextFieldWithTwoIcons( /** * A row of two customizable icon buttons. * - * @param modifier Modifier for the Row. * @param firstIconResource The resource data for the first icon button. * @param onFirstIconClick Callback for when the first icon button is clicked. * @param secondIconResource The resource data for the second icon button. @@ -91,14 +89,13 @@ fun BitwardenTextFieldWithTwoIcons( */ @Composable private fun RowOfIconButtons( - modifier: Modifier, firstIconResource: IconResource, onFirstIconClick: () -> Unit, secondIconResource: IconResource, onSecondIconClick: () -> Unit, ) { Row( - modifier = modifier.padding(start = 8.dp), + modifier = Modifier.padding(start = 8.dp), horizontalArrangement = Arrangement.spacedBy(8.dp), verticalAlignment = Alignment.CenterVertically, ) { @@ -126,7 +123,6 @@ private fun IconButtonWithResource(iconRes: IconResource, onClick: () -> Unit) { Icon( painter = iconRes.iconPainter, contentDescription = iconRes.contentDescription, - modifier = Modifier.padding(8.dp), ) } } diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/BitwardenWideSwitch.kt b/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/BitwardenWideSwitch.kt index 5515f7dfb..2a8e342d5 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/BitwardenWideSwitch.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/BitwardenWideSwitch.kt @@ -1,14 +1,18 @@ package com.x8bit.bitwarden.ui.platform.components +import androidx.compose.foundation.clickable +import androidx.compose.foundation.interaction.MutableInteractionSource import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.height import androidx.compose.foundation.layout.wrapContentHeight +import androidx.compose.material.ripple.rememberRipple import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Switch import androidx.compose.material3.Text import androidx.compose.runtime.Composable +import androidx.compose.runtime.remember import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.semantics.semantics @@ -34,10 +38,16 @@ fun BitwardenWideSwitch( Row( horizontalArrangement = Arrangement.SpaceBetween, verticalAlignment = Alignment.CenterVertically, - modifier = modifier - .semantics(mergeDescendants = true) { } + modifier = Modifier .fillMaxWidth() - .wrapContentHeight(), + .wrapContentHeight() + .clickable( + interactionSource = remember { MutableInteractionSource() }, + indication = rememberRipple(color = MaterialTheme.colorScheme.primary), + onClick = { onCheckedChange?.invoke(!isChecked) }, + ) + .semantics(mergeDescendants = true) { } + .then(modifier), ) { Text( text = label, @@ -49,7 +59,7 @@ fun BitwardenWideSwitch( modifier = Modifier .height(56.dp), checked = isChecked, - onCheckedChange = onCheckedChange, + onCheckedChange = null, ) } } 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 6660f4a0a..e5d652ec9 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 @@ -141,7 +141,6 @@ private fun ScrollContent( modifier = modifier .fillMaxHeight() .background(color = MaterialTheme.colorScheme.surface) - .padding(start = 16.dp, end = 16.dp) .verticalScroll(rememberScrollState()), ) { @@ -165,6 +164,7 @@ private fun ScrollContent( text = stringResource(id = R.string.options), style = MaterialTheme.typography.labelMedium, color = MaterialTheme.colorScheme.onSurfaceVariant, + modifier = Modifier.padding(horizontal = 16.dp), ) } @@ -204,6 +204,7 @@ private fun GeneratedStringItem( contentDescription = stringResource(id = R.string.generate_password), ), onSecondIconClick = onRegenerateClick, + modifier = Modifier.padding(horizontal = 16.dp), ) } @@ -225,6 +226,9 @@ private fun MainStateOptionsItem( optionsWithStrings.entries.first { it.value == selectedOption }.key onMainStateOptionClicked(selectedOptionId) }, + modifier = Modifier + .padding(horizontal = 16.dp) + .fillMaxWidth(), ) } @@ -276,6 +280,9 @@ private fun PasscodeOptionsItem( optionsWithStrings.entries.first { it.value == selectedOption }.key onSubStateOptionClicked(selectedOptionId) }, + modifier = Modifier + .padding(horizontal = 16.dp) + .fillMaxWidth(), ) } @@ -344,6 +351,7 @@ private fun PasswordLengthSliderItem( verticalAlignment = Alignment.CenterVertically, modifier = Modifier .fillMaxWidth() + .padding(horizontal = 16.dp) .semantics(mergeDescendants = true) {}, ) { OutlinedTextField( @@ -383,6 +391,7 @@ private fun PasswordCapitalLettersToggleItem( label = stringResource(id = R.string.uppercase_ato_z), isChecked = useCapitals, onCheckedChange = onPasswordToggleCapitalLettersChange, + modifier = Modifier.padding(horizontal = 16.dp), ) } @@ -395,6 +404,7 @@ private fun PasswordLowercaseLettersToggleItem( label = stringResource(id = R.string.lowercase_ato_z), isChecked = useLowercase, onCheckedChange = onPasswordToggleLowercaseLettersChange, + modifier = Modifier.padding(horizontal = 16.dp), ) } @@ -407,6 +417,7 @@ private fun PasswordNumbersToggleItem( label = stringResource(id = R.string.numbers_zero_to_nine), isChecked = useNumbers, onCheckedChange = onPasswordToggleNumbersChange, + modifier = Modifier.padding(horizontal = 16.dp), ) } @@ -419,6 +430,7 @@ private fun PasswordSpecialCharactersToggleItem( label = stringResource(id = R.string.special_characters), isChecked = useSpecialChars, onCheckedChange = onPasswordToggleSpecialCharactersChange, + modifier = Modifier.padding(horizontal = 16.dp), ) } @@ -444,6 +456,7 @@ private fun PasswordMinNumbersCounterItem( onSecondIconClick = { onPasswordMinNumbersCounterChange(minNumbers + 1) }, + modifier = Modifier.padding(horizontal = 16.dp), ) } @@ -469,6 +482,7 @@ private fun PasswordMinSpecialCharactersCounterItem( onSecondIconClick = { onPasswordMinSpecialCharactersChange(minSpecial + 1) }, + modifier = Modifier.padding(horizontal = 16.dp), ) } @@ -481,6 +495,7 @@ private fun PasswordAvoidAmbiguousCharsToggleItem( label = stringResource(id = R.string.avoid_ambiguous_characters), isChecked = avoidAmbiguousChars, onCheckedChange = onPasswordToggleAvoidAmbiguousCharsChange, + modifier = Modifier.padding(horizontal = 16.dp), ) } @@ -541,6 +556,7 @@ private fun PassphraseNumWordsCounterItem( onSecondIconClick = { onPassphraseNumWordsCounterChange(numWords + 1) }, + modifier = Modifier.padding(horizontal = 16.dp), ) } @@ -555,7 +571,9 @@ private fun PassphraseWordSeparatorInputItem( onValueChange = { onPassphraseWordSeparatorChange(it.toCharArray().firstOrNull()) }, - modifier = Modifier.width(267.dp), + modifier = Modifier + .width(267.dp) + .padding(horizontal = 16.dp), ) } @@ -568,6 +586,7 @@ private fun PassphraseCapitalizeToggleItem( label = stringResource(id = R.string.capitalize), isChecked = capitalize, onCheckedChange = onPassphraseCapitalizeToggleChange, + modifier = Modifier.padding(horizontal = 16.dp), ) } @@ -580,6 +599,7 @@ private fun PassphraseIncludeNumberToggleItem( label = stringResource(id = R.string.include_number), isChecked = includeNumber, onCheckedChange = onPassphraseIncludeNumberToggleChange, + modifier = Modifier.padding(horizontal = 16.dp), ) } 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 613a8247e..757f505f5 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 @@ -3,13 +3,9 @@ package com.x8bit.bitwarden.ui.tools.feature.generator import androidx.compose.ui.semantics.ProgressBarRangeInfo -import androidx.compose.ui.semantics.Role.Companion.Switch import androidx.compose.ui.semantics.SemanticsProperties -import androidx.compose.ui.semantics.SemanticsProperties.Role import androidx.compose.ui.test.SemanticsMatcher.Companion.expectValue 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.hasContentDescription import androidx.compose.ui.test.hasProgressBarRangeInfo @@ -46,6 +42,32 @@ class GeneratorScreenTest : BaseComposeTest() { every { stateFlow } returns mutableStateFlow } + @Test + fun `clicking the Regenerate button should send RegenerateClick action`() { + composeTestRule.setContent { + GeneratorScreen(viewModel = viewModel) + } + + composeTestRule.onNodeWithContentDescription(label = "Generate password").performClick() + + verify { + viewModel.trySendAction(GeneratorAction.RegenerateClick) + } + } + + @Test + fun `clicking the Copy button should send CopyClick action`() { + composeTestRule.setContent { + GeneratorScreen(viewModel = viewModel) + } + + composeTestRule.onNodeWithContentDescription(label = "Copy").performClick() + + verify { + viewModel.trySendAction(GeneratorAction.CopyClick) + } + } + @Test fun `clicking a MainStateOption should send MainTypeOptionSelect action`() { composeTestRule.setContent { @@ -115,27 +137,23 @@ class GeneratorScreenTest : BaseComposeTest() { composeTestRule .onNodeWithText("Uppercase (A to Z)") - .onChildren() - .filterToOne(expectValue(Role, Switch)) - .assertIsOn() + .performScrollTo() + .assertIsDisplayed() composeTestRule .onNodeWithText("Lowercase (A to Z)") - .onChildren() - .filterToOne(expectValue(Role, Switch)) - .assertIsOn() + .performScrollTo() + .assertIsDisplayed() composeTestRule .onNodeWithText("Numbers (0 to 9)") - .onChildren() - .filterToOne(expectValue(Role, Switch)) - .assertIsOn() + .performScrollTo() + .assertIsDisplayed() composeTestRule .onNodeWithText("Special characters (!@#$%^*)") - .onChildren() - .filterToOne(expectValue(Role, Switch)) - .assertIsOff() + .performScrollTo() + .assertIsDisplayed() composeTestRule .onNodeWithContentDescription("Minimum numbers, 1") @@ -153,10 +171,8 @@ class GeneratorScreenTest : BaseComposeTest() { composeTestRule .onNodeWithText("Avoid ambiguous characters") - .onChildren() - .filterToOne(expectValue(Role, Switch)) .performScrollTo() - .assertIsOff() + .assertIsDisplayed() } @Test @@ -198,8 +214,6 @@ class GeneratorScreenTest : BaseComposeTest() { } composeTestRule.onNodeWithText("Uppercase (A to Z)") - .onChildren() - .filterToOne(expectValue(Role, Switch)) .performScrollTo() .performClick() @@ -219,8 +233,6 @@ class GeneratorScreenTest : BaseComposeTest() { } composeTestRule.onNodeWithText("Lowercase (A to Z)") - .onChildren() - .filterToOne(expectValue(Role, Switch)) .performScrollTo() .performClick() @@ -240,8 +252,6 @@ class GeneratorScreenTest : BaseComposeTest() { } composeTestRule.onNodeWithText("Numbers (0 to 9)") - .onChildren() - .filterToOne(expectValue(Role, Switch)) .performScrollTo() .performClick() @@ -261,8 +271,6 @@ class GeneratorScreenTest : BaseComposeTest() { } composeTestRule.onNodeWithText("Special characters (!@#$%^*)") - .onChildren() - .filterToOne(expectValue(Role, Switch)) .performScrollTo() .performClick() @@ -398,8 +406,6 @@ class GeneratorScreenTest : BaseComposeTest() { } composeTestRule.onNodeWithText("Avoid ambiguous characters") - .onChildren() - .filterToOne(expectValue(Role, Switch)) .performScrollTo() .performClick() @@ -492,8 +498,6 @@ class GeneratorScreenTest : BaseComposeTest() { composeTestRule .onNodeWithText("Capitalize") - .onChildren() - .filterToOne(expectValue(Role, Switch)) .performScrollTo() .performClick() @@ -520,8 +524,6 @@ class GeneratorScreenTest : BaseComposeTest() { } composeTestRule.onNodeWithText("Include number") - .onChildren() - .filterToOne(expectValue(Role, Switch)) .performScrollTo() .performClick()