From ea55cbc9149c2f2289e63db900937e573fac79d2 Mon Sep 17 00:00:00 2001 From: Patrick Honkonen <1883101+SaintPatrck@users.noreply.github.com> Date: Mon, 29 Apr 2024 22:19:29 -0400 Subject: [PATCH] Convert digits input to stepper (#70) --- .../feature/edititem/EditItemScreen.kt | 88 +++++++------- .../feature/edititem/EditItemViewModel.kt | 38 +++--- .../feature/edititem/model/EditItemData.kt | 3 +- .../ui/platform/base/util/StringExtensions.kt | 17 +++ .../components/stepper/BitwardenStepper.kt | 110 ++++++++++++++++++ app/src/main/res/drawable/ic_minus.xml | 10 ++ 6 files changed, 203 insertions(+), 63 deletions(-) create mode 100644 app/src/main/kotlin/com/bitwarden/authenticator/ui/platform/components/stepper/BitwardenStepper.kt create mode 100644 app/src/main/res/drawable/ic_minus.xml diff --git a/app/src/main/kotlin/com/bitwarden/authenticator/ui/authenticator/feature/edititem/EditItemScreen.kt b/app/src/main/kotlin/com/bitwarden/authenticator/ui/authenticator/feature/edititem/EditItemScreen.kt index 75117e888a..082ba5c148 100644 --- a/app/src/main/kotlin/com/bitwarden/authenticator/ui/authenticator/feature/edititem/EditItemScreen.kt +++ b/app/src/main/kotlin/com/bitwarden/authenticator/ui/authenticator/feature/edititem/EditItemScreen.kt @@ -35,6 +35,7 @@ import androidx.compose.ui.Modifier import androidx.compose.ui.draw.clip import androidx.compose.ui.input.nestedscroll.nestedScroll import androidx.compose.ui.platform.LocalContext +import androidx.compose.ui.platform.testTag import androidx.compose.ui.res.painterResource import androidx.compose.ui.res.stringResource import androidx.compose.ui.semantics.semantics @@ -61,6 +62,7 @@ import com.bitwarden.authenticator.ui.platform.components.header.BitwardenListHe import com.bitwarden.authenticator.ui.platform.components.icon.BitwardenIcon import com.bitwarden.authenticator.ui.platform.components.model.IconData import com.bitwarden.authenticator.ui.platform.components.scaffold.BitwardenScaffold +import com.bitwarden.authenticator.ui.platform.components.stepper.BitwardenStepper import com.bitwarden.authenticator.ui.platform.theme.DEFAULT_FADE_TRANSITION_TIME_MS import com.bitwarden.authenticator.ui.platform.theme.DEFAULT_STAY_TRANSITION_TIME_MS import kotlinx.collections.immutable.toImmutableList @@ -181,7 +183,7 @@ fun EditItemScreen( ) } }, - onNumberOfDigitsOptionClicked = remember(viewModel) { + onNumberOfDigitsChanged = remember(viewModel) { { viewModel.trySendAction( EditItemAction.NumberOfDigitsOptionClick(it) @@ -217,7 +219,7 @@ fun EditItemContent( onTotpCodeTextChange: (String) -> Unit = {}, onAlgorithmOptionClicked: (AuthenticatorItemAlgorithm) -> Unit = {}, onRefreshPeriodOptionClicked: (AuthenticatorRefreshPeriodOption) -> Unit = {}, - onNumberOfDigitsOptionClicked: (VerificationCodeDigitsOption) -> Unit = {}, + onNumberOfDigitsChanged: (Int) -> Unit = {}, onExpandAdvancedOptionsClicked: () -> Unit = {}, ) { Column(modifier = modifier) { @@ -256,18 +258,18 @@ fun EditItemContent( ) } - item { - Spacer(modifier = Modifier.height(8.dp)) - BitwardenTextField( - modifier = Modifier - .fillMaxWidth() - .padding(horizontal = 16.dp), - label = stringResource(id = R.string.username), - value = viewState.itemData.username.orEmpty(), - onValueChange = onUsernameTextChange, - singleLine = true, - ) - } + item { + Spacer(modifier = Modifier.height(8.dp)) + BitwardenTextField( + modifier = Modifier + .fillMaxWidth() + .padding(horizontal = 16.dp), + label = stringResource(id = R.string.username), + value = viewState.itemData.username.orEmpty(), + onValueChange = onUsernameTextChange, + singleLine = true, + ) + } } Spacer(modifier = Modifier.height(8.dp)) @@ -281,7 +283,7 @@ fun EditItemContent( onAlgorithmOptionClicked = onAlgorithmOptionClicked, onTypeOptionClicked = onTypeOptionClicked, onRefreshPeriodOptionClicked = onRefreshPeriodOptionClicked, - onNumberOfDigitsOptionClicked = onNumberOfDigitsOptionClicked + onNumberOfDigitsChanged = onNumberOfDigitsChanged ) } } @@ -294,7 +296,7 @@ private fun AdvancedOptions( onAlgorithmOptionClicked: (AuthenticatorItemAlgorithm) -> Unit, onTypeOptionClicked: (AuthenticatorItemType) -> Unit, onRefreshPeriodOptionClicked: (AuthenticatorRefreshPeriodOption) -> Unit, - onNumberOfDigitsOptionClicked: (VerificationCodeDigitsOption) -> Unit, + onNumberOfDigitsChanged: (Int) -> Unit, ) { Column(modifier = modifier) { Row( @@ -419,26 +421,12 @@ private fun AdvancedOptions( } item { - val possibleDigitOptions = VerificationCodeDigitsOption.entries - val digitOptionsWithStrings = - possibleDigitOptions.associateWith { it.length.toString() } Spacer(modifier = Modifier.height(8.dp)) - BitwardenMultiSelectButton( - modifier = Modifier - .fillMaxWidth(), - label = stringResource(id = R.string.number_of_digits), - options = digitOptionsWithStrings.values.toImmutableList(), - selectedOption = viewState.itemData.digits.length.toString(), - onOptionSelected = remember(viewState) { - { selectedOption -> - val selectedOptionName = digitOptionsWithStrings - .entries - .first { it.value == selectedOption } - .key - - onNumberOfDigitsOptionClicked(selectedOptionName) - } - } + DigitsCounterItem( + digits = viewState.itemData.digits, + onDigitsCounterChange = onNumberOfDigitsChanged, + minValue = viewState.minDigitsAllowed, + maxValue = viewState.maxDigitsAllowed, ) } } @@ -472,11 +460,30 @@ private fun EditItemDialogs( } } +@Composable +private fun DigitsCounterItem( + digits: Int, + onDigitsCounterChange: (Int) -> Unit, + minValue: Int, + maxValue: Int, +) { + BitwardenStepper( + label = stringResource(id = R.string.number_of_digits), + value = digits.coerceIn(minValue, maxValue), + range = minValue..maxValue, + onValueChange = onDigitsCounterChange, + increaseButtonTestTag = "DigitsIncreaseButton", + decreaseButtonTestTag = "DigitsDecreaseButton", + modifier = Modifier.testTag("DigitsValueLabel"), + ) +} + @Preview(showBackground = true, showSystemUi = true) @Composable private fun EditItemContentExpandedOptionsPreview() { EditItemContent( viewState = EditItemState.ViewState.Content( + isAdvancedOptionsExpanded = true, itemData = EditItemData( refreshPeriod = AuthenticatorRefreshPeriodOption.THIRTY, totpCode = "123456", @@ -484,9 +491,10 @@ private fun EditItemContentExpandedOptionsPreview() { username = "account name", issuer = "issuer", algorithm = AuthenticatorItemAlgorithm.SHA1, - digits = VerificationCodeDigitsOption.SIX + digits = 6 ), - isAdvancedOptionsExpanded = true, + minDigitsAllowed = 5, + maxDigitsAllowed = 10, ) ) } @@ -496,6 +504,7 @@ private fun EditItemContentExpandedOptionsPreview() { private fun EditItemContentCollapsedOptionsPreview() { EditItemContent( viewState = EditItemState.ViewState.Content( + isAdvancedOptionsExpanded = false, itemData = EditItemData( refreshPeriod = AuthenticatorRefreshPeriodOption.THIRTY, totpCode = "123456", @@ -503,9 +512,10 @@ private fun EditItemContentCollapsedOptionsPreview() { username = "account name", issuer = "issuer", algorithm = AuthenticatorItemAlgorithm.SHA1, - digits = VerificationCodeDigitsOption.SIX + digits = 6 ), - isAdvancedOptionsExpanded = false, + minDigitsAllowed = 5, + maxDigitsAllowed = 10, ) ) } diff --git a/app/src/main/kotlin/com/bitwarden/authenticator/ui/authenticator/feature/edititem/EditItemViewModel.kt b/app/src/main/kotlin/com/bitwarden/authenticator/ui/authenticator/feature/edititem/EditItemViewModel.kt index 9db68f1377..8cc67ffc66 100644 --- a/app/src/main/kotlin/com/bitwarden/authenticator/ui/authenticator/feature/edititem/EditItemViewModel.kt +++ b/app/src/main/kotlin/com/bitwarden/authenticator/ui/authenticator/feature/edititem/EditItemViewModel.kt @@ -11,6 +11,8 @@ import com.bitwarden.authenticator.data.authenticator.repository.AuthenticatorRe import com.bitwarden.authenticator.data.authenticator.repository.model.CreateItemResult import com.bitwarden.authenticator.data.platform.repository.model.DataState import com.bitwarden.authenticator.data.platform.repository.util.takeUntilLoaded +import com.bitwarden.authenticator.ui.authenticator.feature.edititem.EditItemState.Companion.MAX_ALLOWED_CODE_DIGITS +import com.bitwarden.authenticator.ui.authenticator.feature.edititem.EditItemState.Companion.MIN_ALLOWED_CODE_DIGITS import com.bitwarden.authenticator.ui.authenticator.feature.edititem.model.EditItemData import com.bitwarden.authenticator.ui.platform.base.BaseViewModel import com.bitwarden.authenticator.ui.platform.base.util.Text @@ -114,7 +116,7 @@ class EditItemViewModel @Inject constructor( type = content.itemData.type, algorithm = content.itemData.algorithm, period = content.itemData.refreshPeriod.seconds, - digits = content.itemData.digits.length, + digits = content.itemData.digits, issuer = content.itemData.issuer.trim(), ) ) @@ -125,7 +127,7 @@ class EditItemViewModel @Inject constructor( private fun handleNumberOfDigitsOptionChange(action: EditItemAction.NumberOfDigitsOptionClick) { updateItemData { currentItemData -> currentItemData.copy( - digits = action.digitsOption + digits = action.digits ) } } @@ -313,6 +315,8 @@ class EditItemViewModel @Inject constructor( isAdvancedOptionsExpanded: Boolean, ) = EditItemState.ViewState.Content( isAdvancedOptionsExpanded = isAdvancedOptionsExpanded, + minDigitsAllowed = MIN_ALLOWED_CODE_DIGITS, + maxDigitsAllowed = MAX_ALLOWED_CODE_DIGITS, itemData = EditItemData( refreshPeriod = AuthenticatorRefreshPeriodOption.fromSeconds(period) ?: AuthenticatorRefreshPeriodOption.THIRTY, @@ -321,9 +325,8 @@ class EditItemViewModel @Inject constructor( username = accountName, issuer = issuer, algorithm = algorithm, - digits = VerificationCodeDigitsOption.fromIntOrNull(digits) - ?: VerificationCodeDigitsOption.SIX, - ) + digits = digits, + ), ) //endregion Utility Functions } @@ -368,6 +371,8 @@ data class EditItemState( @Parcelize data class Content( val isAdvancedOptionsExpanded: Boolean, + val minDigitsAllowed: Int, + val maxDigitsAllowed: Int, val itemData: EditItemData, ) : ViewState() } @@ -394,6 +399,11 @@ data class EditItemState( val message: Text, ) : DialogState() } + + companion object { + const val MIN_ALLOWED_CODE_DIGITS = 5 + const val MAX_ALLOWED_CODE_DIGITS = 10 + } } /** @@ -470,7 +480,7 @@ sealed class EditItemAction { * The user has selected the number of verification code digits. */ data class NumberOfDigitsOptionClick( - val digitsOption: VerificationCodeDigitsOption, + val digits: Int, ) : EditItemAction() data object ExpandAdvancedOptionsClick : EditItemAction() @@ -508,19 +518,3 @@ enum class AuthenticatorRefreshPeriodOption(val seconds: Int) { } } -/** - * Enum class representing valid verification code lengths - */ -enum class VerificationCodeDigitsOption(val length: Int) { - SIX(length = 6), - EIGHT(length = 8), - TEN(length = 10), - TWELVE(length = 12), - ; - - companion object { - fun fromIntOrNull(intValue: Int): VerificationCodeDigitsOption? { - return entries.find { it.length == intValue } - } - } -} diff --git a/app/src/main/kotlin/com/bitwarden/authenticator/ui/authenticator/feature/edititem/model/EditItemData.kt b/app/src/main/kotlin/com/bitwarden/authenticator/ui/authenticator/feature/edititem/model/EditItemData.kt index 16c7928c5d..d03c355830 100644 --- a/app/src/main/kotlin/com/bitwarden/authenticator/ui/authenticator/feature/edititem/model/EditItemData.kt +++ b/app/src/main/kotlin/com/bitwarden/authenticator/ui/authenticator/feature/edititem/model/EditItemData.kt @@ -4,7 +4,6 @@ import android.os.Parcelable import com.bitwarden.authenticator.data.authenticator.datasource.disk.entity.AuthenticatorItemAlgorithm import com.bitwarden.authenticator.data.authenticator.datasource.disk.entity.AuthenticatorItemType import com.bitwarden.authenticator.ui.authenticator.feature.edititem.AuthenticatorRefreshPeriodOption -import com.bitwarden.authenticator.ui.authenticator.feature.edititem.VerificationCodeDigitsOption import kotlinx.parcelize.Parcelize /** @@ -25,5 +24,5 @@ data class EditItemData( val username: String?, val issuer: String, val algorithm: AuthenticatorItemAlgorithm, - val digits: VerificationCodeDigitsOption, + val digits: Int, ) : Parcelable diff --git a/app/src/main/kotlin/com/bitwarden/authenticator/ui/platform/base/util/StringExtensions.kt b/app/src/main/kotlin/com/bitwarden/authenticator/ui/platform/base/util/StringExtensions.kt index 54cf033e98..53c8ea9ead 100644 --- a/app/src/main/kotlin/com/bitwarden/authenticator/ui/platform/base/util/StringExtensions.kt +++ b/app/src/main/kotlin/com/bitwarden/authenticator/ui/platform/base/util/StringExtensions.kt @@ -8,6 +8,23 @@ import androidx.compose.ui.text.rememberTextMeasurer import java.text.Normalizer import kotlin.math.floor +/** + * This character takes up no space but can be used to ensure a string is not empty. It can also + * be used to insert "safe" line-break positions in a string. + * + * Note: Is a string only contains this charactor, it is _not_ considered blank. + */ +const val ZERO_WIDTH_CHARACTER: String = "\u200B" +/** + * Returns the original [String] only if: + * + * - it is non-null + * - it is not blank (where blank refers to empty strings of those containing only white space) + * + * Otherwise `null` is returned. + */ +fun String?.orNullIfBlank(): String? = this?.takeUnless { it.isBlank() } + /** * Returns a new [String] that includes line breaks after [widthPx] worth of text. This is useful * for long values that need to smoothly flow onto the next line without the OS inserting line diff --git a/app/src/main/kotlin/com/bitwarden/authenticator/ui/platform/components/stepper/BitwardenStepper.kt b/app/src/main/kotlin/com/bitwarden/authenticator/ui/platform/components/stepper/BitwardenStepper.kt new file mode 100644 index 0000000000..61ccb328dc --- /dev/null +++ b/app/src/main/kotlin/com/bitwarden/authenticator/ui/platform/components/stepper/BitwardenStepper.kt @@ -0,0 +1,110 @@ +package com.bitwarden.authenticator.ui.platform.components.stepper + +import androidx.compose.runtime.Composable +import androidx.compose.ui.Modifier +import androidx.compose.ui.semantics.semantics +import androidx.compose.ui.semantics.testTag +import androidx.compose.ui.text.input.KeyboardType +import com.bitwarden.authenticator.R +import com.bitwarden.authenticator.ui.platform.base.util.ZERO_WIDTH_CHARACTER +import com.bitwarden.authenticator.ui.platform.base.util.orNullIfBlank +import com.bitwarden.authenticator.ui.platform.components.field.BitwardenTextFieldWithActions +import com.bitwarden.authenticator.ui.platform.components.icon.BitwardenIconButtonWithResource +import com.bitwarden.authenticator.ui.platform.components.model.IconResource +import com.bitwarden.authenticator.ui.platform.components.util.rememberVectorPainter + +/** + * Displays a stepper that allows the user to increment and decrement an int value. + * + * @param label Label for the stepper. + * @param value Value to display. Null will display nothing. Will be clamped to [range] before + * display. + * @param onValueChange callback invoked when the user increments or decrements the count. Note + * that this will not be called if the attempts to move value outside of [range]. + * @param modifier Modifier. + * @param range Range of valid values. + * @param isIncrementEnabled whether or not the increment button should be enabled. + * @param isDecrementEnabled whether or not the decrement button should be enabled. + * @param textFieldReadOnly whether or not the text field should be read only. The stepper + * increment and decrement buttons function regardless of this value. + */ +@Suppress("LongMethod") +@Composable +fun BitwardenStepper( + label: String, + value: Int?, + onValueChange: (Int) -> Unit, + modifier: Modifier = Modifier, + range: ClosedRange = 1..Int.MAX_VALUE, + isIncrementEnabled: Boolean = true, + isDecrementEnabled: Boolean = true, + textFieldReadOnly: Boolean = true, + stepperActionsTestTag: String? = null, + increaseButtonTestTag: String? = null, + decreaseButtonTestTag: String? = null, +) { + val clampedValue = value?.coerceIn(range) + if (clampedValue != value && clampedValue != null) { + onValueChange(clampedValue) + } + BitwardenTextFieldWithActions( + label = label, + // We use the zero width character instead of an empty string to make sure label is shown + // small and above the input + value = clampedValue + ?.toString() + ?: ZERO_WIDTH_CHARACTER, + actionsTestTag = stepperActionsTestTag, + actions = { + BitwardenIconButtonWithResource( + iconRes = IconResource( + iconPainter = rememberVectorPainter(id = R.drawable.ic_minus), + contentDescription = "\u2212", + ), + onClick = { + val decrementedValue = ((value ?: 0) - 1).coerceIn(range) + if (decrementedValue != value) { + onValueChange(decrementedValue) + } + }, + isEnabled = isDecrementEnabled, + modifier = Modifier.semantics { + if (decreaseButtonTestTag != null) { + testTag = decreaseButtonTestTag + } + }, + ) + BitwardenIconButtonWithResource( + iconRes = IconResource( + iconPainter = rememberVectorPainter(id = R.drawable.ic_plus), + contentDescription = "+", + ), + onClick = { + val incrementedValue = ((value ?: 0) + 1).coerceIn(range) + if (incrementedValue != value) { + onValueChange(incrementedValue) + } + }, + isEnabled = isIncrementEnabled, + modifier = Modifier.semantics { + if (increaseButtonTestTag != null) { + testTag = increaseButtonTestTag + } + }, + ) + }, + readOnly = textFieldReadOnly, + keyboardType = KeyboardType.Number, + onValueChange = { newValue -> + onValueChange( + newValue + // Make sure the placeholder is gone, since it will mess up the int conversion + .replace(ZERO_WIDTH_CHARACTER, "") + .orNullIfBlank() + ?.let { it.toIntOrNull()?.coerceIn(range) ?: value } + ?: range.start, + ) + }, + modifier = modifier, + ) +} diff --git a/app/src/main/res/drawable/ic_minus.xml b/app/src/main/res/drawable/ic_minus.xml new file mode 100644 index 0000000000..e48c80009e --- /dev/null +++ b/app/src/main/res/drawable/ic_minus.xml @@ -0,0 +1,10 @@ + + +