From a12bc47c207d1694718fe13afe62258bf1e76613 Mon Sep 17 00:00:00 2001 From: David Perez Date: Thu, 18 Jan 2024 08:31:30 -0600 Subject: [PATCH] Add basic overflow for list screen (#662) --- .../itemlisting/VaultItemListingScreen.kt | 74 +++++++++++------- .../itemlisting/VaultItemListingViewModel.kt | 60 +++++++++++++-- .../handlers/VaultItemListingHandlers.kt | 41 ++++++++++ .../itemlisting/VaultItemListingScreenTest.kt | 76 +++++++++++++++++++ .../VaultItemListingViewModelTest.kt | 36 ++++++++- 5 files changed, 253 insertions(+), 34 deletions(-) create mode 100644 app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/handlers/VaultItemListingHandlers.kt 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 72318cae44..f976a44a71 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 @@ -11,6 +11,7 @@ import androidx.compose.material3.TopAppBarDefaults import androidx.compose.material3.rememberTopAppBarState import androidx.compose.runtime.Composable import androidx.compose.runtime.collectAsState +import androidx.compose.runtime.getValue import androidx.compose.runtime.remember import androidx.compose.ui.Modifier import androidx.compose.ui.input.nestedscroll.nestedScroll @@ -22,10 +23,15 @@ import com.x8bit.bitwarden.R import com.x8bit.bitwarden.ui.platform.base.util.EventsEffect import com.x8bit.bitwarden.ui.platform.components.BitwardenErrorContent import com.x8bit.bitwarden.ui.platform.components.BitwardenLoadingContent +import com.x8bit.bitwarden.ui.platform.components.BitwardenLoadingDialog import com.x8bit.bitwarden.ui.platform.components.BitwardenOverflowActionItem import com.x8bit.bitwarden.ui.platform.components.BitwardenScaffold import com.x8bit.bitwarden.ui.platform.components.BitwardenSearchActionItem import com.x8bit.bitwarden.ui.platform.components.BitwardenTopAppBar +import com.x8bit.bitwarden.ui.platform.components.LoadingDialogState +import com.x8bit.bitwarden.ui.platform.components.OverflowMenuItemData +import com.x8bit.bitwarden.ui.vault.feature.itemlisting.handlers.VaultItemListingHandlers +import kotlinx.collections.immutable.persistentListOf /** * Displays the vault item listing screen. @@ -39,6 +45,7 @@ fun VaultItemListingScreen( onNavigateToEditSendItem: (sendId: String) -> Unit, viewModel: VaultItemListingViewModel = hiltViewModel(), ) { + val state by viewModel.stateFlow.collectAsState() val context = LocalContext.current val resources = context.resources EventsEffect(viewModel = viewModel) { event -> @@ -73,36 +80,38 @@ fun VaultItemListingScreen( } } } + + VaultItemListingDialogs( + dialogState = state.dialogState, + ) + VaultItemListingScaffold( - state = viewModel.stateFlow.collectAsState().value, - backClick = remember(viewModel) { - { viewModel.trySendAction(VaultItemListingsAction.BackClick) } - }, - searchIconClick = remember(viewModel) { - { viewModel.trySendAction(VaultItemListingsAction.SearchIconClick) } - }, - addVaultItemClick = remember(viewModel) { - { viewModel.trySendAction(VaultItemListingsAction.AddVaultItemClick) } - }, - vaultItemClick = remember(viewModel) { - { viewModel.trySendAction(VaultItemListingsAction.ItemClick(it)) } - }, - refreshClick = remember(viewModel) { - { viewModel.trySendAction(VaultItemListingsAction.RefreshClick) } + state = state, + vaultItemListingHandlers = remember(viewModel) { + VaultItemListingHandlers.create(viewModel) }, ) } +@Composable +private fun VaultItemListingDialogs( + dialogState: VaultItemListingState.DialogState?, +) { + when (dialogState) { + is VaultItemListingState.DialogState.Loading -> BitwardenLoadingDialog( + visibilityState = LoadingDialogState.Shown(dialogState.message), + ) + + null -> Unit + } +} + @OptIn(ExperimentalMaterial3Api::class) @Suppress("LongMethod") @Composable private fun VaultItemListingScaffold( state: VaultItemListingState, - backClick: () -> Unit, - searchIconClick: () -> Unit, - addVaultItemClick: () -> Unit, - vaultItemClick: (id: String) -> Unit, - refreshClick: () -> Unit, + vaultItemListingHandlers: VaultItemListingHandlers, ) { val scrollBehavior = TopAppBarDefaults.pinnedScrollBehavior(rememberTopAppBarState()) BitwardenScaffold( @@ -115,13 +124,24 @@ private fun VaultItemListingScaffold( scrollBehavior = scrollBehavior, navigationIcon = painterResource(id = R.drawable.ic_back), navigationIconContentDescription = stringResource(id = R.string.back), - onNavigationIconClick = backClick, + onNavigationIconClick = vaultItemListingHandlers.backClick, actions = { BitwardenSearchActionItem( contentDescription = stringResource(id = R.string.search_vault), - onClick = searchIconClick, + onClick = vaultItemListingHandlers.searchIconClick, + ) + BitwardenOverflowActionItem( + menuItemDataList = persistentListOf( + OverflowMenuItemData( + text = stringResource(id = R.string.sync), + onClick = vaultItemListingHandlers.syncClick, + ), + OverflowMenuItemData( + text = stringResource(id = R.string.lock), + onClick = vaultItemListingHandlers.lockClick, + ), + ), ) - BitwardenOverflowActionItem() }, ) }, @@ -129,7 +149,7 @@ private fun VaultItemListingScaffold( if (state.itemListingType.hasFab) { FloatingActionButton( containerColor = MaterialTheme.colorScheme.primaryContainer, - onClick = addVaultItemClick, + onClick = vaultItemListingHandlers.addVaultItemClick, ) { Icon( painter = painterResource(id = R.drawable.ic_plus), @@ -147,7 +167,7 @@ private fun VaultItemListingScaffold( is VaultItemListingState.ViewState.Content -> { VaultItemListingContent( state = state.viewState, - vaultItemClick = vaultItemClick, + vaultItemClick = vaultItemListingHandlers.itemClick, modifier = modifier, ) } @@ -155,7 +175,7 @@ private fun VaultItemListingScaffold( is VaultItemListingState.ViewState.NoItems -> { VaultItemListingEmpty( itemListingType = state.itemListingType, - addItemClickAction = addVaultItemClick, + addItemClickAction = vaultItemListingHandlers.addVaultItemClick, modifier = modifier, ) } @@ -163,7 +183,7 @@ private fun VaultItemListingScaffold( is VaultItemListingState.ViewState.Error -> { BitwardenErrorContent( message = state.viewState.message(), - onTryAgainClick = refreshClick, + onTryAgainClick = vaultItemListingHandlers.refreshClick, 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 3c3ef08616..5f8a9bea54 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 @@ -1,5 +1,6 @@ package com.x8bit.bitwarden.ui.vault.feature.itemlisting +import android.os.Parcelable import androidx.lifecycle.SavedStateHandle import androidx.lifecycle.viewModelScope import com.x8bit.bitwarden.R @@ -22,6 +23,7 @@ import dagger.hilt.android.lifecycle.HiltViewModel import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.flow.update +import kotlinx.parcelize.Parcelize import javax.inject.Inject /** @@ -43,6 +45,7 @@ class VaultItemListingViewModel @Inject constructor( viewState = VaultItemListingState.ViewState.Loading, baseIconUrl = environmentRepository.environment.environmentUrlData.baseIconUrl, isIconLoadingDisabled = settingsRepository.isIconLoadingDisabled, + dialogState = null, ), ) { @@ -61,6 +64,8 @@ class VaultItemListingViewModel @Inject constructor( override fun handleAction(action: VaultItemListingsAction) { when (action) { is VaultItemListingsAction.BackClick -> handleBackClick() + is VaultItemListingsAction.LockClick -> handleLockClick() + is VaultItemListingsAction.SyncClick -> handleSyncClick() is VaultItemListingsAction.SearchIconClick -> handleSearchIconClick() is VaultItemListingsAction.ItemClick -> handleItemClick(action) is VaultItemListingsAction.AddVaultItemClick -> handleAddVaultItemClick() @@ -108,6 +113,21 @@ class VaultItemListingViewModel @Inject constructor( ) } + private fun handleLockClick() { + vaultRepository.lockVaultForCurrentUser() + } + + private fun handleSyncClick() { + mutableStateFlow.update { + it.copy( + dialogState = VaultItemListingState.DialogState.Loading( + message = R.string.syncing.asText(), + ), + ) + } + vaultRepository.sync() + } + private fun handleSearchIconClick() { sendEvent( event = VaultItemListingEvent.NavigateToVaultSearchScreen, @@ -134,27 +154,28 @@ class VaultItemListingViewModel @Inject constructor( } vaultRepository.vaultDataStateFlow.value.data?.let { vaultData -> - updateStateWithVaultData(vaultData) + updateStateWithVaultData(vaultData, clearDialogState = false) } } //endregion VaultItemListing Handlers private fun vaultErrorReceive(vaultData: DataState.Error) { if (vaultData.data != null) { - updateStateWithVaultData(vaultData = vaultData.data) + updateStateWithVaultData(vaultData = vaultData.data, clearDialogState = true) } else { mutableStateFlow.update { it.copy( viewState = VaultItemListingState.ViewState.Error( message = R.string.generic_error_message.asText(), ), + dialogState = null, ) } } } private fun vaultLoadedReceive(vaultData: DataState.Loaded) { - updateStateWithVaultData(vaultData = vaultData.data) + updateStateWithVaultData(vaultData = vaultData.data, clearDialogState = true) } private fun vaultLoadingReceive() { @@ -163,7 +184,7 @@ class VaultItemListingViewModel @Inject constructor( private fun vaultNoNetworkReceive(vaultData: DataState.NoNetwork) { if (vaultData.data != null) { - updateStateWithVaultData(vaultData = vaultData.data) + updateStateWithVaultData(vaultData = vaultData.data, clearDialogState = true) } else { mutableStateFlow.update { currentState -> currentState.copy( @@ -172,16 +193,17 @@ class VaultItemListingViewModel @Inject constructor( .asText() .concat(R.string.internet_connection_required_message.asText()), ), + dialogState = null, ) } } } private fun vaultPendingReceive(vaultData: DataState.Pending) { - updateStateWithVaultData(vaultData = vaultData.data) + updateStateWithVaultData(vaultData = vaultData.data, clearDialogState = false) } - private fun updateStateWithVaultData(vaultData: VaultData) { + private fun updateStateWithVaultData(vaultData: VaultData, clearDialogState: Boolean) { mutableStateFlow.update { currentState -> currentState.copy( itemListingType = currentState @@ -214,6 +236,7 @@ class VaultItemListingViewModel @Inject constructor( .toViewState() } }, + dialogState = currentState.dialogState.takeUnless { clearDialogState }, ) } } @@ -227,8 +250,23 @@ data class VaultItemListingState( val viewState: ViewState, val baseIconUrl: String, val isIconLoadingDisabled: Boolean, + val dialogState: DialogState?, ) { + /** + * Represents the current state of any dialogs on the screen. + */ + sealed class DialogState : Parcelable { + + /** + * Represents a loading dialog with the given [message]. + */ + @Parcelize + data class Loading( + val message: Text, + ) : DialogState() + } + /** * Represents the specific view states for the [VaultItemListingScreen]. */ @@ -450,6 +488,16 @@ sealed class VaultItemListingsAction { */ data object RefreshClick : VaultItemListingsAction() + /** + * Click the lock button. + */ + data object LockClick : VaultItemListingsAction() + + /** + * Click the refresh button. + */ + data object SyncClick : VaultItemListingsAction() + /** * Click the back button. */ diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/handlers/VaultItemListingHandlers.kt b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/handlers/VaultItemListingHandlers.kt new file mode 100644 index 0000000000..c2a0745060 --- /dev/null +++ b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/itemlisting/handlers/VaultItemListingHandlers.kt @@ -0,0 +1,41 @@ +package com.x8bit.bitwarden.ui.vault.feature.itemlisting.handlers + +import com.x8bit.bitwarden.ui.vault.feature.itemlisting.VaultItemListingViewModel +import com.x8bit.bitwarden.ui.vault.feature.itemlisting.VaultItemListingsAction + +/** + * A collection of handler functions for managing actions within the context of viewing a list of + * items. + */ +data class VaultItemListingHandlers( + val backClick: () -> Unit, + val searchIconClick: () -> Unit, + val addVaultItemClick: () -> Unit, + val itemClick: (id: String) -> Unit, + val refreshClick: () -> Unit, + val syncClick: () -> Unit, + val lockClick: () -> Unit, +) { + companion object { + /** + * Creates an instance of [VaultItemListingHandlers] by binding actions to the provided + * [VaultItemListingViewModel]. + */ + fun create( + viewModel: VaultItemListingViewModel, + ): VaultItemListingHandlers = + VaultItemListingHandlers( + backClick = { viewModel.trySendAction(VaultItemListingsAction.BackClick) }, + searchIconClick = { + viewModel.trySendAction(VaultItemListingsAction.SearchIconClick) + }, + addVaultItemClick = { + viewModel.trySendAction(VaultItemListingsAction.AddVaultItemClick) + }, + itemClick = { viewModel.trySendAction(VaultItemListingsAction.ItemClick(it)) }, + refreshClick = { viewModel.trySendAction(VaultItemListingsAction.RefreshClick) }, + syncClick = { viewModel.trySendAction(VaultItemListingsAction.SyncClick) }, + lockClick = { viewModel.trySendAction(VaultItemListingsAction.LockClick) }, + ) + } +} 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 ebc285f084..bf35c835c5 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 @@ -1,10 +1,17 @@ package com.x8bit.bitwarden.ui.vault.feature.itemlisting +import androidx.compose.ui.test.assert import androidx.compose.ui.test.assertIsDisplayed import androidx.compose.ui.test.assertIsNotDisplayed import androidx.compose.ui.test.assertTextEquals +import androidx.compose.ui.test.filterToOne +import androidx.compose.ui.test.hasAnyAncestor import androidx.compose.ui.test.hasScrollToNodeAction import androidx.compose.ui.test.hasText +import androidx.compose.ui.test.isDialog +import androidx.compose.ui.test.isDisplayed +import androidx.compose.ui.test.isPopup +import androidx.compose.ui.test.onAllNodesWithText import androidx.compose.ui.test.onNodeWithContentDescription import androidx.compose.ui.test.onNodeWithText import androidx.compose.ui.test.performClick @@ -420,6 +427,74 @@ class VaultItemListingScreenTest : BaseComposeTest() { .onNodeWithText(text = "mockName") .assertIsDisplayed() } + + @Test + fun `on overflow item click should display menu`() { + composeTestRule + .onNodeWithContentDescription(label = "More") + .performClick() + + composeTestRule + .onAllNodesWithText(text = "Sync") + .filterToOne(hasAnyAncestor(isPopup())) + .isDisplayed() + composeTestRule + .onAllNodesWithText(text = "Lock") + .filterToOne(hasAnyAncestor(isPopup())) + .isDisplayed() + } + + @Test + fun `on sync click should send SyncClick`() { + composeTestRule + .onNodeWithContentDescription(label = "More") + .performClick() + + composeTestRule + .onAllNodesWithText(text = "Sync") + .filterToOne(hasAnyAncestor(isPopup())) + .performClick() + + verify { + viewModel.trySendAction(VaultItemListingsAction.SyncClick) + } + } + + @Test + fun `on lock click should send LockClick`() { + composeTestRule + .onNodeWithContentDescription(label = "More") + .performClick() + + composeTestRule + .onAllNodesWithText(text = "Lock") + .filterToOne(hasAnyAncestor(isPopup())) + .performClick() + + verify { + viewModel.trySendAction(VaultItemListingsAction.LockClick) + } + } + + @Test + fun `loading dialog should be displayed according to state`() { + val loadingMessage = "syncing" + composeTestRule.onNode(isDialog()).assertDoesNotExist() + composeTestRule.onNodeWithText(loadingMessage).assertDoesNotExist() + + mutableStateFlow.update { + it.copy( + dialogState = VaultItemListingState.DialogState.Loading( + message = loadingMessage.asText(), + ), + ) + } + + composeTestRule + .onNodeWithText(loadingMessage) + .assertIsDisplayed() + .assert(hasAnyAncestor(isDialog())) + } } private val DEFAULT_STATE = VaultItemListingState( @@ -427,6 +502,7 @@ private val DEFAULT_STATE = VaultItemListingState( viewState = VaultItemListingState.ViewState.Loading, isIconLoadingDisabled = false, baseIconUrl = Environment.Us.environmentUrlData.baseIconUrl, + dialogState = null, ) private fun createDisplayItem(number: Int): VaultItemListingState.DisplayItem = 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 f526dc22e0..2cc7002383 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 @@ -21,8 +21,10 @@ import com.x8bit.bitwarden.ui.platform.base.util.concat import com.x8bit.bitwarden.ui.vault.feature.itemlisting.util.createMockItemListingDisplayItem import com.x8bit.bitwarden.ui.vault.model.VaultItemListingType import io.mockk.every +import io.mockk.just import io.mockk.mockk import io.mockk.mockkStatic +import io.mockk.runs import io.mockk.unmockkStatic import io.mockk.verify import kotlinx.coroutines.flow.MutableStateFlow @@ -38,7 +40,7 @@ class VaultItemListingViewModelTest : BaseViewModelTest() { MutableStateFlow>(DataState.Loading) private val vaultRepository: VaultRepository = mockk { every { vaultDataStateFlow } returns mutableVaultDataStateFlow - every { sync() } returns Unit + every { sync() } just runs } private val environmentRepository: EnvironmentRepository = mockk { every { environment } returns Environment.Us @@ -83,6 +85,37 @@ class VaultItemListingViewModelTest : BaseViewModelTest() { } } + @Test + fun `LockClick should call lockVaultForCurrentUser`() { + every { vaultRepository.lockVaultForCurrentUser() } just runs + val viewModel = createVaultItemListingViewModel() + + viewModel.trySendAction(VaultItemListingsAction.LockClick) + + verify(exactly = 1) { + vaultRepository.lockVaultForCurrentUser() + } + } + + @Test + fun `SyncClick should display the loading dialog and call sync`() { + val viewModel = createVaultItemListingViewModel() + + viewModel.trySendAction(VaultItemListingsAction.SyncClick) + + assertEquals( + initialState.copy( + dialogState = VaultItemListingState.DialogState.Loading( + message = R.string.syncing.asText(), + ), + ), + viewModel.stateFlow.value, + ) + verify(exactly = 1) { + vaultRepository.sync() + } + } + @Test fun `ItemClick for vault item should emit NavigateToVaultItem`() = runTest { val viewModel = createVaultItemListingViewModel() @@ -559,5 +592,6 @@ class VaultItemListingViewModelTest : BaseViewModelTest() { viewState = viewState, baseIconUrl = environmentRepository.environment.environmentUrlData.baseIconUrl, isIconLoadingDisabled = settingsRepository.isIconLoadingDisabled, + dialogState = null, ) }