PM-27202: Update ItemListingScreen layout for improved spacing (#6065)

This commit is contained in:
David Perez 2025-10-22 09:42:35 -05:00 committed by GitHub
parent 4597337500
commit 92cfce1224
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 86 additions and 96 deletions

View File

@ -332,22 +332,26 @@ private fun ItemListingContent(
) {
var isLocalHeaderExpanded by rememberSaveable { mutableStateOf(value = true) }
LazyColumn(modifier = modifier.fillMaxSize()) {
item(key = "action_card") {
ActionCard(
actionCardState = state.actionCard,
onDownloadBitwardenClick = onDownloadBitwardenClick,
onDownloadBitwardenDismissClick = onDismissDownloadBitwardenClick,
onSyncWithBitwardenClick = onSyncWithBitwardenClick,
onSyncWithBitwardenDismissClick = onDismissSyncWithBitwardenClick,
onSyncLearnMoreClick = onSyncLearnMoreClick,
modifier = Modifier
.standardHorizontalMargin()
.padding(top = 12.dp, bottom = 16.dp)
.animateItem(),
)
state.actionCard?.let {
item(key = "action_card") {
Spacer(modifier = Modifier.height(height = 12.dp))
ActionCard(
actionCardState = it,
onDownloadBitwardenClick = onDownloadBitwardenClick,
onDownloadBitwardenDismissClick = onDismissDownloadBitwardenClick,
onSyncWithBitwardenClick = onSyncWithBitwardenClick,
onSyncWithBitwardenDismissClick = onDismissSyncWithBitwardenClick,
onSyncLearnMoreClick = onSyncLearnMoreClick,
modifier = Modifier
.standardHorizontalMargin()
.animateItem(),
)
}
}
if (state.favoriteItems.isNotEmpty()) {
item(key = "favorites_header") {
Spacer(modifier = Modifier.height(height = 12.dp))
BitwardenListHeaderText(
label = stringResource(id = BitwardenString.favorites),
supportingLabel = state.favoriteItems.count().toString(),
@ -399,6 +403,10 @@ private fun ItemListingContent(
.animateItem(),
)
}
} else if (state.sharedItems.isEmpty()) {
item(key = "local_items_spacer") {
Spacer(modifier = Modifier.height(height = 16.dp))
}
}
if (isLocalHeaderExpanded) {
@ -494,7 +502,7 @@ private fun ItemListingContent(
@Suppress("LongMethod")
@Composable
fun EmptyItemListingContent(
actionCardState: ItemListingState.ActionCardState,
actionCardState: ItemListingState.ActionCardState?,
onAddCodeClick: () -> Unit,
onDownloadBitwardenClick: () -> Unit,
onDismissDownloadBitwardenClick: () -> Unit,
@ -508,22 +516,24 @@ fun EmptyItemListingContent(
.fillMaxSize()
.verticalScroll(rememberScrollState()),
verticalArrangement = when (actionCardState) {
ItemListingState.ActionCardState.None -> Arrangement.Center
null -> Arrangement.Center
ItemListingState.ActionCardState.DownloadBitwardenApp -> Arrangement.Top
ItemListingState.ActionCardState.SyncWithBitwarden -> Arrangement.Top
},
) {
ActionCard(
actionCardState = actionCardState,
onDownloadBitwardenClick = onDownloadBitwardenClick,
onDownloadBitwardenDismissClick = onDismissDownloadBitwardenClick,
onSyncWithBitwardenClick = onSyncWithBitwardenClick,
onSyncWithBitwardenDismissClick = onDismissSyncWithBitwardenClick,
onSyncLearnMoreClick = onSyncLearnMoreClick,
modifier = Modifier
.standardHorizontalMargin()
.padding(top = 12.dp, bottom = 16.dp),
)
actionCardState?.let {
Spacer(modifier = Modifier.height(height = 12.dp))
ActionCard(
actionCardState = it,
onDownloadBitwardenClick = onDownloadBitwardenClick,
onDownloadBitwardenDismissClick = onDismissDownloadBitwardenClick,
onSyncWithBitwardenClick = onSyncWithBitwardenClick,
onSyncWithBitwardenDismissClick = onDismissSyncWithBitwardenClick,
onSyncLearnMoreClick = onSyncLearnMoreClick,
modifier = Modifier.standardHorizontalMargin(),
)
Spacer(modifier = Modifier.height(height = 16.dp))
}
Column(
modifier = modifier
@ -620,8 +630,6 @@ private fun ActionCard(
},
)
}
ItemListingState.ActionCardState.None -> Unit
}
}
@ -646,7 +654,7 @@ private fun ContentPreview() {
BitwardenTheme {
ItemListingContent(
state = ItemListingState.ViewState.Content(
actionCard = ItemListingState.ActionCardState.None,
actionCard = null,
favoriteItems = persistentListOf(),
itemList = persistentListOf(
VerificationCodeDisplayItem(

View File

@ -542,13 +542,8 @@ class ItemListingViewModel @Inject constructor(
it.copy(
viewState = when (it.viewState) {
ItemListingState.ViewState.Loading -> it.viewState
is ItemListingState.ViewState.Content -> it.viewState.copy(
actionCard = ItemListingState.ActionCardState.None,
)
is ItemListingState.ViewState.NoItems -> it.viewState.copy(
actionCard = ItemListingState.ActionCardState.None,
)
is ItemListingState.ViewState.Content -> it.viewState.copy(actionCard = null)
is ItemListingState.ViewState.NoItems -> it.viewState.copy(actionCard = null)
},
)
}
@ -564,13 +559,8 @@ class ItemListingViewModel @Inject constructor(
it.copy(
viewState = when (it.viewState) {
ItemListingState.ViewState.Loading -> it.viewState
is ItemListingState.ViewState.Content -> it.viewState.copy(
actionCard = ItemListingState.ActionCardState.None,
)
is ItemListingState.ViewState.NoItems -> it.viewState.copy(
actionCard = ItemListingState.ActionCardState.None,
)
is ItemListingState.ViewState.Content -> it.viewState.copy(actionCard = null)
is ItemListingState.ViewState.NoItems -> it.viewState.copy(actionCard = null)
},
)
}
@ -602,20 +592,20 @@ class ItemListingViewModel @Inject constructor(
/**
* Converts a [SharedVerificationCodesState] into an action card for display.
*/
private fun SharedVerificationCodesState.toActionCard(): ItemListingState.ActionCardState =
private fun SharedVerificationCodesState.toActionCard(): ItemListingState.ActionCardState? =
when (this) {
SharedVerificationCodesState.AppNotInstalled ->
if (!settingsRepository.hasUserDismissedDownloadBitwardenCard) {
ItemListingState.ActionCardState.DownloadBitwardenApp
} else {
ItemListingState.ActionCardState.None
null
}
SharedVerificationCodesState.SyncNotEnabled ->
if (!settingsRepository.hasUserDismissedSyncWithBitwardenCard) {
ItemListingState.ActionCardState.SyncWithBitwarden
} else {
ItemListingState.ActionCardState.None
null
}
SharedVerificationCodesState.Error,
@ -623,7 +613,7 @@ class ItemListingViewModel @Inject constructor(
SharedVerificationCodesState.Loading,
SharedVerificationCodesState.OsVersionNotSupported,
is SharedVerificationCodesState.Success,
-> ItemListingState.ActionCardState.None
-> null
}
private fun String.toAuthenticatorEntityOrNull(): AuthenticatorItemEntity? {
@ -733,7 +723,7 @@ data class ItemListingState(
*/
@Parcelize
data class NoItems(
val actionCard: ActionCardState,
val actionCard: ActionCardState?,
) : ViewState()
/**
@ -741,7 +731,7 @@ data class ItemListingState(
*/
@Parcelize
data class Content(
val actionCard: ActionCardState,
val actionCard: ActionCardState?,
val favoriteItems: ImmutableList<VerificationCodeDisplayItem>,
val itemList: ImmutableList<VerificationCodeDisplayItem>,
val sharedItems: SharedCodesDisplayState,
@ -752,13 +742,10 @@ data class ItemListingState(
*/
val shouldShowLocalHeader
get() =
// Only show header if there are shared items
!sharedItems.isEmpty() &&
// And also local items
itemList.isNotEmpty() &&
// But there are no favorite items
// (If there are favorite items, the favorites header will take care of us)
favoriteItems.isEmpty()
// Only show if local codes are present
itemList.isNotEmpty() &&
// and if there are shared items or favorites
(!sharedItems.isEmpty() || favoriteItems.isNotEmpty())
}
}
@ -766,12 +753,6 @@ data class ItemListingState(
* Display an action card on the item [ItemListingScreen].
*/
sealed class ActionCardState : Parcelable {
/**
* Display no action card.
*/
@Parcelize
data object None : ActionCardState()
/**
* Display the "Download the Bitwarden app" card.
*/

View File

@ -15,6 +15,7 @@ import androidx.compose.material3.minimumInteractiveComponentSize
import androidx.compose.runtime.Composable
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.draw.clip
import androidx.compose.ui.draw.rotate
import androidx.compose.ui.semantics.semantics
import androidx.compose.ui.tooling.preview.Preview
@ -39,6 +40,7 @@ fun AuthenticatorExpandingHeader(
verticalAlignment = Alignment.CenterVertically,
modifier = modifier
.fillMaxWidth()
.clip(shape = BitwardenTheme.shapes.content)
.clickable(
onClickLabel = onClickLabel,
onClick = onClick,
@ -53,10 +55,10 @@ fun AuthenticatorExpandingHeader(
label = "expanderIconRotationAnimation",
)
Text(
text = label,
style = BitwardenTheme.typography.labelMedium,
text = label.uppercase(),
style = BitwardenTheme.typography.eyebrowMedium,
color = BitwardenTheme.colorScheme.text.secondary,
modifier = Modifier.weight(1f, fill = false),
modifier = Modifier.weight(weight = 1f, fill = false),
)
Spacer(modifier = Modifier.width(width = 8.dp))
Icon(

View File

@ -119,7 +119,7 @@ class ItemListingScreenTest : AuthenticatorComposeTest() {
fun `shared accounts error message should show when view is Content with SharedCodesDisplayState Error`() {
mutableStateFlow.value = DEFAULT_STATE.copy(
viewState = ItemListingState.ViewState.Content(
actionCard = ItemListingState.ActionCardState.None,
actionCard = null,
favoriteItems = persistentListOf(),
itemList = persistentListOf(),
sharedItems = SharedCodesDisplayState.Error,
@ -132,7 +132,7 @@ class ItemListingScreenTest : AuthenticatorComposeTest() {
mutableStateFlow.value = DEFAULT_STATE.copy(
viewState = ItemListingState.ViewState.Content(
actionCard = ItemListingState.ActionCardState.None,
actionCard = null,
favoriteItems = persistentListOf(),
itemList = persistentListOf(),
sharedItems = SharedCodesDisplayState.Codes(persistentListOf()),
@ -148,7 +148,7 @@ class ItemListingScreenTest : AuthenticatorComposeTest() {
fun `clicking shared accounts verification code item should send ItemClick action`() {
mutableStateFlow.value = DEFAULT_STATE.copy(
viewState = ItemListingState.ViewState.Content(
actionCard = ItemListingState.ActionCardState.None,
actionCard = null,
favoriteItems = persistentListOf(),
itemList = persistentListOf(),
sharedItems = SharedCodesDisplayState.Codes(
@ -354,7 +354,7 @@ class ItemListingScreenTest : AuthenticatorComposeTest() {
fun `clicking Copy to Bitwarden vault should send DropdownMenuClick with COPY_TO_BITWARDEN`() {
mutableStateFlow.value = DEFAULT_STATE.copy(
viewState = ItemListingState.ViewState.Content(
actionCard = ItemListingState.ActionCardState.None,
actionCard = null,
favoriteItems = persistentListOf(),
itemList = persistentListOf(LOCAL_CODE),
sharedItems = SharedCodesDisplayState.Error,
@ -385,7 +385,7 @@ class ItemListingScreenTest : AuthenticatorComposeTest() {
fun `Copy to Bitwarden vault long press action should not show when showMoveToBitwarden is false`() {
mutableStateFlow.value = DEFAULT_STATE.copy(
viewState = ItemListingState.ViewState.Content(
actionCard = ItemListingState.ActionCardState.None,
actionCard = null,
favoriteItems = persistentListOf(),
itemList = persistentListOf(LOCAL_CODE.copy(showMoveToBitwarden = false)),
sharedItems = SharedCodesDisplayState.Error,
@ -405,7 +405,7 @@ class ItemListingScreenTest : AuthenticatorComposeTest() {
mutableStateFlow.update {
DEFAULT_STATE.copy(
viewState = ItemListingState.ViewState.Content(
actionCard = ItemListingState.ActionCardState.None,
actionCard = null,
favoriteItems = persistentListOf(),
itemList = persistentListOf(),
sharedItems = SharedCodesDisplayState.Codes(persistentListOf()),
@ -432,7 +432,7 @@ class ItemListingScreenTest : AuthenticatorComposeTest() {
fun `local codes header should be displayed and expanded when syncing is enabled`() {
mutableStateFlow.value = DEFAULT_STATE.copy(
viewState = ItemListingState.ViewState.Content(
actionCard = ItemListingState.ActionCardState.None,
actionCard = null,
favoriteItems = persistentListOf(),
itemList = persistentListOf(LOCAL_CODE),
sharedItems = SharedCodesDisplayState.Codes(
@ -442,7 +442,7 @@ class ItemListingScreenTest : AuthenticatorComposeTest() {
)
composeTestRule
.onNodeWithText("Local codes (1)")
.onNodeWithText(text = "LOCAL CODES (1)")
.assertIsDisplayed()
composeTestRule
@ -456,7 +456,7 @@ class ItemListingScreenTest : AuthenticatorComposeTest() {
sections = persistentListOf(SHARED_ACCOUNTS_SECTION),
)
val viewState = ItemListingState.ViewState.Content(
actionCard = ItemListingState.ActionCardState.None,
actionCard = null,
favoriteItems = persistentListOf(),
itemList = persistentListOf(LOCAL_CODE),
sharedItems = sharedItems,
@ -464,7 +464,7 @@ class ItemListingScreenTest : AuthenticatorComposeTest() {
mutableStateFlow.value = DEFAULT_STATE.copy(viewState = viewState)
composeTestRule
.onNodeWithTextAfterScroll("test@test.com | bitwarden.com (1)")
.onNodeWithTextAfterScroll(text = "TEST@TEST.COM | BITWARDEN.COM (1)")
.performClick()
verify {
@ -478,7 +478,7 @@ class ItemListingScreenTest : AuthenticatorComposeTest() {
sections = persistentListOf(SHARED_ACCOUNTS_SECTION),
)
val viewState = ItemListingState.ViewState.Content(
actionCard = ItemListingState.ActionCardState.None,
actionCard = null,
favoriteItems = persistentListOf(),
itemList = persistentListOf(LOCAL_CODE),
sharedItems = sharedItems,
@ -486,7 +486,7 @@ class ItemListingScreenTest : AuthenticatorComposeTest() {
mutableStateFlow.value = DEFAULT_STATE.copy(viewState = viewState)
composeTestRule
.onNodeWithTextAfterScroll("test@test.com | bitwarden.com (1)")
.onNodeWithTextAfterScroll(text = "TEST@TEST.COM | BITWARDEN.COM (1)")
.assertIsDisplayed()
composeTestRule
@ -502,7 +502,7 @@ class ItemListingScreenTest : AuthenticatorComposeTest() {
)
composeTestRule
.onNodeWithTextAfterScroll("test@test.com | bitwarden.com (1)")
.onNodeWithTextAfterScroll(text = "TEST@TEST.COM | BITWARDEN.COM (1)")
.assertIsDisplayed()
composeTestRule
@ -514,7 +514,7 @@ class ItemListingScreenTest : AuthenticatorComposeTest() {
fun `local codes should be displayed based on expanding header state`() {
mutableStateFlow.value = DEFAULT_STATE.copy(
viewState = ItemListingState.ViewState.Content(
actionCard = ItemListingState.ActionCardState.None,
actionCard = null,
favoriteItems = persistentListOf(),
itemList = persistentListOf(LOCAL_CODE),
sharedItems = SharedCodesDisplayState.Codes(
@ -528,7 +528,7 @@ class ItemListingScreenTest : AuthenticatorComposeTest() {
.assertIsDisplayed()
composeTestRule
.onNodeWithText("Local codes (1)")
.onNodeWithText(text = "LOCAL CODES (1)")
.performClick()
composeTestRule
@ -575,7 +575,7 @@ private val SHARED_ACCOUNTS_SECTION = SharedCodesDisplayState.SharedCodesAccount
private val DEFAULT_STATE = ItemListingState(
alertThresholdSeconds = ALERT_THRESHOLD,
viewState = ItemListingState.ViewState.NoItems(
actionCard = ItemListingState.ActionCardState.None,
actionCard = null,
),
dialog = null,
)

View File

@ -42,8 +42,7 @@ class ItemListingViewModelTest : BaseViewModelTest() {
MutableStateFlow<DataState<List<VerificationCodeItem>>>(DataState.Loading)
private val mutableSharedCodesFlow =
MutableStateFlow<SharedVerificationCodesState>(SharedVerificationCodesState.Loading)
private val firstTimeAccountSyncChannel: Channel<Unit> =
Channel(capacity = Channel.UNLIMITED)
private val firstTimeAccountSyncChannel: Channel<Unit> = Channel(capacity = Channel.UNLIMITED)
private val authenticatorRepository: AuthenticatorRepository = mockk {
every { totpCodeFlow } returns emptyFlow()
@ -89,7 +88,7 @@ class ItemListingViewModelTest : BaseViewModelTest() {
fun `stateFlow value should not show download bitwarden card when local items are empty and shared state is AppNotInstalled but user has dismissed card`() {
val expectedState = DEFAULT_STATE.copy(
viewState = ItemListingState.ViewState.NoItems(
actionCard = ItemListingState.ActionCardState.None,
actionCard = null,
),
)
every { settingsRepository.hasUserDismissedDownloadBitwardenCard } returns true
@ -122,7 +121,7 @@ class ItemListingViewModelTest : BaseViewModelTest() {
fun `stateFlow value should not show download bitwarden card when there are local items and shared state is AppNotInstalled but user has dismissed card`() {
val expectedState = DEFAULT_STATE.copy(
viewState = ItemListingState.ViewState.Content(
actionCard = ItemListingState.ActionCardState.None,
actionCard = null,
favoriteItems = LOCAL_FAVORITE_ITEMS,
itemList = LOCAL_NON_FAVORITE_ITEMS,
sharedItems = SharedCodesDisplayState.Codes(persistentListOf()),
@ -140,7 +139,7 @@ class ItemListingViewModelTest : BaseViewModelTest() {
fun `stateFlow sharedItems value should be Error when shared state is Error `() {
val expectedState = DEFAULT_STATE.copy(
viewState = ItemListingState.ViewState.Content(
actionCard = ItemListingState.ActionCardState.None,
actionCard = null,
favoriteItems = LOCAL_FAVORITE_ITEMS,
itemList = LOCAL_NON_FAVORITE_ITEMS,
sharedItems = SharedCodesDisplayState.Error,
@ -157,7 +156,7 @@ class ItemListingViewModelTest : BaseViewModelTest() {
fun `stateFlow sharedItems value should be Codes with empty list when shared state is Success `() {
val expectedState = DEFAULT_STATE.copy(
viewState = ItemListingState.ViewState.Content(
actionCard = ItemListingState.ActionCardState.None,
actionCard = null,
favoriteItems = LOCAL_FAVORITE_ITEMS
.map { it.copy(showMoveToBitwarden = true) }
.toImmutableList(),
@ -179,7 +178,7 @@ class ItemListingViewModelTest : BaseViewModelTest() {
fun `stateFlow sharedItems value should show items even when local items are empty`() {
val expectedState = DEFAULT_STATE.copy(
viewState = ItemListingState.ViewState.NoItems(
actionCard = ItemListingState.ActionCardState.None,
actionCard = null,
),
)
mutableVerificationCodesFlow.value = DataState.Loaded(emptyList())
@ -194,7 +193,7 @@ class ItemListingViewModelTest : BaseViewModelTest() {
fun `stateFlow viewState value should be NoItems when both local and shared codes are empty`() {
val expectedState = DEFAULT_STATE.copy(
viewState = ItemListingState.ViewState.Content(
actionCard = ItemListingState.ActionCardState.None,
actionCard = null,
favoriteItems = persistentListOf(),
itemList = persistentListOf(),
sharedItems = SHARED_DISPLAY_ITEMS,
@ -222,7 +221,7 @@ class ItemListingViewModelTest : BaseViewModelTest() {
runTest {
val expectedState = DEFAULT_STATE.copy(
viewState = ItemListingState.ViewState.Content(
actionCard = ItemListingState.ActionCardState.None,
actionCard = null,
favoriteItems = LOCAL_FAVORITE_ITEMS,
itemList = LOCAL_NON_FAVORITE_ITEMS,
sharedItems = SharedCodesDisplayState.Codes(persistentListOf()),
@ -243,7 +242,7 @@ class ItemListingViewModelTest : BaseViewModelTest() {
runTest {
val expectedState = DEFAULT_STATE.copy(
viewState = ItemListingState.ViewState.NoItems(
actionCard = ItemListingState.ActionCardState.None,
actionCard = null,
),
)
every { settingsRepository.hasUserDismissedDownloadBitwardenCard = true } just runs
@ -270,7 +269,7 @@ class ItemListingViewModelTest : BaseViewModelTest() {
runTest {
val expectedState = DEFAULT_STATE.copy(
viewState = ItemListingState.ViewState.Content(
actionCard = ItemListingState.ActionCardState.None,
actionCard = null,
favoriteItems = LOCAL_FAVORITE_ITEMS,
itemList = LOCAL_NON_FAVORITE_ITEMS,
sharedItems = SharedCodesDisplayState.Codes(persistentListOf()),
@ -292,7 +291,7 @@ class ItemListingViewModelTest : BaseViewModelTest() {
runTest {
val expectedState = DEFAULT_STATE.copy(
viewState = ItemListingState.ViewState.NoItems(
actionCard = ItemListingState.ActionCardState.None,
actionCard = null,
),
)
every { settingsRepository.hasUserDismissedSyncWithBitwardenCard = true } just runs
@ -324,7 +323,7 @@ class ItemListingViewModelTest : BaseViewModelTest() {
fun `stateFlow value should not show download bitwarden card when local items are empty and shared state is SyncNotEnabled but user has dismissed card`() {
val expectedState = DEFAULT_STATE.copy(
viewState = ItemListingState.ViewState.NoItems(
actionCard = ItemListingState.ActionCardState.None,
actionCard = null,
),
)
every { settingsRepository.hasUserDismissedSyncWithBitwardenCard } returns true
@ -357,7 +356,7 @@ class ItemListingViewModelTest : BaseViewModelTest() {
fun `stateFlow value should not show sync with bitwarden card when there are local items and shared state is AppNotInstalled but user has dismissed card`() {
val expectedState = DEFAULT_STATE.copy(
viewState = ItemListingState.ViewState.Content(
actionCard = ItemListingState.ActionCardState.None,
actionCard = null,
favoriteItems = LOCAL_FAVORITE_ITEMS,
itemList = LOCAL_NON_FAVORITE_ITEMS,
sharedItems = SharedCodesDisplayState.Codes(persistentListOf()),
@ -518,7 +517,7 @@ class ItemListingViewModelTest : BaseViewModelTest() {
fun `on SectionExpandedClick should update expanded state for clicked section`() = runTest {
val expectedState = DEFAULT_STATE.copy(
viewState = ItemListingState.ViewState.Content(
actionCard = ItemListingState.ActionCardState.None,
actionCard = null,
favoriteItems = LOCAL_FAVORITE_ITEMS
.map { it.copy(showMoveToBitwarden = true) }
.toImmutableList(),