[PM-31393] Sends: UI/UX inconsistency of the password field (#6435)

This commit is contained in:
Shamim Shahrier Emon 2026-02-02 23:58:19 +06:00 committed by GitHub
parent 12eb42097c
commit 74d45c3906
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 321 additions and 2 deletions

View File

@ -217,6 +217,7 @@ fun NavGraphBuilder.vaultUnlockedGraph(
addEditSendDestination(
onNavigateBack = { navController.popBackStack() },
onNavigateUpToSearchOrRoot = { navController.navigateUpToSearchOrVaultUnlockedRoot() },
onNavigateToGeneratorModal = { navController.navigateToGeneratorModal(mode = it) },
)
viewSendDestination(
onNavigateBack = { navController.popBackStack() },

View File

@ -37,6 +37,7 @@ import com.bitwarden.ui.platform.base.util.standardHorizontalMargin
import com.bitwarden.ui.platform.components.animation.AnimateNullableContentVisibility
import com.bitwarden.ui.platform.components.button.BitwardenOutlinedButton
import com.bitwarden.ui.platform.components.button.BitwardenOutlinedErrorButton
import com.bitwarden.ui.platform.components.button.BitwardenStandardIconButton
import com.bitwarden.ui.platform.components.card.BitwardenInfoCalloutCard
import com.bitwarden.ui.platform.components.dialog.BitwardenTwoButtonDialog
import com.bitwarden.ui.platform.components.field.BitwardenPasswordField
@ -367,6 +368,7 @@ private fun AddEditSendOptions(
addSendHandlers: AddEditSendHandlers,
) {
var isExpanded by rememberSaveable { mutableStateOf(false) }
var shouldShowDialog by rememberSaveable { mutableStateOf(false) }
BitwardenExpandingHeader(
isExpanded = isExpanded,
onClick = { isExpanded = !isExpanded },
@ -437,7 +439,47 @@ private fun AddEditSendOptions(
modifier = Modifier
.fillMaxWidth()
.standardHorizontalMargin(),
)
) {
BitwardenStandardIconButton(
vectorIconRes = BitwardenDrawable.ic_generate,
contentDescription = stringResource(id = BitwardenString.generate_password),
onClick = {
if (state.common.passwordInput.isEmpty()) {
addSendHandlers.onOpenPasswordGeneratorClick()
} else {
shouldShowDialog = true
}
},
modifier = Modifier.testTag(tag = "RegeneratePasswordButton"),
)
BitwardenStandardIconButton(
vectorIconRes = BitwardenDrawable.ic_copy,
contentDescription = stringResource(id = BitwardenString.copy_password),
isEnabled = state.common.passwordInput.isNotEmpty(),
onClick = {
addSendHandlers.onPasswordCopyClick(state.common.passwordInput)
},
modifier = Modifier.testTag(tag = "CopyPasswordButton"),
)
}
if (shouldShowDialog) {
BitwardenTwoButtonDialog(
title = stringResource(id = BitwardenString.password),
message = stringResource(id = BitwardenString.password_override_alert),
confirmButtonText = stringResource(id = BitwardenString.yes),
dismissButtonText = stringResource(id = BitwardenString.no),
onConfirmClick = {
shouldShowDialog = false
addSendHandlers.onOpenPasswordGeneratorClick()
},
onDismissClick = {
shouldShowDialog = false
},
onDismissRequest = {
shouldShowDialog = false
},
)
}
Spacer(modifier = Modifier.height(height = 8.dp))
BitwardenSwitch(
modifier = Modifier

View File

@ -6,6 +6,7 @@ import androidx.navigation.NavGraphBuilder
import androidx.navigation.NavOptions
import androidx.navigation.toRoute
import com.bitwarden.ui.platform.base.util.composableWithSlideTransitions
import com.x8bit.bitwarden.ui.tools.feature.generator.model.GeneratorMode
import com.x8bit.bitwarden.ui.tools.feature.send.addedit.model.AddEditSendType
import com.x8bit.bitwarden.ui.tools.feature.send.model.SendItemType
import kotlinx.serialization.Serializable
@ -57,11 +58,13 @@ fun SavedStateHandle.toAddEditSendArgs(): AddEditSendArgs {
fun NavGraphBuilder.addEditSendDestination(
onNavigateBack: () -> Unit,
onNavigateUpToSearchOrRoot: () -> Unit,
onNavigateToGeneratorModal: (GeneratorMode.Modal) -> Unit,
) {
composableWithSlideTransitions<AddEditSendRoute> {
AddEditSendScreen(
onNavigateBack = onNavigateBack,
onNavigateUpToSearchOrRoot = onNavigateUpToSearchOrRoot,
onNavigateToGeneratorModal = onNavigateToGeneratorModal,
)
}
}

View File

@ -38,6 +38,7 @@ import com.bitwarden.ui.platform.resource.BitwardenDrawable
import com.bitwarden.ui.platform.resource.BitwardenString
import com.x8bit.bitwarden.ui.platform.composition.LocalPermissionsManager
import com.x8bit.bitwarden.ui.platform.manager.permissions.PermissionsManager
import com.x8bit.bitwarden.ui.tools.feature.generator.model.GeneratorMode
import com.x8bit.bitwarden.ui.tools.feature.send.addedit.handlers.AddEditSendHandlers
/**
@ -53,6 +54,7 @@ fun AddEditSendScreen(
permissionsManager: PermissionsManager = LocalPermissionsManager.current,
onNavigateBack: () -> Unit,
onNavigateUpToSearchOrRoot: () -> Unit,
onNavigateToGeneratorModal: (GeneratorMode.Modal) -> Unit,
) {
val state by viewModel.stateFlow.collectAsStateWithLifecycle()
val addSendHandlers = remember(viewModel) { AddEditSendHandlers.create(viewModel) }
@ -88,6 +90,10 @@ fun AddEditSendScreen(
}
is AddEditSendEvent.ShowSnackbar -> snackbarHostState.showSnackbar(event.data)
is AddEditSendEvent.NavigateToGeneratorModal -> {
onNavigateToGeneratorModal(event.generatorMode)
}
}
}

View File

@ -26,12 +26,15 @@ import com.x8bit.bitwarden.data.platform.manager.clipboard.BitwardenClipboardMan
import com.x8bit.bitwarden.data.platform.manager.network.NetworkConnectionManager
import com.x8bit.bitwarden.data.platform.manager.util.getActivePolicies
import com.x8bit.bitwarden.data.platform.repository.EnvironmentRepository
import com.x8bit.bitwarden.data.tools.generator.repository.GeneratorRepository
import com.x8bit.bitwarden.data.tools.generator.repository.model.GeneratorResult
import com.x8bit.bitwarden.data.vault.repository.VaultRepository
import com.x8bit.bitwarden.data.vault.repository.model.CreateSendResult
import com.x8bit.bitwarden.data.vault.repository.model.DeleteSendResult
import com.x8bit.bitwarden.data.vault.repository.model.RemovePasswordSendResult
import com.x8bit.bitwarden.data.vault.repository.model.UpdateSendResult
import com.x8bit.bitwarden.ui.platform.model.SnackbarRelay
import com.x8bit.bitwarden.ui.tools.feature.generator.model.GeneratorMode
import com.x8bit.bitwarden.ui.tools.feature.send.addedit.model.AddEditSendType
import com.x8bit.bitwarden.ui.tools.feature.send.addedit.util.shouldFinishOnComplete
import com.x8bit.bitwarden.ui.tools.feature.send.addedit.util.toSendName
@ -41,6 +44,7 @@ import com.x8bit.bitwarden.ui.tools.feature.send.addedit.util.toViewState
import com.x8bit.bitwarden.ui.tools.feature.send.model.SendItemType
import com.x8bit.bitwarden.ui.tools.feature.send.util.toSendUrl
import dagger.hilt.android.lifecycle.HiltViewModel
import kotlinx.coroutines.flow.first
import kotlinx.coroutines.flow.launchIn
import kotlinx.coroutines.flow.map
import kotlinx.coroutines.flow.onEach
@ -62,11 +66,12 @@ private const val MAX_FILE_SIZE_BYTES: Long = 100 * 1024 * 1024
/**
* View model for the add/edit send screen.
*/
@Suppress("TooManyFunctions", "LongParameterList")
@Suppress("TooManyFunctions", "LongParameterList", "LargeClass")
@HiltViewModel
class AddEditSendViewModel @Inject constructor(
savedStateHandle: SavedStateHandle,
authRepo: AuthRepository,
generatorRepository: GeneratorRepository,
private val clock: Clock,
private val clipboardManager: BitwardenClipboardManager,
private val environmentRepo: EnvironmentRepository,
@ -151,6 +156,18 @@ class AddEditSendViewModel @Inject constructor(
.launchIn(viewModelScope)
}
}
generatorRepository
.generatorResultFlow
.map { result ->
// Wait until we have a Content screen to update
mutableStateFlow.first {
it.viewState is AddEditSendState.ViewState.Content
}
AddEditSendAction.Internal.GeneratorResultReceive(generatorResult = result)
}
.onEach(::sendAction)
.launchIn(viewModelScope)
}
override fun handleAction(action: AddEditSendAction): Unit = when (action) {
@ -173,6 +190,10 @@ class AddEditSendViewModel @Inject constructor(
is AddEditSendAction.DeactivateThisSendToggle -> handleDeactivateThisSendToggle(action)
is AddEditSendAction.HideMyEmailToggle -> handleHideMyEmailToggle(action)
is AddEditSendAction.Internal -> handleInternalAction(action)
is AddEditSendAction.OpenPasswordGeneratorClick -> handleAddEditOpenPasswordGeneratorClick()
is AddEditSendAction.PasswordCopyClick -> {
handleCopyClick(password = action.password)
}
}
private fun handleInternalAction(action: AddEditSendAction.Internal): Unit = when (action) {
@ -193,6 +214,17 @@ class AddEditSendViewModel @Inject constructor(
}
is AddEditSendAction.Internal.SendDataReceive -> handleSendDataReceive(action)
is AddEditSendAction.Internal.GeneratorResultReceive -> {
handleGeneratorResultReceive(action)
}
}
private fun handleCopyClick(password: String) {
clipboardManager.setText(
text = password,
toastDescriptorOverride = BitwardenString.password.asText(),
)
}
private fun handleCreateSendResultReceive(
@ -319,6 +351,16 @@ class AddEditSendViewModel @Inject constructor(
}
}
private fun handleGeneratorResultReceive(
action: AddEditSendAction.Internal.GeneratorResultReceive,
) {
(action.generatorResult as? GeneratorResult.Password)?.let { passwordData ->
updateCommonContent {
it.copy(passwordInput = passwordData.password)
}
}
}
@Suppress("LongMethod")
private fun handleSendDataReceive(action: AddEditSendAction.Internal.SendDataReceive) {
when (val sendDataState = action.sendDataState) {
@ -619,6 +661,10 @@ class AddEditSendViewModel @Inject constructor(
}
}
private fun handleAddEditOpenPasswordGeneratorClick() {
sendEvent(event = AddEditSendEvent.NavigateToGeneratorModal(GeneratorMode.Modal.Password))
}
private fun navigateBack(isDeleted: Boolean = false) {
specialCircumstanceManager.specialCircumstance = null
sendEvent(
@ -856,6 +902,13 @@ sealed class AddEditSendEvent {
*/
data object NavigateBack : AddEditSendEvent()
/**
* Navigate to the generator modal.
*/
data class NavigateToGeneratorModal(
val generatorMode: GeneratorMode.Modal,
) : AddEditSendEvent()
/**
* Navigate up to the search screen or the root screen depending where you came from.
*/
@ -886,6 +939,11 @@ sealed class AddEditSendEvent {
*/
sealed class AddEditSendAction {
/**
* Represents the action to open the password generator.
*/
data object OpenPasswordGeneratorClick : AddEditSendAction()
/**
* User has chosen a file to be part of the send.
*/
@ -901,6 +959,13 @@ sealed class AddEditSendAction {
*/
data object CopyLinkClick : AddEditSendAction()
/**
* Represents the action triggered when a password copy button is clicked.
*
* @param password The [String] to be copied.
*/
data class PasswordCopyClick(val password: String) : AddEditSendAction()
/**
* User clicked the share link button.
*/
@ -987,6 +1052,13 @@ sealed class AddEditSendAction {
*/
data class CreateSendResultReceive(val result: CreateSendResult) : Internal()
/**
* Indicates that the vault totp code result has been received.
*/
data class GeneratorResultReceive(
val generatorResult: GeneratorResult,
) : Internal()
/**
* Indicates a result for updating a send has been received.
*/

View File

@ -22,6 +22,8 @@ data class AddEditSendHandlers(
val onDeactivateSendToggle: (Boolean) -> Unit,
val onDeletionDateChange: (ZonedDateTime) -> Unit,
val onDeleteClick: () -> Unit,
val onOpenPasswordGeneratorClick: () -> Unit,
val onPasswordCopyClick: (String) -> Unit,
) {
@Suppress("UndocumentedPublicClass")
companion object {
@ -59,6 +61,16 @@ data class AddEditSendHandlers(
viewModel.trySendAction(AddEditSendAction.DeletionDateChange(it))
},
onDeleteClick = { viewModel.trySendAction(AddEditSendAction.DeleteClick) },
onOpenPasswordGeneratorClick = {
viewModel.trySendAction(AddEditSendAction.OpenPasswordGeneratorClick)
},
onPasswordCopyClick = {
viewModel.trySendAction(
AddEditSendAction.PasswordCopyClick(
password = it,
),
)
},
)
}
}

View File

@ -15,6 +15,7 @@ import androidx.compose.ui.test.isDialog
import androidx.compose.ui.test.isPopup
import androidx.compose.ui.test.onAllNodesWithText
import androidx.compose.ui.test.onNodeWithContentDescription
import androidx.compose.ui.test.onNodeWithTag
import androidx.compose.ui.test.onNodeWithText
import androidx.compose.ui.test.performClick
import androidx.compose.ui.test.performScrollTo
@ -29,6 +30,7 @@ import com.bitwarden.ui.util.isEditableText
import com.bitwarden.ui.util.isProgressBar
import com.x8bit.bitwarden.ui.platform.base.BitwardenComposeTest
import com.x8bit.bitwarden.ui.platform.manager.permissions.FakePermissionManager
import com.x8bit.bitwarden.ui.tools.feature.generator.model.GeneratorMode
import com.x8bit.bitwarden.ui.tools.feature.send.addedit.model.AddEditSendType
import com.x8bit.bitwarden.ui.tools.feature.send.model.SendItemType
import io.mockk.every
@ -42,11 +44,13 @@ import kotlinx.coroutines.test.runTest
import org.junit.Assert.assertTrue
import org.junit.Before
import org.junit.Test
import org.junit.jupiter.api.Assertions.assertEquals
import java.time.ZonedDateTime
@Suppress("LargeClass")
class AddEditSendScreenTest : BitwardenComposeTest() {
private var navgatedGeneratorMode: GeneratorMode.Modal? = null
private var onNavigateBackCalled = false
private var onNavigateUpToSearchOrRootCalled = false
@ -75,6 +79,9 @@ class AddEditSendScreenTest : BitwardenComposeTest() {
viewModel = viewModel,
onNavigateBack = { onNavigateBackCalled = true },
onNavigateUpToSearchOrRoot = { onNavigateUpToSearchOrRootCalled = true },
onNavigateToGeneratorModal = { mode ->
navgatedGeneratorMode = mode
},
)
}
}
@ -91,6 +98,17 @@ class AddEditSendScreenTest : BitwardenComposeTest() {
assertTrue(onNavigateUpToSearchOrRootCalled)
}
@Test
fun `on NavigateToGeneratorModal event should call onNavigateToGeneratorModal`() {
val mode = GeneratorMode.Modal.Password
mutableEventFlow.tryEmit(
AddEditSendEvent.NavigateToGeneratorModal(mode),
)
assertEquals(mode, navgatedGeneratorMode)
}
@Test
fun `ExitApp should call exitApplication on ExitManager`() {
mutableEventFlow.tryEmit(AddEditSendEvent.ExitApp)
@ -175,6 +193,104 @@ class AddEditSendScreenTest : BitwardenComposeTest() {
verify { viewModel.trySendAction(AddEditSendAction.SaveClick) }
}
@Test
fun `on copyPassword click should send PasswordCopyClick when passwordInput is not empty`() {
// Expand options section:
composeTestRule
.onNodeWithText("Additional options")
.performScrollTo()
.performClick()
mutableStateFlow.update {
it.copy(
viewState = DEFAULT_VIEW_STATE.copy(
common = DEFAULT_COMMON_STATE.copy(passwordInput = "somePass"),
),
)
}
composeTestRule
.onNodeWithTag(testTag = "CopyPasswordButton")
.performScrollTo()
.performClick()
verify {
viewModel.trySendAction(
AddEditSendAction.PasswordCopyClick(
"somePass",
),
)
}
}
@Test
fun `on copyPassword click should not send PasswordCopyClick when passwordInput is empty`() {
// Expand options section:
composeTestRule
.onNodeWithText("Additional options")
.performScrollTo()
.performClick()
mutableStateFlow.update {
it.copy(
viewState = DEFAULT_VIEW_STATE.copy(
common = DEFAULT_COMMON_STATE.copy(passwordInput = ""),
),
)
}
composeTestRule
.onNodeWithTag(testTag = "CopyPasswordButton")
.performScrollTo()
.performClick()
verify(exactly = 0) {
viewModel.trySendAction(
AddEditSendAction.PasswordCopyClick(password = ""),
)
}
}
@Suppress("MaxLineLength")
@Test
fun `on generatePassword click should send OpenPasswordGeneratorClick when passwordInput is empty`() {
// Expand options section:
composeTestRule
.onNodeWithText("Additional options")
.performScrollTo()
.performClick()
composeTestRule
.onNodeWithTag(testTag = "RegeneratePasswordButton")
.performScrollTo()
.performClick()
verify {
viewModel.trySendAction(
AddEditSendAction.OpenPasswordGeneratorClick,
)
}
}
@Suppress("MaxLineLength")
@Test
fun `on generatePassword click should show Password Overwrite dialog when passwordInput is not empty`() {
// Expand options section:
composeTestRule
.onNodeWithText("Additional options")
.performScrollTo()
.performClick()
mutableStateFlow.update {
it.copy(
viewState = DEFAULT_VIEW_STATE.copy(
common = DEFAULT_COMMON_STATE.copy(passwordInput = "somePass"),
),
)
}
composeTestRule
.onNodeWithTag(testTag = "RegeneratePasswordButton")
.performScrollTo()
.performClick()
composeTestRule
.onNodeWithText("Are you sure you want to overwrite the current password?")
.assertIsDisplayed()
}
@Test
fun `on overflow button click should display overflow menu`() {
mutableStateFlow.value = DEFAULT_STATE.copy(

View File

@ -4,6 +4,7 @@ import android.net.Uri
import androidx.lifecycle.SavedStateHandle
import app.cash.turbine.test
import com.bitwarden.core.data.repository.model.DataState
import com.bitwarden.core.data.repository.util.bufferedMutableSharedFlow
import com.bitwarden.data.repository.model.Environment
import com.bitwarden.network.model.PolicyTypeJson
import com.bitwarden.network.model.createMockPolicy
@ -25,6 +26,8 @@ import com.x8bit.bitwarden.data.platform.manager.clipboard.BitwardenClipboardMan
import com.x8bit.bitwarden.data.platform.manager.model.FirstTimeState
import com.x8bit.bitwarden.data.platform.manager.network.NetworkConnectionManager
import com.x8bit.bitwarden.data.platform.repository.EnvironmentRepository
import com.x8bit.bitwarden.data.tools.generator.repository.GeneratorRepository
import com.x8bit.bitwarden.data.tools.generator.repository.model.GeneratorResult
import com.x8bit.bitwarden.data.vault.datasource.sdk.model.createMockSendView
import com.x8bit.bitwarden.data.vault.repository.VaultRepository
import com.x8bit.bitwarden.data.vault.repository.model.CreateSendResult
@ -32,6 +35,7 @@ import com.x8bit.bitwarden.data.vault.repository.model.DeleteSendResult
import com.x8bit.bitwarden.data.vault.repository.model.RemovePasswordSendResult
import com.x8bit.bitwarden.data.vault.repository.model.UpdateSendResult
import com.x8bit.bitwarden.ui.platform.model.SnackbarRelay
import com.x8bit.bitwarden.ui.tools.feature.generator.model.GeneratorMode
import com.x8bit.bitwarden.ui.tools.feature.send.addedit.model.AddEditSendType
import com.x8bit.bitwarden.ui.tools.feature.send.addedit.util.toSendView
import com.x8bit.bitwarden.ui.tools.feature.send.addedit.util.toViewState
@ -70,10 +74,14 @@ class AddEditSendViewModelTest : BaseViewModelTest() {
private val clipboardManager: BitwardenClipboardManager = mockk {
every { setText(any<String>(), toastDescriptorOverride = any<Text>()) } just runs
}
private val mutableGeneratorResultFlow = bufferedMutableSharedFlow<GeneratorResult>()
private val mutableUserStateFlow = MutableStateFlow<UserState?>(DEFAULT_USER_STATE)
private val authRepository = mockk<AuthRepository> {
every { userStateFlow } returns mutableUserStateFlow
}
private val generatorRepository = mockk<GeneratorRepository> (relaxed = true) {
every { generatorResultFlow } returns mutableGeneratorResultFlow
}
private val environmentRepository: EnvironmentRepository = mockk {
every { environment } returns Environment.Us
}
@ -483,6 +491,35 @@ class AddEditSendViewModelTest : BaseViewModelTest() {
)
}
@Test
fun `RegeneratePassword updates password in viewState`() = runTest {
val generatedPassword = "some-password"
val initialState = DEFAULT_STATE.copy(
viewState = DEFAULT_VIEW_STATE.copy(
common = DEFAULT_COMMON_STATE.copy(passwordInput = ""),
),
)
val viewModel = createViewModel(state = initialState)
viewModel.trySendAction(
AddEditSendAction.Internal.GeneratorResultReceive(
GeneratorResult.Password(password = generatedPassword),
),
)
val expectedState = initialState.copy(
viewState = DEFAULT_VIEW_STATE.copy(
common = DEFAULT_COMMON_STATE.copy(
passwordInput = generatedPassword,
),
),
)
assertEquals(expectedState, viewModel.stateFlow.value)
}
@Test
fun `CopyLinkClick with nonnull sendUrl should copy to clipboard`() {
val sendUrl = "www.test.com/send-stuff"
@ -516,6 +553,20 @@ class AddEditSendViewModelTest : BaseViewModelTest() {
}
}
@Test
fun `OpenPasswordGeneratorClick should emit NavigateToGeneratorModal`() = runTest {
val viewModel = createViewModel()
viewModel.eventFlow.test {
viewModel.trySendAction(AddEditSendAction.OpenPasswordGeneratorClick)
assertEquals(
AddEditSendEvent.NavigateToGeneratorModal(
GeneratorMode.Modal.Password,
),
awaitItem(),
)
}
}
@Test
fun `in add item state, RemovePasswordClick should do nothing`() = runTest {
val viewModel = createViewModel()
@ -993,6 +1044,21 @@ class AddEditSendViewModelTest : BaseViewModelTest() {
}
}
@Test
fun `PasswordCopyClick copies password to clipboard`() = runTest {
val viewModel = createViewModel()
viewModel.trySendAction(
AddEditSendAction.PasswordCopyClick("some-password"),
)
verify(exactly = 1) {
clipboardManager.setText(
text = "some-password",
toastDescriptorOverride = BitwardenString.password.asText(),
)
}
}
private fun createViewModel(
state: AddEditSendState? = null,
addEditSendType: AddEditSendType = AddEditSendType.AddItem,
@ -1015,6 +1081,7 @@ class AddEditSendViewModelTest : BaseViewModelTest() {
policyManager = policyManager,
networkConnectionManager = networkConnectionManager,
snackbarRelayManager = snackbarRelayManager,
generatorRepository = generatorRepository,
)
}