From 82ef39e15d735cdceedcca306e73c04d7285f9ae Mon Sep 17 00:00:00 2001 From: Brian Yencho Date: Sun, 28 Jan 2024 22:42:13 -0600 Subject: [PATCH] Show appropriate empty states for autofill flow (#843) --- .../itemlisting/VaultItemListingEmpty.kt | 36 +++----- .../itemlisting/VaultItemListingScreen.kt | 2 +- .../itemlisting/VaultItemListingViewModel.kt | 7 +- .../util/VaultItemListingDataExtensions.kt | 41 ++++++++- .../ui/vault/feature/vault/VaultNoItems.kt | 3 +- .../itemlisting/VaultItemListingScreenTest.kt | 66 ++++++++------- .../VaultItemListingViewModelTest.kt | 46 ++++++++-- .../VaultItemListingDataExtensionsTest.kt | 84 ++++++++++++++++++- 8 files changed, 214 insertions(+), 71 deletions(-) diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingEmpty.kt b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingEmpty.kt index 3d8df99c7..23610890a 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingEmpty.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingEmpty.kt @@ -9,10 +9,8 @@ import androidx.compose.material3.Text import androidx.compose.runtime.Composable import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier -import androidx.compose.ui.res.stringResource import androidx.compose.ui.text.style.TextAlign import androidx.compose.ui.unit.dp -import com.x8bit.bitwarden.R import com.x8bit.bitwarden.ui.vault.feature.vault.VaultNoItems /** @@ -20,31 +18,21 @@ import com.x8bit.bitwarden.ui.vault.feature.vault.VaultNoItems */ @Composable fun VaultItemListingEmpty( - itemListingType: VaultItemListingState.ItemListingType, + state: VaultItemListingState.ViewState.NoItems, addItemClickAction: () -> Unit, modifier: Modifier = Modifier, ) { - when (itemListingType) { - is VaultItemListingState.ItemListingType.Vault.Folder -> { - GenericNoItems( - modifier = modifier, - text = stringResource(id = R.string.no_items_folder), - ) - } - - is VaultItemListingState.ItemListingType.Vault.Trash -> { - GenericNoItems( - modifier = modifier, - text = stringResource(id = R.string.no_items_trash), - ) - } - - else -> { - VaultNoItems( - modifier = modifier, - addItemClickAction = addItemClickAction, - ) - } + if (state.shouldShowAddButton) { + VaultNoItems( + message = state.message(), + modifier = modifier, + addItemClickAction = addItemClickAction, + ) + } else { + GenericNoItems( + text = state.message(), + modifier = modifier, + ) } } diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingScreen.kt b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingScreen.kt index 3471b891c..d24c9263d 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingScreen.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingScreen.kt @@ -242,7 +242,7 @@ private fun VaultItemListingScaffold( is VaultItemListingState.ViewState.NoItems -> { VaultItemListingEmpty( - itemListingType = state.itemListingType, + state = state.viewState, addItemClickAction = vaultItemListingHandlers.addVaultItemClick, modifier = modifier, ) diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingViewModel.kt b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingViewModel.kt index 12f37931d..2683a47ba 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingViewModel.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingViewModel.kt @@ -545,8 +545,10 @@ class VaultItemListingViewModel @Inject constructor( } .toFilteredList(state.vaultFilterType) .toViewState( + itemListingType = listingType, baseIconUrl = state.baseIconUrl, isIconLoadingDisabled = state.isIconLoadingDisabled, + autofillSelectionData = state.autofillSelectionData, ) } @@ -702,7 +704,10 @@ data class VaultItemListingState( /** * Represents a state where the [VaultItemListingScreen] has no items to display. */ - data object NoItems : ViewState() { + data class NoItems( + val message: Text, + val shouldShowAddButton: Boolean, + ) : ViewState() { override val isPullToRefreshEnabled: Boolean get() = true } diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/util/VaultItemListingDataExtensions.kt b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/util/VaultItemListingDataExtensions.kt index 479ae3280..b10ee9c88 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/util/VaultItemListingDataExtensions.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/util/VaultItemListingDataExtensions.kt @@ -8,7 +8,10 @@ import com.bitwarden.core.FolderView import com.bitwarden.core.SendType import com.bitwarden.core.SendView import com.x8bit.bitwarden.R +import com.x8bit.bitwarden.data.autofill.model.AutofillSelectionData import com.x8bit.bitwarden.data.platform.util.subtitle +import com.x8bit.bitwarden.ui.platform.base.util.asText +import com.x8bit.bitwarden.ui.platform.base.util.toHostOrPathOrNull import com.x8bit.bitwarden.ui.platform.components.model.IconData import com.x8bit.bitwarden.ui.platform.util.toFormattedPattern import com.x8bit.bitwarden.ui.tools.feature.send.util.toLabelIcons @@ -78,8 +81,10 @@ fun SendView.determineListingPredicate( * Transforms a list of [CipherView] into [VaultItemListingState.ViewState]. */ fun List.toViewState( + itemListingType: VaultItemListingState.ItemListingType.Vault, baseIconUrl: String, isIconLoadingDisabled: Boolean, + autofillSelectionData: AutofillSelectionData?, ): VaultItemListingState.ViewState = if (isNotEmpty()) { VaultItemListingState.ViewState.Content( @@ -89,7 +94,36 @@ fun List.toViewState( ), ) } else { - VaultItemListingState.ViewState.NoItems + // Use the autofill empty message if necessary, otherwise use normal type-specific message + val message = autofillSelectionData + ?.uri + ?.toHostOrPathOrNull() + ?.let { R.string.no_items_for_uri.asText(it) } + ?: run { + when (itemListingType) { + is VaultItemListingState.ItemListingType.Vault.Folder -> { + R.string.no_items_folder + } + + VaultItemListingState.ItemListingType.Vault.Trash -> { + R.string.no_items_trash + } + + else -> R.string.no_items + } + .asText() + } + val shouldShowAddButton = when (itemListingType) { + is VaultItemListingState.ItemListingType.Vault.Folder, + VaultItemListingState.ItemListingType.Vault.Trash, + -> false + + else -> true + } + VaultItemListingState.ViewState.NoItems( + message = message, + shouldShowAddButton = shouldShowAddButton, + ) } /** @@ -107,7 +141,10 @@ fun List.toViewState( ), ) } else { - VaultItemListingState.ViewState.NoItems + VaultItemListingState.ViewState.NoItems( + message = R.string.no_items.asText(), + shouldShowAddButton = true, + ) } /** * Updates a [VaultItemListingState.ItemListingType] with the given data if necessary. */ diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/vault/VaultNoItems.kt b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/vault/VaultNoItems.kt index 45db0c126..24ff2133b 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/vault/VaultNoItems.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/vault/VaultNoItems.kt @@ -25,6 +25,7 @@ import com.x8bit.bitwarden.R fun VaultNoItems( addItemClickAction: () -> Unit, modifier: Modifier = Modifier, + message: String = stringResource(id = R.string.no_items), ) { Column( modifier = modifier, @@ -36,7 +37,7 @@ fun VaultNoItems( modifier = Modifier .fillMaxWidth() .padding(horizontal = 16.dp), - text = stringResource(id = R.string.no_items), + text = message, style = MaterialTheme.typography.bodyMedium, ) diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingScreenTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingScreenTest.kt index cba162c51..ae15a10d9 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingScreenTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingScreenTest.kt @@ -300,7 +300,14 @@ class VaultItemListingScreenTest : BaseComposeTest() { @Test fun `add an item button click should send AddItemClick action`() { - mutableStateFlow.update { it.copy(viewState = VaultItemListingState.ViewState.NoItems) } + mutableStateFlow.update { + it.copy( + viewState = VaultItemListingState.ViewState.NoItems( + message = "There are no items in your vault.".asText(), + shouldShowAddButton = true, + ), + ) + } composeTestRule .onNodeWithText("Add an Item") .performClick() @@ -385,7 +392,12 @@ class VaultItemListingScreenTest : BaseComposeTest() { .assertIsDisplayed() mutableStateFlow.update { - it.copy(viewState = VaultItemListingState.ViewState.NoItems) + it.copy( + viewState = VaultItemListingState.ViewState.NoItems( + message = "There are no items in your vault.".asText(), + shouldShowAddButton = true, + ), + ) } composeTestRule @@ -401,7 +413,14 @@ class VaultItemListingScreenTest : BaseComposeTest() { .onNodeWithText(message) .assertIsNotDisplayed() - mutableStateFlow.update { it.copy(viewState = VaultItemListingState.ViewState.NoItems) } + mutableStateFlow.update { + it.copy( + viewState = VaultItemListingState.ViewState.NoItems( + message = "There are no items in your vault.".asText(), + shouldShowAddButton = true, + ), + ) + } composeTestRule .onNodeWithText(message) .assertIsNotDisplayed() @@ -422,23 +441,23 @@ class VaultItemListingScreenTest : BaseComposeTest() { .assertDoesNotExist() mutableStateFlow.update { - it.copy(viewState = VaultItemListingState.ViewState.NoItems) + it.copy( + viewState = VaultItemListingState.ViewState.NoItems( + message = "There are no items in your vault.".asText(), + shouldShowAddButton = true, + ), + ) } + composeTestRule .onNodeWithText(text = "Add an Item") .assertIsDisplayed() - mutableStateFlow.update { - it.copy(itemListingType = VaultItemListingState.ItemListingType.Vault.Trash) - } - composeTestRule - .onNodeWithText(text = "Add an Item") - .assertDoesNotExist() - mutableStateFlow.update { it.copy( - itemListingType = VaultItemListingState.ItemListingType.Vault.Folder( - folderId = null, + viewState = VaultItemListingState.ViewState.NoItems( + message = "There are no items in your vault.".asText(), + shouldShowAddButton = false, ), ) } @@ -449,29 +468,16 @@ class VaultItemListingScreenTest : BaseComposeTest() { @Test fun `empty text should be displayed according to state`() { - mutableStateFlow.update { - DEFAULT_STATE.copy(viewState = VaultItemListingState.ViewState.NoItems) - } - composeTestRule - .onNodeWithText(text = "There are no items in your vault.") - .assertIsDisplayed() - - mutableStateFlow.update { - it.copy(itemListingType = VaultItemListingState.ItemListingType.Vault.Trash) - } - composeTestRule - .onNodeWithText(text = "There are no items in the trash.") - .assertIsDisplayed() - mutableStateFlow.update { it.copy( - itemListingType = VaultItemListingState.ItemListingType.Vault.Folder( - folderId = null, + viewState = VaultItemListingState.ViewState.NoItems( + message = "There are no items in your vault.".asText(), + shouldShowAddButton = true, ), ) } composeTestRule - .onNodeWithText(text = "There are no items in this folder.") + .onNodeWithText(text = "There are no items in your vault.") .assertIsDisplayed() } diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingViewModelTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingViewModelTest.kt index ad3306c68..1a48dc133 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingViewModelTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/VaultItemListingViewModelTest.kt @@ -621,7 +621,12 @@ class VaultItemListingViewModelTest : BaseViewModelTest() { assertEquals(VaultItemListingEvent.DismissPullToRefresh, awaitItem()) } assertEquals( - createVaultItemListingState(viewState = VaultItemListingState.ViewState.NoItems), + createVaultItemListingState( + viewState = VaultItemListingState.ViewState.NoItems( + message = R.string.no_items.asText(), + shouldShowAddButton = true, + ), + ), viewModel.stateFlow.value, ) } @@ -645,7 +650,10 @@ class VaultItemListingViewModelTest : BaseViewModelTest() { } assertEquals( createVaultItemListingState( - viewState = VaultItemListingState.ViewState.NoItems, + viewState = VaultItemListingState.ViewState.NoItems( + message = R.string.no_items.asText(), + shouldShowAddButton = true, + ), ), viewModel.stateFlow.value, ) @@ -708,7 +716,12 @@ class VaultItemListingViewModelTest : BaseViewModelTest() { val viewModel = createVaultItemListingViewModel() assertEquals( - createVaultItemListingState(viewState = VaultItemListingState.ViewState.NoItems), + createVaultItemListingState( + viewState = VaultItemListingState.ViewState.NoItems( + message = R.string.no_items.asText(), + shouldShowAddButton = true, + ), + ), viewModel.stateFlow.value, ) } @@ -729,7 +742,12 @@ class VaultItemListingViewModelTest : BaseViewModelTest() { val viewModel = createVaultItemListingViewModel() assertEquals( - createVaultItemListingState(viewState = VaultItemListingState.ViewState.NoItems), + createVaultItemListingState( + viewState = VaultItemListingState.ViewState.NoItems( + message = R.string.no_items.asText(), + shouldShowAddButton = true, + ), + ), viewModel.stateFlow.value, ) } @@ -810,7 +828,10 @@ class VaultItemListingViewModelTest : BaseViewModelTest() { } assertEquals( createVaultItemListingState( - viewState = VaultItemListingState.ViewState.NoItems, + viewState = VaultItemListingState.ViewState.NoItems( + message = R.string.no_items.asText(), + shouldShowAddButton = true, + ), ), viewModel.stateFlow.value, ) @@ -836,7 +857,10 @@ class VaultItemListingViewModelTest : BaseViewModelTest() { } assertEquals( createVaultItemListingState( - viewState = VaultItemListingState.ViewState.NoItems, + viewState = VaultItemListingState.ViewState.NoItems( + message = R.string.no_items.asText(), + shouldShowAddButton = true, + ), ), viewModel.stateFlow.value, ) @@ -916,7 +940,10 @@ class VaultItemListingViewModelTest : BaseViewModelTest() { } assertEquals( createVaultItemListingState( - viewState = VaultItemListingState.ViewState.NoItems, + viewState = VaultItemListingState.ViewState.NoItems( + message = R.string.no_items.asText(), + shouldShowAddButton = true, + ), ), viewModel.stateFlow.value, ) @@ -941,7 +968,10 @@ class VaultItemListingViewModelTest : BaseViewModelTest() { } assertEquals( createVaultItemListingState( - viewState = VaultItemListingState.ViewState.NoItems, + viewState = VaultItemListingState.ViewState.NoItems( + message = R.string.no_items.asText(), + shouldShowAddButton = true, + ), ), viewModel.stateFlow.value, ) diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/util/VaultItemListingDataExtensionsTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/util/VaultItemListingDataExtensionsTest.kt index b357c15a7..987352910 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/util/VaultItemListingDataExtensionsTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/util/VaultItemListingDataExtensionsTest.kt @@ -4,6 +4,8 @@ import android.net.Uri import com.bitwarden.core.CipherType import com.bitwarden.core.CipherView import com.bitwarden.core.SendType +import com.x8bit.bitwarden.R +import com.x8bit.bitwarden.data.autofill.model.AutofillSelectionData import com.x8bit.bitwarden.data.platform.repository.model.Environment import com.x8bit.bitwarden.data.platform.repository.util.baseIconUrl import com.x8bit.bitwarden.data.platform.repository.util.baseWebSendUrl @@ -12,13 +14,15 @@ import com.x8bit.bitwarden.data.vault.datasource.sdk.model.createMockCipherView import com.x8bit.bitwarden.data.vault.datasource.sdk.model.createMockCollectionView import com.x8bit.bitwarden.data.vault.datasource.sdk.model.createMockFolderView import com.x8bit.bitwarden.data.vault.datasource.sdk.model.createMockSendView +import com.x8bit.bitwarden.ui.platform.base.util.asText import com.x8bit.bitwarden.ui.vault.feature.itemlisting.VaultItemListingState import io.mockk.every import io.mockk.mockk import io.mockk.mockkStatic import io.mockk.unmockkStatic -import org.junit.Assert.assertEquals -import org.junit.Test +import org.junit.jupiter.api.AfterEach +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Test import java.time.Clock import java.time.Instant import java.time.ZoneOffset @@ -30,6 +34,12 @@ class VaultItemListingDataExtensionsTest { ZoneOffset.UTC, ) + @AfterEach + fun tearDown() { + unmockkStatic(Uri::class) + unmockkStatic(CipherView::subtitle) + } + @Test @Suppress("MaxLineLength") fun `determineListingPredicate should return the correct predicate for non trash Login cipherView`() { @@ -333,8 +343,10 @@ class VaultItemListingDataExtensionsTest { ) val result = cipherViewList.toViewState( + itemListingType = VaultItemListingState.ItemListingType.Vault.Login, isIconLoadingDisabled = false, baseIconUrl = Environment.Us.environmentUrlData.baseIconUrl, + autofillSelectionData = null, ) assertEquals( @@ -364,9 +376,73 @@ class VaultItemListingDataExtensionsTest { ), result, ) + } - unmockkStatic(CipherView::subtitle) - unmockkStatic(Uri::class) + @Suppress("MaxLineLength") + @Test + fun `toViewState should transform an empty list of CipherViews into a NoItems ViewState with the appropriate data`() { + val cipherViewList = emptyList() + + // Trash + assertEquals( + VaultItemListingState.ViewState.NoItems( + message = R.string.no_items_trash.asText(), + shouldShowAddButton = false, + ), + cipherViewList.toViewState( + itemListingType = VaultItemListingState.ItemListingType.Vault.Trash, + isIconLoadingDisabled = false, + baseIconUrl = Environment.Us.environmentUrlData.baseIconUrl, + autofillSelectionData = null, + ), + ) + + // Folders + assertEquals( + VaultItemListingState.ViewState.NoItems( + message = R.string.no_items_folder.asText(), + shouldShowAddButton = false, + ), + cipherViewList.toViewState( + itemListingType = VaultItemListingState.ItemListingType.Vault.Folder( + folderId = "folderId", + ), + isIconLoadingDisabled = false, + baseIconUrl = Environment.Us.environmentUrlData.baseIconUrl, + autofillSelectionData = null, + ), + ) + + // Other ciphers + assertEquals( + VaultItemListingState.ViewState.NoItems( + message = R.string.no_items.asText(), + shouldShowAddButton = true, + ), + cipherViewList.toViewState( + itemListingType = VaultItemListingState.ItemListingType.Vault.Login, + isIconLoadingDisabled = false, + baseIconUrl = Environment.Us.environmentUrlData.baseIconUrl, + autofillSelectionData = null, + ), + ) + + // Autofill + assertEquals( + VaultItemListingState.ViewState.NoItems( + message = R.string.no_items_for_uri.asText("www.test.com"), + shouldShowAddButton = true, + ), + cipherViewList.toViewState( + itemListingType = VaultItemListingState.ItemListingType.Vault.Login, + isIconLoadingDisabled = false, + baseIconUrl = Environment.Us.environmentUrlData.baseIconUrl, + autofillSelectionData = AutofillSelectionData( + type = AutofillSelectionData.Type.LOGIN, + uri = "https://www.test.com", + ), + ), + ) } @Test