diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/item/component/ItemHeader.kt b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/item/component/ItemHeader.kt index 5ee3dbf88c..5b5cd45d3b 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/item/component/ItemHeader.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/item/component/ItemHeader.kt @@ -13,7 +13,6 @@ import androidx.compose.foundation.layout.size import androidx.compose.foundation.lazy.LazyColumn import androidx.compose.foundation.lazy.LazyItemScope import androidx.compose.foundation.lazy.LazyListScope -import androidx.compose.foundation.lazy.items import androidx.compose.foundation.lazy.itemsIndexed import androidx.compose.material3.Icon import androidx.compose.material3.Text @@ -47,11 +46,6 @@ import com.x8bit.bitwarden.ui.vault.feature.item.model.VaultItemLocation import kotlinx.collections.immutable.ImmutableList import kotlinx.collections.immutable.persistentListOf -/** - * The max number of items that can be displayed before the "show more" text is visible. - */ -private const val EXPANDABLE_THRESHOLD = 2 - /** * Reusable composable for displaying the cipher name, favorite status, and related locations. * @@ -65,7 +59,7 @@ private const val EXPANDABLE_THRESHOLD = 2 * @param textFieldTestTag The test tag for the name field. * @param onExpandClick The action to be performed when the expandable text row is clicked. */ -@Suppress("LongMethod", "LongParameterList") +@Suppress("CyclomaticComplexMethod", "LongMethod", "LongParameterList") fun LazyListScope.itemHeader( value: String, isFavorite: Boolean, @@ -77,6 +71,15 @@ fun LazyListScope.itemHeader( textFieldTestTag: String? = null, onExpandClick: () -> Unit, ) { + val organizationLocation = relatedLocations + .firstOrNull { it is VaultItemLocation.Organization } + + val collectionLocations = relatedLocations + .filterIsInstance() + + val folderLocations = relatedLocations + .filterIsInstance() + item { Row( verticalAlignment = Alignment.CenterVertically, @@ -124,6 +127,8 @@ fun LazyListScope.itemHeader( } } + // When the item does not belong to an Org and is not assigned to a collection or folder we + // display the "No Folder" indicator. if (relatedLocations.isEmpty()) { item(key = "noFolder") { ItemLocationListItem( @@ -144,38 +149,97 @@ fun LazyListScope.itemHeader( return } - itemsIndexed( - key = { index, _ -> "locations_$index" }, - items = relatedLocations.take(EXPANDABLE_THRESHOLD), - ) { index, location -> - ItemLocationListItem( - vectorPainter = rememberVectorPainter(location.icon), - iconTestTag = "ItemLocationIcon", - text = location.name, - modifier = Modifier - .fillMaxWidth() - .standardHorizontalMargin() - .animateItem() - .cardStyle( - cardStyle = if (index == relatedLocations.size - 1) { - CardStyle.Bottom - } else { - CardStyle.Middle(hasDivider = false) - }, - paddingVertical = 0.dp, - paddingHorizontal = 16.dp, - ), - ) + // When the item is owned by an Org we display the organization name. + if (organizationLocation != null) { + item(key = "organization") { + ItemLocationListItem( + vectorPainter = rememberVectorPainter(organizationLocation.icon), + iconTestTag = "ItemLocationIcon", + text = organizationLocation.name, + modifier = Modifier + .fillMaxWidth() + .standardHorizontalMargin() + .animateItem() + .cardStyle( + cardStyle = if (relatedLocations.size == 1) { + CardStyle.Bottom + } else { + CardStyle.Middle(hasDivider = false) + }, + paddingVertical = 0.dp, + paddingHorizontal = 16.dp, + ), + ) + } } - if (isExpanded) { - items( - key = { "expandableLocations_$it" }, - items = relatedLocations.drop(EXPANDABLE_THRESHOLD), - ) { + // When the item is assigned to a single collection and a single folder we display both the + // collection and folder names. + if (collectionLocations.size == 1 && folderLocations.size == 1) { + itemsIndexed( + items = collectionLocations + folderLocations, + key = { index, location -> "locations_$index" }, + ) { index, location -> ItemLocationListItem( - vectorPainter = rememberVectorPainter(it.icon), - text = it.name, + vectorPainter = rememberVectorPainter(location.icon), + iconTestTag = "ItemLocationIcon", + text = location.name, + modifier = Modifier + .fillMaxWidth() + .standardHorizontalMargin() + .animateItem() + .cardStyle( + cardStyle = if (index == 1) { + CardStyle.Bottom + } else { + CardStyle.Middle(hasDivider = false) + }, + paddingVertical = 0.dp, + paddingHorizontal = 16.dp, + ), + ) + } + return + } + + // When the item is assigned to multiple collections we only display the first collection by + // default and collapse the remaining locations. + collectionLocations.firstOrNull() + ?.let { + item(key = "visibleCollection") { + ItemLocationListItem( + vectorPainter = rememberVectorPainter(it.icon), + iconTestTag = "ItemLocationIcon", + text = if (collectionLocations.size > 1 && !isExpanded) { + stringResource(R.string.x_ellipses, it.name) + } else { + it.name + }, + modifier = Modifier + .fillMaxWidth() + .standardHorizontalMargin() + .animateItem() + .cardStyle( + cardStyle = if (collectionLocations.size > 1) { + CardStyle.Middle(hasDivider = false) + } else { + CardStyle.Bottom + }, + paddingVertical = 0.dp, + paddingHorizontal = 16.dp, + ), + ) + } + } + + if (isExpanded) { + itemsIndexed( + key = { index, _ -> "expandableLocations_$index" }, + items = collectionLocations.drop(1) + folderLocations, + ) { index, location -> + ItemLocationListItem( + vectorPainter = rememberVectorPainter(location.icon), + text = location.name, iconTestTag = "ItemLocationIcon", modifier = Modifier .fillMaxWidth() @@ -190,8 +254,8 @@ fun LazyListScope.itemHeader( } } - if (relatedLocations.size > EXPANDABLE_THRESHOLD) { - item(key = "expandableLocationsShowMore") { + if (collectionLocations.size > 1) { + item(key = "expandableLocationsExpansionIndicator") { BitwardenExpandingHeader( collapsedText = stringResource(R.string.show_more), expandedText = stringResource(R.string.show_less), diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index ec600ce257..31ad45f6f7 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -1226,4 +1226,5 @@ Do you want to switch to this account? Show more No folder Show less + %s... diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemScreenTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemScreenTest.kt index 1e4363d897..811e07c778 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemScreenTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemScreenTest.kt @@ -431,6 +431,7 @@ class VaultItemScreenTest : BaseComposeTest() { relatedLocations = persistentListOf( VaultItemLocation.Organization("My organization"), VaultItemLocation.Collection("My collection"), + VaultItemLocation.Collection("My other collection"), VaultItemLocation.Folder("My folder"), ), ), @@ -458,6 +459,7 @@ class VaultItemScreenTest : BaseComposeTest() { relatedLocations = persistentListOf( VaultItemLocation.Organization("My organization"), VaultItemLocation.Collection("My collection"), + VaultItemLocation.Collection("My other collection"), VaultItemLocation.Folder("My folder"), ), ), @@ -465,21 +467,76 @@ class VaultItemScreenTest : BaseComposeTest() { ) } - // Verify only two locations are shown when content is collapsed. + // Verify only the first collection name is shown + composeTestRule + .onNodeWithText("My collection...") + .assertIsDisplayed() + + // Verify other collection names are not shown by default. + composeTestRule + .onNodeWithText("My other collection") + .assertIsNotDisplayed() + + // Verify folder name is not shown by default. composeTestRule .onNodeWithText("My folder") .assertIsNotDisplayed() - // Verify all locations are show when content is expanded. + // Verify all locations are show when content is expanded and ellipses is removed from + // the first collection name. composeTestRule .onNodeWithText("Show more") .performClick() + composeTestRule + .onNodeWithText("My collection") + .assertIsDisplayed() + composeTestRule + .onNodeWithText("My other collection") + .assertIsDisplayed() composeTestRule .onNodeWithText("My folder") .assertIsDisplayed() } } + @Test + fun `ExpandingHeader should show all locations when assigned to org, collection, and folder`() { + DEFAULT_VIEW_STATES.forEach { viewState -> + mutableStateFlow.update { + DEFAULT_STATE.copy( + viewState = viewState.copy( + common = DEFAULT_COMMON.copy( + relatedLocations = persistentListOf( + VaultItemLocation.Organization("My organization"), + VaultItemLocation.Collection("My collection"), + VaultItemLocation.Folder("My folder"), + ), + ), + ), + ) + } + + // Verify all location names are not shown by default. + composeTestRule + .onNodeWithText("My organization") + .assertIsDisplayed() + composeTestRule + .onNodeWithText("My collection") + .assertIsDisplayed() + composeTestRule + .onNodeWithText("My folder") + .assertIsDisplayed() + + // Verify expander button is not displayed when all locations are shown. + composeTestRule + .onNodeWithText("Show more") + .assertIsNotDisplayed() + composeTestRule + .onNodeWithText("Show less") + .assertIsNotDisplayed() + } + } + @Test fun `lastUpdated should be displayed according to state`() { EMPTY_VIEW_STATES