From 3b0532f7fd94af8e06c84f03a49b63e72e775602 Mon Sep 17 00:00:00 2001 From: David Perez Date: Tue, 21 Nov 2023 11:48:37 -0600 Subject: [PATCH] Minor updates to the text fields (#266) --- .../components/BitwardenPasswordField.kt | 106 ++++++++---------- .../BitwardenPasswordFieldWithActions.kt | 3 + .../platform/components/BitwardenTextField.kt | 57 ++++------ .../BitwardenTextFieldWithActions.kt | 3 + .../environment/EnvironmentScreenTest.kt | 25 ++++- .../tools/feature/send/NewSendScreenTest.kt | 48 ++++++-- 6 files changed, 136 insertions(+), 106 deletions(-) diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/BitwardenPasswordField.kt b/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/BitwardenPasswordField.kt index bc7d67d5be..b7c18f3a20 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/BitwardenPasswordField.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/BitwardenPasswordField.kt @@ -2,11 +2,6 @@ package com.x8bit.bitwarden.ui.platform.components import androidx.annotation.DrawableRes import androidx.annotation.StringRes -import androidx.compose.foundation.layout.Column -import androidx.compose.foundation.layout.Spacer -import androidx.compose.foundation.layout.fillMaxWidth -import androidx.compose.foundation.layout.height -import androidx.compose.foundation.layout.padding import androidx.compose.foundation.text.KeyboardOptions import androidx.compose.material3.Icon import androidx.compose.material3.IconButton @@ -31,7 +26,6 @@ import androidx.compose.ui.text.input.KeyboardType import androidx.compose.ui.text.input.PasswordVisualTransformation import androidx.compose.ui.text.input.VisualTransformation import androidx.compose.ui.tooling.preview.Preview -import androidx.compose.ui.unit.dp import com.x8bit.bitwarden.R /** @@ -45,6 +39,7 @@ import com.x8bit.bitwarden.R * @param showPasswordChange Lambda that is called when user request show/hide be toggled. * @param onValueChange Callback that is triggered when the password changes. * @param modifier Modifier for the composable. + * @param readOnly `true` if the input should be read-only and not accept user interactions. * @param hint optional hint text that will appear below the text input. * @param showPasswordTestTag The test tag to be used on the show password button (testing tool). * @param autoFocus When set to true, the view will request focus after the first recomposition. @@ -58,66 +53,56 @@ fun BitwardenPasswordField( showPasswordChange: (Boolean) -> Unit, onValueChange: (String) -> Unit, modifier: Modifier = Modifier, + readOnly: Boolean = false, hint: String? = null, showPasswordTestTag: String? = null, autoFocus: Boolean = false, ) { val focusRequester = remember { FocusRequester() } - Column( - modifier = modifier, - ) { - OutlinedTextField( - modifier = Modifier - .fillMaxWidth() - .focusRequester(focusRequester), - textStyle = MaterialTheme.typography.bodyLarge, - label = { Text(text = label) }, - value = value, - onValueChange = onValueChange, - visualTransformation = if (showPassword) { - VisualTransformation.None - } else { - PasswordVisualTransformation() - }, - singleLine = true, - keyboardOptions = KeyboardOptions(keyboardType = KeyboardType.Password), - trailingIcon = { - IconButton( - onClick = { showPasswordChange.invoke(!showPassword) }, - ) { - @DrawableRes - val painterRes = if (showPassword) { - R.drawable.ic_visibility_off - } else { - R.drawable.ic_visibility - } - - @StringRes - val contentDescriptionRes = if (showPassword) R.string.hide else R.string.show - Icon( - modifier = Modifier.semantics { showPasswordTestTag?.let { testTag = it } }, - painter = painterResource(id = painterRes), - contentDescription = stringResource(id = contentDescriptionRes), - tint = MaterialTheme.colorScheme.onSurfaceVariant, - ) + OutlinedTextField( + modifier = modifier.focusRequester(focusRequester), + textStyle = MaterialTheme.typography.bodyLarge, + label = { Text(text = label) }, + value = value, + onValueChange = onValueChange, + visualTransformation = if (showPassword) { + VisualTransformation.None + } else { + PasswordVisualTransformation() + }, + singleLine = true, + readOnly = readOnly, + keyboardOptions = KeyboardOptions(keyboardType = KeyboardType.Password), + supportingText = { + hint?.let { + Text( + text = hint, + style = MaterialTheme.typography.bodySmall, + ) + } + }, + trailingIcon = { + IconButton( + onClick = { showPasswordChange.invoke(!showPassword) }, + ) { + @DrawableRes + val painterRes = if (showPassword) { + R.drawable.ic_visibility_off + } else { + R.drawable.ic_visibility } - }, - ) - hint?.let { - Spacer( - modifier = Modifier.height(4.dp), - ) - Text( - text = hint, - style = MaterialTheme.typography.bodySmall, - color = MaterialTheme.colorScheme.onSurfaceVariant, - modifier = Modifier - .fillMaxWidth() - .padding(horizontal = 16.dp), - ) - } - } + @StringRes + val contentDescriptionRes = if (showPassword) R.string.hide else R.string.show + Icon( + modifier = Modifier.semantics { showPasswordTestTag?.let { testTag = it } }, + painter = painterResource(id = painterRes), + contentDescription = stringResource(id = contentDescriptionRes), + tint = MaterialTheme.colorScheme.onSurfaceVariant, + ) + } + }, + ) if (autoFocus) { LaunchedEffect(Unit) { focusRequester.requestFocus() } } @@ -131,6 +116,7 @@ fun BitwardenPasswordField( * @param value Current next on the text field. * @param onValueChange Callback that is triggered when the password changes. * @param modifier Modifier for the composable. + * @param readOnly `true` if the input should be read-only and not accept user interactions. * @param hint optional hint text that will appear below the text input. * @param initialShowPassword The initial state of the show/hide password control. A value of * `false` (the default) indicates that that password should begin in the hidden state. @@ -144,6 +130,7 @@ fun BitwardenPasswordField( value: String, onValueChange: (String) -> Unit, modifier: Modifier = Modifier, + readOnly: Boolean = false, hint: String? = null, initialShowPassword: Boolean = false, showPasswordTestTag: String? = null, @@ -157,6 +144,7 @@ fun BitwardenPasswordField( showPassword = showPassword, showPasswordChange = { showPassword = !showPassword }, onValueChange = onValueChange, + readOnly = readOnly, hint = hint, showPasswordTestTag = showPasswordTestTag, autoFocus = autoFocus, diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/BitwardenPasswordFieldWithActions.kt b/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/BitwardenPasswordFieldWithActions.kt index b766aa1526..0983191c1c 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/BitwardenPasswordFieldWithActions.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/BitwardenPasswordFieldWithActions.kt @@ -24,6 +24,7 @@ import com.x8bit.bitwarden.ui.platform.theme.BitwardenTheme * @param value Current next on the text field. * @param onValueChange Callback that is triggered when the password changes. * @param modifier Modifier for the composable. + * @param readOnly `true` if the input should be read-only and not accept user interactions. * @param actions A lambda containing the set of actions (usually icons or similar) to display * in the app bar's trailing side. This lambda extends [RowScope], allowing flexibility in * defining the layout of the actions. @@ -34,6 +35,7 @@ fun BitwardenPasswordFieldWithActions( value: String, onValueChange: (String) -> Unit, modifier: Modifier = Modifier, + readOnly: Boolean = false, actions: @Composable RowScope.() -> Unit = {}, ) { Row( @@ -44,6 +46,7 @@ fun BitwardenPasswordFieldWithActions( label = label, value = value, onValueChange = onValueChange, + readOnly = readOnly, modifier = Modifier .weight(1f) .padding(end = 8.dp), diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/BitwardenTextField.kt b/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/BitwardenTextField.kt index cb82b75276..8f03a6354e 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/BitwardenTextField.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/BitwardenTextField.kt @@ -1,10 +1,5 @@ package com.x8bit.bitwarden.ui.platform.components -import androidx.compose.foundation.layout.Column -import androidx.compose.foundation.layout.Spacer -import androidx.compose.foundation.layout.fillMaxWidth -import androidx.compose.foundation.layout.height -import androidx.compose.foundation.layout.padding import androidx.compose.foundation.text.KeyboardOptions import androidx.compose.material3.MaterialTheme import androidx.compose.material3.OutlinedTextField @@ -13,7 +8,6 @@ import androidx.compose.runtime.Composable import androidx.compose.ui.Modifier import androidx.compose.ui.text.input.KeyboardType import androidx.compose.ui.tooling.preview.Preview -import androidx.compose.ui.unit.dp /** * Component that allows the user to input text. This composable will manage the state of @@ -26,6 +20,7 @@ import androidx.compose.ui.unit.dp * the [value] is empty. * @param hint optional hint text that will appear below the text input. * @param readOnly `true` if the input should be read-only and not accept user interactions. + * @param enabled Whether or not the text field is enabled. * @param keyboardType the preferred type of keyboard input. */ @Composable @@ -37,38 +32,30 @@ fun BitwardenTextField( placeholder: String? = null, hint: String? = null, readOnly: Boolean = false, + enabled: Boolean = true, keyboardType: KeyboardType = KeyboardType.Text, ) { - Column( + OutlinedTextField( modifier = modifier, - ) { - OutlinedTextField( - modifier = Modifier.fillMaxWidth(), - label = { Text(text = label) }, - value = value, - placeholder = placeholder?.let { - { Text(text = it) } - }, - onValueChange = onValueChange, - singleLine = true, - readOnly = readOnly, - keyboardOptions = KeyboardOptions.Default.copy(keyboardType = keyboardType), - ) - - hint?.let { - Spacer( - modifier = Modifier.height(4.dp), - ) - Text( - text = hint, - style = MaterialTheme.typography.bodySmall, - color = MaterialTheme.colorScheme.onSurfaceVariant, - modifier = Modifier - .fillMaxWidth() - .padding(horizontal = 16.dp), - ) - } - } + enabled = enabled, + label = { Text(text = label) }, + value = value, + placeholder = placeholder?.let { + { Text(text = it) } + }, + supportingText = { + hint?.let { + Text( + text = hint, + style = MaterialTheme.typography.bodySmall, + ) + } + }, + onValueChange = onValueChange, + singleLine = true, + readOnly = readOnly, + keyboardOptions = KeyboardOptions.Default.copy(keyboardType = keyboardType), + ) } @Preview diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/BitwardenTextFieldWithActions.kt b/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/BitwardenTextFieldWithActions.kt index a45a07176e..87331d2aa3 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/BitwardenTextFieldWithActions.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/BitwardenTextFieldWithActions.kt @@ -22,6 +22,7 @@ import com.x8bit.bitwarden.ui.platform.theme.BitwardenTheme * @param value Current text in the text field. * @param onValueChange Callback that is triggered when the text content changes. * @param modifier [Modifier] applied to this layout composable. + * @param readOnly `true` if the input should be read-only and not accept user interactions. * @param actions A lambda containing the set of actions (usually icons or similar) to display * next to the text field. This lambda extends [RowScope], * providing flexibility in the layout definition. @@ -32,6 +33,7 @@ fun BitwardenTextFieldWithActions( value: String, onValueChange: (String) -> Unit, modifier: Modifier = Modifier, + readOnly: Boolean = false, keyboardType: KeyboardType = KeyboardType.Text, actions: @Composable RowScope.() -> Unit = {}, ) { @@ -44,6 +46,7 @@ fun BitwardenTextFieldWithActions( .weight(1f), label = label, value = value, + readOnly = readOnly, onValueChange = onValueChange, keyboardType = keyboardType, ) diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/environment/EnvironmentScreenTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/environment/EnvironmentScreenTest.kt index 46f443993c..5d56d7d5ec 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/environment/EnvironmentScreenTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/environment/EnvironmentScreenTest.kt @@ -114,13 +114,22 @@ class EnvironmentScreenTest : BaseComposeTest() { .onNodeWithText("Server URL") // Click to focus to see placeholder .performClick() - .assertTextEquals("Server URL", "ex. https://bitwarden.company.com", "") + .assertTextEquals( + "Server URL", + "Specify the base URL of your on-premise hosted Bitwarden installation.", + "ex. https://bitwarden.company.com", + "", + ) mutableStateFlow.update { it.copy(serverUrl = "server-url") } composeTestRule .onNodeWithText("Server URL") - .assertTextEquals("Server URL", "server-url") + .assertTextEquals( + "Server URL", + "Specify the base URL of your on-premise hosted Bitwarden installation.", + "server-url", + ) } @Test @@ -216,13 +225,21 @@ class EnvironmentScreenTest : BaseComposeTest() { fun `icons server URL should change according to the state`() { composeTestRule .onNodeWithText("Icons server URL") - .assertTextEquals("Icons server URL", "") + .assertTextEquals( + "Icons server URL", + "For advanced users. You can specify the base URL of each service independently.", + "", + ) mutableStateFlow.update { it.copy(iconsServerUrl = "icons-url") } composeTestRule .onNodeWithText("Icons server URL") - .assertTextEquals("Icons server URL", "icons-url") + .assertTextEquals( + "Icons server URL", + "For advanced users. You can specify the base URL of each service independently.", + "icons-url", + ) } @Test diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/send/NewSendScreenTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/send/NewSendScreenTest.kt index 5b7ce926ec..5586969d81 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/send/NewSendScreenTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/send/NewSendScreenTest.kt @@ -78,14 +78,22 @@ class NewSendScreenTest : BaseComposeTest() { fun `name input should change according to the state`() { composeTestRule .onNodeWithText("Name") - .assertTextEquals("Name", "") + .assertTextEquals( + "Name", + "A friendly name to describe this Send.", + "", + ) mutableStateFlow.update { it.copy(name = "input") } composeTestRule .onNodeWithText("Name") - .assertTextEquals("Name", "input") + .assertTextEquals( + "Name", + "A friendly name to describe this Send.", + "input", + ) } @Test @@ -129,7 +137,11 @@ class NewSendScreenTest : BaseComposeTest() { composeTestRule .onAllNodesWithText("Text") .filterToOne(hasSetTextAction()) - .assertTextEquals("Text", "") + .assertTextEquals( + "Text", + "The text you want to send.", + "", + ) mutableStateFlow.update { it.copy( @@ -142,7 +154,11 @@ class NewSendScreenTest : BaseComposeTest() { composeTestRule .onAllNodesWithText("Text") .filterToOne(hasSetTextAction()) - .assertTextEquals("Text", "input") + .assertTextEquals( + "Text", + "The text you want to send.", + "input", + ) } @Test @@ -311,7 +327,11 @@ class NewSendScreenTest : BaseComposeTest() { .performClick() composeTestRule .onNodeWithText("New password") - .assertTextEquals("New password", "") + .assertTextEquals( + "New password", + "Optionally require a password for users to access this Send.", + "", + ) mutableStateFlow.update { it.copy( @@ -320,7 +340,11 @@ class NewSendScreenTest : BaseComposeTest() { } composeTestRule .onNodeWithText("New password") - .assertTextEquals("New password", "•••••") + .assertTextEquals( + "New password", + "Optionally require a password for users to access this Send.", + "•••••", + ) } @Test @@ -346,7 +370,11 @@ class NewSendScreenTest : BaseComposeTest() { .performClick() composeTestRule .onNodeWithText("Notes") - .assertTextEquals("Notes", "") + .assertTextEquals( + "Notes", + "Private notes about this Send.", + "", + ) mutableStateFlow.update { it.copy( @@ -355,7 +383,11 @@ class NewSendScreenTest : BaseComposeTest() { } composeTestRule .onNodeWithText("Notes") - .assertTextEquals("Notes", "input") + .assertTextEquals( + "Notes", + "Private notes about this Send.", + "input", + ) } @Test