[PM-26355] Improve SelectAccountScreen state handling (#5965)

This commit is contained in:
Patrick Honkonen 2025-10-02 17:05:08 -04:00 committed by GitHub
parent 2eb829a25b
commit acc9113f9a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 313 additions and 96 deletions

View File

@ -4,6 +4,7 @@ import androidx.compose.foundation.layout.Spacer
import androidx.compose.foundation.layout.fillMaxSize
import androidx.compose.foundation.layout.fillMaxWidth
import androidx.compose.foundation.layout.height
import androidx.compose.foundation.layout.navigationBarsPadding
import androidx.compose.foundation.lazy.LazyColumn
import androidx.compose.foundation.lazy.itemsIndexed
import androidx.compose.material3.ExperimentalMaterial3Api
@ -18,7 +19,7 @@ import androidx.compose.ui.res.stringResource
import androidx.compose.ui.text.style.TextAlign
import androidx.compose.ui.tooling.preview.Preview
import androidx.compose.ui.unit.dp
import androidx.credentials.providerevents.exception.ImportCredentialsUnknownErrorException
import androidx.credentials.providerevents.exception.ImportCredentialsCancellationException
import androidx.hilt.lifecycle.viewmodel.compose.hiltViewModel
import androidx.lifecycle.compose.collectAsStateWithLifecycle
import com.bitwarden.cxf.manager.CredentialExchangeCompletionManager
@ -27,7 +28,8 @@ import com.bitwarden.cxf.ui.composition.LocalCredentialExchangeCompletionManager
import com.bitwarden.ui.platform.base.util.EventsEffect
import com.bitwarden.ui.platform.base.util.standardHorizontalMargin
import com.bitwarden.ui.platform.base.util.toListItemCardStyle
import com.bitwarden.ui.platform.components.scaffold.BitwardenScaffold
import com.bitwarden.ui.platform.components.content.BitwardenEmptyContent
import com.bitwarden.ui.platform.components.content.BitwardenLoadingContent
import com.bitwarden.ui.platform.components.util.rememberVectorPainter
import com.bitwarden.ui.platform.resource.BitwardenDrawable
import com.bitwarden.ui.platform.resource.BitwardenString
@ -36,6 +38,7 @@ import com.x8bit.bitwarden.ui.vault.feature.exportitems.component.AccountSummary
import com.x8bit.bitwarden.ui.vault.feature.exportitems.component.ExportItemsScaffold
import com.x8bit.bitwarden.ui.vault.feature.exportitems.model.AccountSelectionListItem
import com.x8bit.bitwarden.ui.vault.feature.exportitems.selectaccount.handlers.rememberSelectAccountHandlers
import kotlinx.collections.immutable.ImmutableList
import kotlinx.collections.immutable.persistentListOf
/**
@ -43,6 +46,7 @@ import kotlinx.collections.immutable.persistentListOf
*/
@OptIn(ExperimentalMaterial3Api::class)
@Composable
@Suppress("LongMethod")
fun SelectAccountScreen(
onAccountSelected: (userId: String) -> Unit,
viewModel: SelectAccountViewModel = hiltViewModel(),
@ -57,9 +61,7 @@ fun SelectAccountScreen(
credentialExchangeCompletionManager
.completeCredentialExport(
exportResult = ExportCredentialsResult.Failure(
// TODO: [PM-26094] Use ImportCredentialsCancellationException once
// public.
error = ImportCredentialsUnknownErrorException(
error = ImportCredentialsCancellationException(
errorMessage = "User cancelled import.",
),
),
@ -82,19 +84,40 @@ fun SelectAccountScreen(
.fillMaxSize()
.nestedScroll(scrollBehavior.nestedScrollConnection),
) {
SelectAccountContent(
state = state,
onAccountClick = handlers.onAccountClick,
modifier = Modifier
.fillMaxSize()
.standardHorizontalMargin(),
)
when (val viewState = state.viewState) {
is SelectAccountState.ViewState.Content -> {
SelectAccountContent(
accountSelectionListItems = viewState.accountSelectionListItems,
onAccountClick = handlers.onAccountClick,
modifier = Modifier.fillMaxSize(),
)
}
SelectAccountState.ViewState.Loading -> {
BitwardenLoadingContent(
text = stringResource(BitwardenString.loading),
modifier = Modifier.fillMaxSize(),
)
}
SelectAccountState.ViewState.NoItems -> {
BitwardenEmptyContent(
title = stringResource(BitwardenString.no_accounts_available),
titleTestTag = "NoAccountsTitle",
text = stringResource(
BitwardenString.you_dont_have_any_accounts_you_can_import_from,
),
labelTestTag = "NoAccountsText",
modifier = Modifier.fillMaxSize(),
)
}
}
}
}
@Composable
private fun SelectAccountContent(
state: SelectAccountState,
accountSelectionListItems: ImmutableList<AccountSelectionListItem>,
onAccountClick: (userId: String) -> Unit,
modifier: Modifier = Modifier,
) {
@ -106,62 +129,121 @@ private fun SelectAccountContent(
text = stringResource(BitwardenString.select_account),
textAlign = TextAlign.Center,
style = BitwardenTheme.typography.titleMedium,
modifier = Modifier.fillMaxWidth(),
modifier = Modifier
.fillMaxWidth()
.standardHorizontalMargin(),
)
}
item { Spacer(Modifier.height(16.dp)) }
itemsIndexed(
items = state.accountSelectionListItems,
items = accountSelectionListItems,
key = { _, item -> "AccountSummaryItem_${item.userId}" },
) { index, item ->
AccountSummaryListItem(
item = item,
cardStyle = state.accountSelectionListItems.toListItemCardStyle(index),
cardStyle = accountSelectionListItems.toListItemCardStyle(index),
clickable = true,
onClick = onAccountClick,
modifier = Modifier
.fillMaxWidth()
.standardHorizontalMargin()
.animateItem(),
)
}
item { Spacer(Modifier.height(16.dp)) }
item { Spacer(Modifier.navigationBarsPadding()) }
}
}
@Preview(showBackground = true)
@OptIn(ExperimentalMaterial3Api::class)
@Preview(
showBackground = true,
name = "Select account content",
showSystemUi = true,
)
@Composable
private fun SelectAccountContentPreview() {
val state = SelectAccountState(
accountSelectionListItems = persistentListOf(
AccountSelectionListItem(
userId = "1",
email = "john.doe@example.com",
initials = "JD",
avatarColorHex = "#FFFF0000",
isItemRestricted = false,
),
AccountSelectionListItem(
userId = "2",
email = "jane.smith@example.com",
initials = "JS",
avatarColorHex = "#FF00FF00",
isItemRestricted = true,
),
AccountSelectionListItem(
userId = "3",
email = "another.user@example.com",
initials = "AU",
avatarColorHex = "#FF0000FF",
isItemRestricted = false,
),
),
)
BitwardenScaffold {
private fun SelectAccountContent_preview() {
ExportItemsScaffold(
navIcon = rememberVectorPainter(BitwardenDrawable.ic_close),
navigationIconContentDescription = stringResource(BitwardenString.close),
onNavigationIconClick = { },
scrollBehavior = TopAppBarDefaults.pinnedScrollBehavior(rememberTopAppBarState()),
) {
SelectAccountContent(
state = state,
accountSelectionListItems = persistentListOf(
AccountSelectionListItem(
userId = "1",
email = "john.doe@example.com",
initials = "JD",
avatarColorHex = "#FFFF0000",
isItemRestricted = false,
),
AccountSelectionListItem(
userId = "2",
email = "jane.smith@example.com",
initials = "JS",
avatarColorHex = "#FF00FF00",
isItemRestricted = true,
),
AccountSelectionListItem(
userId = "3",
email = "another.user@example.com",
initials = "AU",
avatarColorHex = "#FF0000FF",
isItemRestricted = false,
),
),
onAccountClick = { },
modifier = Modifier.fillMaxSize(),
)
}
}
@OptIn(ExperimentalMaterial3Api::class)
@Preview(
showBackground = true,
name = "No accounts content",
showSystemUi = true,
)
@Composable
private fun NoAccountsContent_preview() {
ExportItemsScaffold(
navIcon = rememberVectorPainter(BitwardenDrawable.ic_close),
navigationIconContentDescription = stringResource(BitwardenString.close),
onNavigationIconClick = { },
scrollBehavior = TopAppBarDefaults.pinnedScrollBehavior(rememberTopAppBarState()),
) {
BitwardenEmptyContent(
title = stringResource(BitwardenString.no_accounts_available),
titleTestTag = "NoAccountsTitle",
text = stringResource(
BitwardenString.you_dont_have_any_accounts_you_can_import_from,
),
labelTestTag = "NoAccountsText",
modifier = Modifier.fillMaxSize(),
)
}
}
@OptIn(ExperimentalMaterial3Api::class)
@Preview(
showBackground = true,
name = "Loading content",
showSystemUi = true,
)
@Composable
private fun LoadingContent_preview() {
ExportItemsScaffold(
navIcon = rememberVectorPainter(BitwardenDrawable.ic_close),
navigationIconContentDescription = stringResource(BitwardenString.close),
onNavigationIconClick = { },
scrollBehavior = TopAppBarDefaults.pinnedScrollBehavior(rememberTopAppBarState()),
) {
BitwardenLoadingContent(
text = stringResource(BitwardenString.loading),
modifier = Modifier.fillMaxSize(),
)
}
}

View File

@ -1,5 +1,6 @@
package com.x8bit.bitwarden.ui.vault.feature.exportitems.selectaccount
import android.os.Parcelable
import androidx.lifecycle.viewModelScope
import com.bitwarden.network.model.PolicyTypeJson
import com.bitwarden.network.model.SyncResponseJson
@ -11,12 +12,13 @@ import com.x8bit.bitwarden.ui.vault.feature.exportitems.model.AccountSelectionLi
import com.x8bit.bitwarden.ui.vault.feature.vault.util.initials
import dagger.hilt.android.lifecycle.HiltViewModel
import kotlinx.collections.immutable.ImmutableList
import kotlinx.collections.immutable.persistentListOf
import kotlinx.collections.immutable.toImmutableList
import kotlinx.coroutines.flow.combine
import kotlinx.coroutines.flow.launchIn
import kotlinx.coroutines.flow.onEach
import kotlinx.coroutines.flow.update
import kotlinx.parcelize.Parcelize
import kotlinx.serialization.Serializable
import javax.inject.Inject
/**
@ -28,7 +30,7 @@ class SelectAccountViewModel @Inject constructor(
policyManager: PolicyManager,
) : BaseViewModel<SelectAccountState, SelectAccountEvent, SelectAccountAction>(
initialState = SelectAccountState(
accountSelectionListItems = persistentListOf(),
viewState = SelectAccountState.ViewState.Loading,
),
) {
@ -95,30 +97,38 @@ class SelectAccountViewModel @Inject constructor(
.filter { it.isEnabled }
.map { it.organizationId }
val accountSelectionListItems = action.userState
?.accounts
.orEmpty()
// We only want accounts that do not restrict personal vault ownership
.filter { account ->
account
.organizations
.none { org -> org.id in personalOwnershipRestrictedOrgIds }
}
.map { account ->
AccountSelectionListItem(
userId = account.userId,
email = account.email,
initials = account.initials,
avatarColorHex = account.avatarColorHex,
// Indicate which accounts have item restrictions applied.
isItemRestricted = account
.organizations
.any { org -> org.id in itemRestrictedOrgIds },
)
}
.toImmutableList()
mutableStateFlow.update {
it.copy(
accountSelectionListItems = action.userState
?.accounts
.orEmpty()
// We only want accounts that do not restrict personal vault ownership
.filter { account ->
account
.organizations
.none { org -> org.id in personalOwnershipRestrictedOrgIds }
}
.map { account ->
AccountSelectionListItem(
userId = account.userId,
email = account.email,
initials = account.initials,
avatarColorHex = account.avatarColorHex,
// Indicate which accounts have item restrictions applied.
isItemRestricted = account
.organizations
.any { org -> org.id in itemRestrictedOrgIds },
)
}
.toImmutableList(),
viewState = if (accountSelectionListItems.isEmpty()) {
SelectAccountState.ViewState.NoItems
} else {
SelectAccountState.ViewState.Content(
accountSelectionListItems = accountSelectionListItems,
)
},
)
}
}
@ -126,12 +136,40 @@ class SelectAccountViewModel @Inject constructor(
/**
* Represents the state for the select account screen.
*
* @param accountSelectionListItems The list of account summaries to be displayed for selection.
*/
@Parcelize
@Serializable
data class SelectAccountState(
val accountSelectionListItems: ImmutableList<AccountSelectionListItem>,
)
val viewState: ViewState,
) : Parcelable {
/**
* Represents the different states for the select account screen.
*/
@Parcelize
@Serializable
sealed class ViewState : Parcelable {
/**
* Represents the loading state for the select account screen.
*/
data object Loading : ViewState()
/**
* Represents the content state for the select account screen.
*
* @param accountSelectionListItems The list of account summaries to be displayed for
* selection.
*/
data class Content(
val accountSelectionListItems: ImmutableList<AccountSelectionListItem>,
) : ViewState()
/**
* Represents the no items state for the select account screen.
*/
data object NoItems : ViewState()
}
}
/**
* Represents the actions that can be performed on the select account screen.

View File

@ -1,6 +1,7 @@
package com.x8bit.bitwarden.ui.vault.feature.exportitems
import androidx.compose.ui.test.isDisplayed
import androidx.compose.ui.test.isNotDisplayed
import androidx.compose.ui.test.onNodeWithContentDescription
import androidx.compose.ui.test.onNodeWithText
import androidx.compose.ui.test.performClick
@ -124,6 +125,42 @@ class SelectAccountScreenTest : BitwardenComposeTest() {
assertTrue(onAccountSelectedCalled)
}
@Test
fun `NoItemsContent should be displayed according to state`() = runTest {
mockkStateFlow.emit(
DEFAULT_STATE.copy(
viewState = SelectAccountState.ViewState.NoItems,
),
)
composeTestRule
.onNodeWithText("No accounts available")
.isDisplayed()
composeTestRule
.onNodeWithText(
text = "You don't have any accounts you can import from.",
substring = true,
)
.isDisplayed()
composeTestRule
.onNodeWithText("Select an account")
.isNotDisplayed()
}
@Test
fun `Loading content should be displayed according to state`() = runTest {
mockkStateFlow.emit(
DEFAULT_STATE.copy(
viewState = SelectAccountState.ViewState.Loading,
),
)
composeTestRule
.onNodeWithText("Loading")
.isDisplayed()
}
}
private val ACTIVE_ACCOUNT_SUMMARY = AccountSummary(
@ -148,20 +185,22 @@ private val LOCKED_ACCOUNT_SUMMARY = AccountSummary(
)
private val DEFAULT_STATE = SelectAccountState(
accountSelectionListItems = persistentListOf(
AccountSelectionListItem(
userId = ACTIVE_ACCOUNT_SUMMARY.userId,
email = ACTIVE_ACCOUNT_SUMMARY.email,
initials = "AA",
avatarColorHex = "#FFFF0000",
isItemRestricted = false,
),
AccountSelectionListItem(
userId = LOCKED_ACCOUNT_SUMMARY.userId,
email = LOCKED_ACCOUNT_SUMMARY.email,
initials = "LU",
avatarColorHex = "#FF00FF00",
isItemRestricted = false,
viewState = SelectAccountState.ViewState.Content(
accountSelectionListItems = persistentListOf(
AccountSelectionListItem(
userId = ACTIVE_ACCOUNT_SUMMARY.userId,
email = ACTIVE_ACCOUNT_SUMMARY.email,
initials = "AA",
avatarColorHex = "#FFFF0000",
isItemRestricted = false,
),
AccountSelectionListItem(
userId = LOCKED_ACCOUNT_SUMMARY.userId,
email = LOCKED_ACCOUNT_SUMMARY.email,
initials = "LU",
avatarColorHex = "#FF00FF00",
isItemRestricted = false,
),
),
),
)

View File

@ -50,11 +50,8 @@ class SelectAccountViewModelTest : BaseViewModelTest() {
@Test
fun `initial state should be correct`() = runTest {
val viewModel = createViewModel()
assertEquals(
SelectAccountState(
accountSelectionListItems = persistentListOf(),
),
SelectAccountState(viewState = SelectAccountState.ViewState.Loading),
viewModel.stateFlow.value,
)
}
@ -79,7 +76,11 @@ class SelectAccountViewModelTest : BaseViewModelTest() {
)
assertEquals(
SelectAccountState(accountSelectionListItems = persistentListOf(expectedItem)),
SelectAccountState(
viewState = SelectAccountState.ViewState.Content(
accountSelectionListItems = persistentListOf(expectedItem),
),
),
viewModel.stateFlow.value,
)
}
@ -116,9 +117,7 @@ class SelectAccountViewModelTest : BaseViewModelTest() {
),
)
assertEquals(
SelectAccountState(
accountSelectionListItems = persistentListOf(),
),
SelectAccountState(viewState = SelectAccountState.ViewState.NoItems),
viewModel.stateFlow.value,
)
}
@ -163,7 +162,11 @@ class SelectAccountViewModelTest : BaseViewModelTest() {
),
)
assertEquals(
SelectAccountState(accountSelectionListItems = persistentListOf(expectedItem)),
SelectAccountState(
viewState = SelectAccountState.ViewState.Content(
accountSelectionListItems = persistentListOf(expectedItem),
),
),
viewModel.stateFlow.value,
)
}

View File

@ -3,6 +3,7 @@ package com.bitwarden.ui.platform.components.content
import androidx.compose.foundation.layout.Arrangement
import androidx.compose.foundation.layout.Column
import androidx.compose.foundation.layout.Spacer
import androidx.compose.foundation.layout.fillMaxSize
import androidx.compose.foundation.layout.fillMaxWidth
import androidx.compose.foundation.layout.height
import androidx.compose.foundation.layout.navigationBarsPadding
@ -12,11 +13,14 @@ import androidx.compose.runtime.Composable
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.text.style.TextAlign
import androidx.compose.ui.tooling.preview.Preview
import androidx.compose.ui.unit.dp
import com.bitwarden.ui.platform.base.util.nullableTestTag
import com.bitwarden.ui.platform.base.util.standardHorizontalMargin
import com.bitwarden.ui.platform.components.icon.BitwardenIcon
import com.bitwarden.ui.platform.components.icon.model.IconData
import com.bitwarden.ui.platform.components.scaffold.BitwardenScaffold
import com.bitwarden.ui.platform.resource.BitwardenDrawable
import com.bitwarden.ui.platform.theme.BitwardenTheme
/**
@ -28,6 +32,8 @@ fun BitwardenEmptyContent(
modifier: Modifier = Modifier,
illustrationData: IconData? = null,
labelTestTag: String? = null,
title: String? = null,
titleTestTag: String? = null,
) {
Column(
modifier = modifier,
@ -41,6 +47,19 @@ fun BitwardenEmptyContent(
)
Spacer(modifier = Modifier.height(height = 24.dp))
}
title?.let {
Text(
text = title,
style = BitwardenTheme.typography.titleMedium,
color = BitwardenTheme.colorScheme.text.primary,
textAlign = TextAlign.Center,
modifier = Modifier
.fillMaxWidth()
.standardHorizontalMargin()
.nullableTestTag(tag = titleTestTag),
)
Spacer(modifier = Modifier.height(height = 8.dp))
}
Text(
text = text,
style = BitwardenTheme.typography.bodyMedium,
@ -54,3 +73,20 @@ fun BitwardenEmptyContent(
Spacer(modifier = Modifier.navigationBarsPadding())
}
}
@Preview(showBackground = true, name = "Bitwarden empty content")
@Composable
private fun BitwardenEmptyContent_preview() {
BitwardenScaffold {
BitwardenEmptyContent(
title = "Empty content",
titleTestTag = "TitleTestTag",
text = "There is no content to display",
labelTestTag = "EmptyContentLabel",
illustrationData = IconData.Local(BitwardenDrawable.ic_empty_vault),
modifier = Modifier
.fillMaxSize()
.standardHorizontalMargin(),
)
}
}

View File

@ -3,6 +3,7 @@ package com.bitwarden.ui.platform.components.content
import androidx.compose.foundation.layout.Arrangement
import androidx.compose.foundation.layout.Column
import androidx.compose.foundation.layout.Spacer
import androidx.compose.foundation.layout.fillMaxSize
import androidx.compose.foundation.layout.height
import androidx.compose.foundation.layout.navigationBarsPadding
import androidx.compose.foundation.layout.size
@ -11,8 +12,11 @@ import androidx.compose.runtime.Composable
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.platform.testTag
import androidx.compose.ui.tooling.preview.Preview
import androidx.compose.ui.unit.dp
import com.bitwarden.ui.platform.base.util.standardHorizontalMargin
import com.bitwarden.ui.platform.components.indicator.BitwardenCircularProgressIndicator
import com.bitwarden.ui.platform.components.scaffold.BitwardenScaffold
import com.bitwarden.ui.platform.theme.BitwardenTheme
/**
@ -46,3 +50,16 @@ fun BitwardenLoadingContent(
Spacer(modifier = Modifier.navigationBarsPadding())
}
}
@Preview(showBackground = true, name = "Bitwarden loading content")
@Composable
private fun BitwardenLoadingContent_preview() {
BitwardenScaffold {
BitwardenLoadingContent(
text = "Loading...",
modifier = Modifier
.fillMaxSize()
.standardHorizontalMargin(),
)
}
}

View File

@ -1119,6 +1119,8 @@ Do you want to switch to this account?</string>
<string name="not_now">Not now</string>
<string name="import_from_bitwarden">Import from Bitwarden</string>
<string name="select_account">Select account</string>
<string name="no_accounts_available">No accounts available</string>
<string name="you_dont_have_any_accounts_you_can_import_from">You dont have any accounts you can import from. Your organizations security policy may restrict importing items from Bitwarden to another app.</string>
<string name="import_restricted_unable_to_import_credit_cards">Import restricted, unable to import cards from this account.</string>
<string name="verify_your_master_password">Verify your master password</string>
<string name="having_trouble_with_autofill">Having trouble with autofill?</string>