From bd27b438d2316f02f91050c753bb0a30c10de8d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20G=C3=B6ransson?= Date: Tue, 23 Jul 2024 16:05:58 +0200 Subject: [PATCH] Refactor select location view even more --- .../mullvadvpn/compose/cell/FilterCell.kt | 56 +- .../compose/screen/SelectLocationScreen.kt | 585 ++++++++---------- .../compose/state/SelectLocationUiState.kt | 18 +- .../net/mullvad/mullvadvpn/di/UiModule.kt | 17 +- .../CustomListsRelayItemUseCase.kt | 26 + .../viewmodel/SelectLocationViewModel.kt | 127 ++-- .../viewmodel/SelectLocationViewModelTest.kt | 8 +- 7 files changed, 394 insertions(+), 443 deletions(-) diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/cell/FilterCell.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/cell/FilterCell.kt index 6dfd8f3eb1ce..1a70c6c001cd 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/cell/FilterCell.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/cell/FilterCell.kt @@ -2,10 +2,8 @@ package net.mullvad.mullvadvpn.compose.cell import androidx.compose.foundation.horizontalScroll import androidx.compose.foundation.layout.Row -import androidx.compose.foundation.layout.Spacer import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.padding -import androidx.compose.foundation.layout.size import androidx.compose.foundation.rememberScrollState import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Text @@ -16,6 +14,7 @@ import androidx.compose.ui.res.stringResource import androidx.compose.ui.tooling.preview.Preview import net.mullvad.mullvadvpn.R import net.mullvad.mullvadvpn.compose.component.MullvadFilterChip +import net.mullvad.mullvadvpn.compose.state.FilterChip import net.mullvad.mullvadvpn.lib.model.Ownership import net.mullvad.mullvadvpn.lib.theme.AppTheme import net.mullvad.mullvadvpn.lib.theme.Dimens @@ -23,22 +22,14 @@ import net.mullvad.mullvadvpn.lib.theme.Dimens @Preview @Composable private fun PreviewFilterCell() { - AppTheme { - FilterCell( - ownershipFilter = Ownership.MullvadOwned, - selectedProviderFilter = 3, - removeOwnershipFilter = {}, - removeProviderFilter = {} - ) - } + AppTheme { FilterRow(emptyList(), {}, {}) } } @Composable -fun FilterCell( - ownershipFilter: Ownership?, - selectedProviderFilter: Int?, - removeOwnershipFilter: () -> Unit, - removeProviderFilter: () -> Unit +fun FilterRow( + filters: List, + onRemoveOwnershipFilter: () -> Unit, + onRemoveProviderFilter: () -> Unit ) { val scrollState = rememberScrollState() Row( @@ -54,26 +45,29 @@ fun FilterCell( modifier = Modifier.padding(end = Dimens.filterTittlePadding), text = stringResource(id = R.string.filtered), color = MaterialTheme.colorScheme.onPrimary, - style = MaterialTheme.typography.labelMedium - ) - - if (selectedProviderFilter != null) { - MullvadFilterChip( - text = stringResource(id = R.string.number_of_providers, selectedProviderFilter), - onRemoveClick = removeProviderFilter - ) - Spacer(modifier = Modifier.size(Dimens.chipSpace)) - } - - if (ownershipFilter != null) { - MullvadFilterChip( - text = stringResource(ownershipFilter.stringResources()), - onRemoveClick = removeOwnershipFilter - ) + style = MaterialTheme.typography.labelMedium) + filters.forEach { + when (it) { + is FilterChip.Ownership -> OwnershipFilterChip(it.ownership, onRemoveOwnershipFilter) + is FilterChip.Provider -> ProviderFilterChip(it.count, onRemoveProviderFilter) + } } } } +@Composable +fun ProviderFilterChip(providers: Int, onRemoveClick: () -> Unit) { + MullvadFilterChip( + text = stringResource(id = R.string.number_of_providers, providers), + onRemoveClick = onRemoveClick) +} + +@Composable +fun OwnershipFilterChip(ownership: Ownership, onRemoveClick: () -> Unit) { + MullvadFilterChip( + text = stringResource(ownership.stringResources()), onRemoveClick = onRemoveClick) +} + private fun Ownership.stringResources(): Int = when (this) { Ownership.MullvadOwned -> R.string.owned diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/SelectLocationScreen.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/SelectLocationScreen.kt index b92079f626af..8083e23162d8 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/SelectLocationScreen.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/SelectLocationScreen.kt @@ -63,7 +63,7 @@ import com.ramcosta.composedestinations.spec.DestinationSpec import kotlinx.coroutines.launch import mullvad_daemon.management_interface.customList import net.mullvad.mullvadvpn.R -import net.mullvad.mullvadvpn.compose.cell.FilterCell +import net.mullvad.mullvadvpn.compose.cell.FilterRow import net.mullvad.mullvadvpn.compose.cell.HeaderCell import net.mullvad.mullvadvpn.compose.cell.IconCell import net.mullvad.mullvadvpn.compose.cell.RelayItemCell @@ -117,12 +117,10 @@ private fun PreviewSelectLocationScreen() { val state = SelectLocationUiState.Content( searchTerm = "", - selectedOwnership = null, - selectedProvidersCount = 0, + emptyList(), relayListItems = emptyList(), customLists = emptyList(), - selectedItem = null - ) + ) AppTheme { SelectLocationScreen( state = state, @@ -155,43 +153,30 @@ fun SelectLocation( is SelectLocationSideEffect.LocationAddedToCustomList -> launch { snackbarHostState.showResultSnackbar( - context = context, - result = it.result, - onUndo = vm::performAction - ) + context = context, result = it.result, onUndo = vm::performAction) } is SelectLocationSideEffect.LocationRemovedFromCustomList -> launch { snackbarHostState.showResultSnackbar( - context = context, - result = it.result, - onUndo = vm::performAction - ) + context = context, result = it.result, onUndo = vm::performAction) } SelectLocationSideEffect.GenericError -> launch { snackbarHostState.showSnackbarImmediately( message = context.getString(R.string.error_occurred), - duration = SnackbarDuration.Short - ) + duration = SnackbarDuration.Short) } } } createCustomListDialogResultRecipient.OnCustomListNavResult( - snackbarHostState, - vm::performAction - ) + snackbarHostState, vm::performAction) editCustomListNameDialogResultRecipient.OnCustomListNavResult( - snackbarHostState, - vm::performAction - ) + snackbarHostState, vm::performAction) deleteCustomListDialogResultRecipient.OnCustomListNavResult( - snackbarHostState, - vm::performAction - ) + snackbarHostState, vm::performAction) updateCustomListResultRecipient.OnCustomListNavResult(snackbarHostState, vm::performAction) @@ -218,9 +203,7 @@ fun SelectLocation( dropUnlessResumed { customList: RelayItem.CustomList -> navigator.navigate( EditCustomListNameDestination( - customListId = customList.id, - initialName = customList.customList.name - ), + customListId = customList.id, initialName = customList.customList.name), ) }, onEditLocationsCustomList = @@ -233,12 +216,9 @@ fun SelectLocation( dropUnlessResumed { customList: RelayItem.CustomList -> navigator.navigate( DeleteCustomListDestination( - customListId = customList.id, - name = customList.customList.name - ), + customListId = customList.id, name = customList.customList.name), ) - } - ) + }) } @OptIn(ExperimentalFoundationApi::class) @@ -272,156 +252,143 @@ fun SelectLocationScreen( snackbarHost = { SnackbarHost( snackbarHostState, - snackbar = { snackbarData -> MullvadSnackbar(snackbarData = snackbarData) } - ) - } - ) { - var bottomSheetState by remember { mutableStateOf(null) } - BottomSheets( - bottomSheetState = bottomSheetState, - onCreateCustomList = onCreateCustomList, - onEditCustomLists = onEditCustomLists, - onAddLocationToList = onAddLocationToList, - onRemoveLocationFromList = onRemoveLocationFromList, - onEditCustomListName = onEditCustomListName, - onEditLocationsCustomList = onEditLocationsCustomList, - onDeleteCustomList = onDeleteCustomList, - onHideBottomSheet = { bottomSheetState = null } - ) + snackbar = { snackbarData -> MullvadSnackbar(snackbarData = snackbarData) }) + }) { + var bottomSheetState by remember { mutableStateOf(null) } + BottomSheets( + bottomSheetState = bottomSheetState, + onCreateCustomList = onCreateCustomList, + onEditCustomLists = onEditCustomLists, + onAddLocationToList = onAddLocationToList, + onRemoveLocationFromList = onRemoveLocationFromList, + onEditCustomListName = onEditCustomListName, + onEditLocationsCustomList = onEditLocationsCustomList, + onDeleteCustomList = onDeleteCustomList, + onHideBottomSheet = { bottomSheetState = null }) - Column(modifier = Modifier.padding(it).background(backgroundColor).fillMaxSize()) { - SelectLocationTopBar(onBackClick = onBackClick, onFilterClick = onFilterClick) - - when (state) { - SelectLocationUiState.Loading -> {} - is SelectLocationUiState.Content -> { - if (state.hasFilter) { - FilterCell( - ownershipFilter = state.selectedOwnership, - selectedProviderFilter = state.selectedProvidersCount, - removeOwnershipFilter = removeOwnershipFilter, - removeProviderFilter = removeProviderFilter, - ) + Column(modifier = Modifier.padding(it).background(backgroundColor).fillMaxSize()) { + SelectLocationTopBar(onBackClick = onBackClick, onFilterClick = onFilterClick) + + when (state) { + SelectLocationUiState.Loading -> {} + is SelectLocationUiState.Content -> { + if (state.filterChips.isNotEmpty()) { + FilterRow( + filters = state.filterChips, + removeOwnershipFilter, + removeProviderFilter) + } } } - } - SearchTextField( - modifier = - Modifier.fillMaxWidth() - .height(Dimens.searchFieldHeight) - .padding(horizontal = Dimens.searchFieldHorizontalPadding), - backgroundColor = MaterialTheme.colorScheme.tertiaryContainer, - textColor = MaterialTheme.colorScheme.onTertiaryContainer, - ) { searchString -> - onSearchTermInput.invoke(searchString) - } - Spacer(modifier = Modifier.height(height = Dimens.verticalSpace)) - val lazyListState = rememberLazyListState() - val selectedItemCode = (state as? SelectLocationUiState.Content)?.selectedItem ?: "" - RunOnKeyChange(key = selectedItemCode) { - val index = state.indexOfSelectedRelayItem() - - if (index >= 0) { - lazyListState.scrollToItem(index) - lazyListState.animateScrollAndCentralizeItem(index) + SearchTextField( + modifier = + Modifier.fillMaxWidth() + .height(Dimens.searchFieldHeight) + .padding(horizontal = Dimens.searchFieldHorizontalPadding), + backgroundColor = MaterialTheme.colorScheme.tertiaryContainer, + textColor = MaterialTheme.colorScheme.onTertiaryContainer, + ) { searchString -> + onSearchTermInput.invoke(searchString) } - } - - LazyColumn( - modifier = - Modifier.fillMaxSize() - .drawVerticalScrollbar( - lazyListState, - MaterialTheme.colorScheme.onBackground.copy(alpha = AlphaScrollbar), - ), - state = lazyListState, - horizontalAlignment = Alignment.CenterHorizontally, - ) { - when (state) { - SelectLocationUiState.Loading -> { - loading() - } - is SelectLocationUiState.Content -> { - itemsIndexed( - items = state.relayListItems, - key = { index: Int, item: RelayListItem -> item.key }, - contentType = { _, _ -> null }, - itemContent = { index: Int, listItem: RelayListItem -> - Column(modifier = Modifier.animateItem()) { - if (index != 0) { - HorizontalDivider(color = backgroundColor) - } - when (listItem) { - RelayListItem.CustomListHeader -> - CustomListHeader( - onShowCustomListBottomSheet = { - bottomSheetState = - ShowCustomListsBottomSheet( - editListEnabled = state.customLists.isNotEmpty() - ) - } - ) - is RelayListItem.CustomListItem -> - CustomListItem( - listItem, - onSelectRelay, - { - bottomSheetState = - ShowEditCustomListBottomSheet(it) - }, - { customListId, expand -> - onToggleExpand(customListId, null, expand) - } - ) - is RelayListItem.CustomListEntryItem -> - CustomListEntryItem( - listItem, - { onSelectRelay(listItem.item) }, - { - bottomSheetState = - ShowCustomListsEntryBottomSheet( + Spacer(modifier = Modifier.height(height = Dimens.verticalSpace)) + val lazyListState = rememberLazyListState() + +// val selectedItemCode = (state as? SelectLocationUiState.Content)?.selectedItem ?: "" +// RunOnKeyChange(key = selectedItemCode) { +// val index = state.indexOfSelectedRelayItem() +// +// if (index >= 0) { +// lazyListState.scrollToItem(index) +// lazyListState.animateScrollAndCentralizeItem(index) +// } +// } + + LazyColumn( + modifier = + Modifier.fillMaxSize() + .drawVerticalScrollbar( + lazyListState, + MaterialTheme.colorScheme.onBackground.copy(alpha = AlphaScrollbar), + ), + state = lazyListState, + horizontalAlignment = Alignment.CenterHorizontally, + ) { + when (state) { + SelectLocationUiState.Loading -> { + loading() + } + is SelectLocationUiState.Content -> { + itemsIndexed( + items = state.relayListItems, + key = { index: Int, item: RelayListItem -> item.key }, + contentType = { _, _ -> null }, + itemContent = { index: Int, listItem: RelayListItem -> + Column(modifier = Modifier.animateItem()) { + if (index != 0) { + HorizontalDivider(color = backgroundColor) + } + when (listItem) { + RelayListItem.CustomListHeader -> + CustomListHeader( + onShowCustomListBottomSheet = { + bottomSheetState = + ShowCustomListsBottomSheet( + editListEnabled = + state.customLists.isNotEmpty()) + }) + is RelayListItem.CustomListItem -> + CustomListItem( + listItem, + onSelectRelay, + { + bottomSheetState = + ShowEditCustomListBottomSheet(it) + }, + { customListId, expand -> + onToggleExpand(customListId, null, expand) + }) + is RelayListItem.CustomListEntryItem -> + CustomListEntryItem( + listItem, + { onSelectRelay(listItem.item) }, + { + bottomSheetState = + ShowCustomListsEntryBottomSheet( + listItem.parentId, listItem.item) + }, + { expand: Boolean -> + onToggleExpand( + listItem.item.id, listItem.parentId, - listItem.item - ) - }, - { expand: Boolean -> - onToggleExpand( - listItem.item.id, - listItem.parentId, - expand - ) - } - ) - is RelayListItem.CustomListFooter -> - CustomListFooter(listItem) - RelayListItem.LocationHeader -> RelayLocationHeader() - is RelayListItem.GeoLocationItem -> - RelayLocationItem( - listItem, - { onSelectRelay(listItem.item) }, - { - bottomSheetState = - ShowLocationBottomSheet( - state.customLists, - listItem.item - ) - }, - { expand -> - onToggleExpand(listItem.item.id, null, expand) - } - ) - is RelayListItem.LocationsEmptyText -> - LocationsEmptyText(listItem.searchTerm) + expand) + }) + is RelayListItem.CustomListFooter -> + CustomListFooter(listItem) + RelayListItem.LocationHeader -> RelayLocationHeader() + is RelayListItem.GeoLocationItem -> + RelayLocationItem( + listItem, + { onSelectRelay(listItem.item) }, + { + bottomSheetState = + ShowLocationBottomSheet( + state.customLists, listItem.item) + }, + { expand -> + onToggleExpand( + listItem.item.id, null, expand) + }) + is RelayListItem.LocationsEmptyText -> + LocationsEmptyText(listItem.searchTerm) + } } - } - } - ) + }) + } } } } } - } } @Composable @@ -444,8 +411,7 @@ fun LazyItemScope.RelayLocationItem( { onLongClick() }, { onExpand(it) }, relayItem.expanded, - relayItem.depth - ) + relayItem.depth) } @Composable @@ -463,8 +429,7 @@ fun LazyItemScope.CustomListItem( { onShowEditBottomSheet(customListItem) }, { onExpand(customListItem.id, it) }, itemState.expanded, - 0 - ) + 0) } @Composable @@ -482,8 +447,7 @@ fun LazyItemScope.CustomListEntryItem( onShowEditCustomListEntryBottomSheet, onToggleExpand, itemState.expanded, - itemState.depth - ) + itemState.depth) } @Composable @@ -495,8 +459,7 @@ fun LazyItemScope.CustomListFooter(item: RelayListItem.CustomListFooter) { } else { stringResource(R.string.to_create_a_custom_list) }, - modifier = Modifier.background(MaterialTheme.colorScheme.background) - ) + modifier = Modifier.background(MaterialTheme.colorScheme.background)) } @Composable @@ -538,8 +501,7 @@ private fun LazyItemScope.CustomListHeader(onShowCustomListBottomSheet: () -> Un ThreeDotCell( text = stringResource(R.string.custom_lists), onClickDots = onShowCustomListBottomSheet, - modifier = Modifier.testTag(SELECT_LOCATION_CUSTOM_LIST_HEADER_TEST_TAG) - ) + modifier = Modifier.testTag(SELECT_LOCATION_CUSTOM_LIST_HEADER_TEST_TAG)) } @OptIn(ExperimentalMaterial3Api::class) @@ -580,8 +542,7 @@ private fun BottomSheets( bottomSheetState = bottomSheetState, onCreateCustomList = { onCreateCustomList(null) }, onEditCustomLists = onEditCustomLists, - closeBottomSheet = onCloseBottomSheet - ) + closeBottomSheet = onCloseBottomSheet) } is ShowLocationBottomSheet -> { LocationBottomSheet( @@ -591,8 +552,7 @@ private fun BottomSheets( item = bottomSheetState.item, onCreateCustomList = onCreateCustomList, onAddLocationToList = onAddLocationToList, - closeBottomSheet = onCloseBottomSheet - ) + closeBottomSheet = onCloseBottomSheet) } is ShowEditCustomListBottomSheet -> { EditCustomListBottomSheet( @@ -602,8 +562,7 @@ private fun BottomSheets( onEditName = onEditCustomListName, onEditLocations = onEditLocationsCustomList, onDeleteCustomList = onDeleteCustomList, - closeBottomSheet = onCloseBottomSheet - ) + closeBottomSheet = onCloseBottomSheet) } is ShowCustomListsEntryBottomSheet -> { CustomListEntryBottomSheet( @@ -612,8 +571,7 @@ private fun BottomSheets( customListId = bottomSheetState.parentId, item = bottomSheetState.item, onRemoveLocationFromList = onRemoveLocationFromList, - closeBottomSheet = onCloseBottomSheet - ) + closeBottomSheet = onCloseBottomSheet) } null -> { /* Do nothing */ @@ -651,43 +609,38 @@ private fun CustomListsBottomSheet( MullvadModalBottomSheet( sheetState = sheetState, onDismissRequest = { closeBottomSheet(false) }, - modifier = Modifier.testTag(SELECT_LOCATION_CUSTOM_LIST_BOTTOM_SHEET_TEST_TAG) - ) { - HeaderCell( - text = stringResource(id = R.string.edit_custom_lists), - background = Color.Unspecified - ) - HorizontalDivider(color = onBackgroundColor) - IconCell( - iconId = R.drawable.icon_add, - title = stringResource(id = R.string.new_list), - titleColor = onBackgroundColor, - onClick = { - onCreateCustomList() - closeBottomSheet(true) - }, - background = Color.Unspecified - ) - IconCell( - iconId = R.drawable.icon_edit, - title = stringResource(id = R.string.edit_lists), - titleColor = - onBackgroundColor.copy( - alpha = - if (bottomSheetState.editListEnabled) { - AlphaVisible - } else { - AlphaInactive - } - ), - onClick = { - onEditCustomLists() - closeBottomSheet(true) - }, - background = Color.Unspecified, - enabled = bottomSheetState.editListEnabled - ) - } + modifier = Modifier.testTag(SELECT_LOCATION_CUSTOM_LIST_BOTTOM_SHEET_TEST_TAG)) { + HeaderCell( + text = stringResource(id = R.string.edit_custom_lists), + background = Color.Unspecified) + HorizontalDivider(color = onBackgroundColor) + IconCell( + iconId = R.drawable.icon_add, + title = stringResource(id = R.string.new_list), + titleColor = onBackgroundColor, + onClick = { + onCreateCustomList() + closeBottomSheet(true) + }, + background = Color.Unspecified) + IconCell( + iconId = R.drawable.icon_edit, + title = stringResource(id = R.string.edit_lists), + titleColor = + onBackgroundColor.copy( + alpha = + if (bottomSheetState.editListEnabled) { + AlphaVisible + } else { + AlphaInactive + }), + onClick = { + onEditCustomLists() + closeBottomSheet(true) + }, + background = Color.Unspecified, + enabled = bottomSheetState.editListEnabled) + } } @OptIn(ExperimentalMaterial3Api::class) @@ -704,48 +657,44 @@ private fun LocationBottomSheet( MullvadModalBottomSheet( sheetState = sheetState, onDismissRequest = { closeBottomSheet(false) }, - modifier = Modifier.testTag(SELECT_LOCATION_LOCATION_BOTTOM_SHEET_TEST_TAG) - ) { -> - HeaderCell( - text = stringResource(id = R.string.add_location_to_list, item.name), - background = Color.Unspecified - ) - HorizontalDivider(color = onBackgroundColor) - customLists.forEach { - val enabled = it.canAddLocation(item) - IconCell( - iconId = null, - title = - if (enabled) { - it.name - } else { - stringResource(id = R.string.location_added, it.name) - }, - titleColor = - if (enabled) { - onBackgroundColor - } else { - MaterialTheme.colorScheme.onSecondary + modifier = Modifier.testTag(SELECT_LOCATION_LOCATION_BOTTOM_SHEET_TEST_TAG)) { -> + HeaderCell( + text = stringResource(id = R.string.add_location_to_list, item.name), + background = Color.Unspecified) + HorizontalDivider(color = onBackgroundColor) + customLists.forEach { + val enabled = it.canAddLocation(item) + IconCell( + iconId = null, + title = + if (enabled) { + it.name + } else { + stringResource(id = R.string.location_added, it.name) + }, + titleColor = + if (enabled) { + onBackgroundColor + } else { + MaterialTheme.colorScheme.onSecondary + }, + onClick = { + onAddLocationToList(item, it) + closeBottomSheet(true) }, + background = Color.Unspecified, + enabled = enabled) + } + IconCell( + iconId = R.drawable.icon_add, + title = stringResource(id = R.string.new_list), + titleColor = onBackgroundColor, onClick = { - onAddLocationToList(item, it) + onCreateCustomList(item) closeBottomSheet(true) }, - background = Color.Unspecified, - enabled = enabled - ) + background = Color.Unspecified) } - IconCell( - iconId = R.drawable.icon_add, - title = stringResource(id = R.string.new_list), - titleColor = onBackgroundColor, - onClick = { - onCreateCustomList(item) - closeBottomSheet(true) - }, - background = Color.Unspecified - ) - } } @OptIn(ExperimentalMaterial3Api::class) @@ -760,42 +709,37 @@ private fun EditCustomListBottomSheet( closeBottomSheet: (animate: Boolean) -> Unit ) { MullvadModalBottomSheet( - sheetState = sheetState, - onDismissRequest = { closeBottomSheet(false) } - ) { - HeaderCell(text = customList.name, background = Color.Unspecified) - IconCell( - iconId = R.drawable.icon_edit, - title = stringResource(id = R.string.edit_name), - titleColor = onBackgroundColor, - onClick = { - onEditName(customList) - closeBottomSheet(true) - }, - background = Color.Unspecified - ) - IconCell( - iconId = R.drawable.icon_add, - title = stringResource(id = R.string.edit_locations), - titleColor = onBackgroundColor, - onClick = { - onEditLocations(customList) - closeBottomSheet(true) - }, - background = Color.Unspecified - ) - HorizontalDivider(color = onBackgroundColor) - IconCell( - iconId = R.drawable.icon_delete, - title = stringResource(id = R.string.delete), - titleColor = onBackgroundColor, - onClick = { - onDeleteCustomList(customList) - closeBottomSheet(true) - }, - background = Color.Unspecified - ) - } + sheetState = sheetState, onDismissRequest = { closeBottomSheet(false) }) { + HeaderCell(text = customList.name, background = Color.Unspecified) + IconCell( + iconId = R.drawable.icon_edit, + title = stringResource(id = R.string.edit_name), + titleColor = onBackgroundColor, + onClick = { + onEditName(customList) + closeBottomSheet(true) + }, + background = Color.Unspecified) + IconCell( + iconId = R.drawable.icon_add, + title = stringResource(id = R.string.edit_locations), + titleColor = onBackgroundColor, + onClick = { + onEditLocations(customList) + closeBottomSheet(true) + }, + background = Color.Unspecified) + HorizontalDivider(color = onBackgroundColor) + IconCell( + iconId = R.drawable.icon_delete, + title = stringResource(id = R.string.delete), + titleColor = onBackgroundColor, + onClick = { + onDeleteCustomList(customList) + closeBottomSheet(true) + }, + background = Color.Unspecified) + } } @OptIn(ExperimentalMaterial3Api::class) @@ -811,25 +755,22 @@ private fun CustomListEntryBottomSheet( MullvadModalBottomSheet( sheetState = sheetState, onDismissRequest = { closeBottomSheet(false) }, - modifier = Modifier.testTag(SELECT_LOCATION_LOCATION_BOTTOM_SHEET_TEST_TAG) - ) { - HeaderCell( - text = stringResource(id = R.string.remove_location_from_list, item.name), - background = Color.Unspecified - ) - HorizontalDivider(color = onBackgroundColor) - - IconCell( - iconId = R.drawable.ic_remove, - title = stringResource(id = R.string.remove_button), - titleColor = onBackgroundColor, - onClick = { - onRemoveLocationFromList(item, customListId) - closeBottomSheet(true) - }, - background = Color.Unspecified - ) - } + modifier = Modifier.testTag(SELECT_LOCATION_LOCATION_BOTTOM_SHEET_TEST_TAG)) { + HeaderCell( + text = stringResource(id = R.string.remove_location_from_list, item.name), + background = Color.Unspecified) + HorizontalDivider(color = onBackgroundColor) + + IconCell( + iconId = R.drawable.ic_remove, + title = stringResource(id = R.string.remove_button), + titleColor = onBackgroundColor, + onClick = { + onRemoveLocationFromList(item, customListId) + closeBottomSheet(true) + }, + background = Color.Unspecified) + } } private suspend fun LazyListState.animateScrollAndCentralizeItem(index: Int) { @@ -852,8 +793,7 @@ private suspend fun SnackbarHostState.showResultSnackbar( message = result.message(context), actionLabel = context.getString(R.string.undo), duration = SnackbarDuration.Long, - onAction = { onUndo(result.undo) } - ) + onAction = { onUndo(result.undo) }) } private fun CustomListSuccess.message(context: Context): String = @@ -884,10 +824,7 @@ private fun ResultRecipient // Handle result scope.launch { snackbarHostState.showResultSnackbar( - context = context, - result = result.value, - onUndo = performAction - ) + context = context, result = result.value, onUndo = performAction) } } } diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/state/SelectLocationUiState.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/state/SelectLocationUiState.kt index 9a74c63918b9..30f93d86e338 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/state/SelectLocationUiState.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/state/SelectLocationUiState.kt @@ -1,10 +1,7 @@ package net.mullvad.mullvadvpn.compose.state import net.mullvad.mullvadvpn.lib.model.CustomListId -import net.mullvad.mullvadvpn.lib.model.Ownership import net.mullvad.mullvadvpn.lib.model.RelayItem -import net.mullvad.mullvadvpn.lib.model.RelayItemId -import net.mullvad.mullvadvpn.relaylist.MIN_SEARCH_LENGTH sealed interface SelectLocationUiState { @@ -12,15 +9,16 @@ sealed interface SelectLocationUiState { data class Content( val searchTerm: String, - val selectedOwnership: Ownership?, - val selectedProvidersCount: Int?, + val filterChips: List, val relayListItems: List, val customLists: List, - val selectedItem: RelayItemId? - ) : SelectLocationUiState { - val hasFilter: Boolean = (selectedProvidersCount != null || selectedOwnership != null) - val inSearch = searchTerm.length >= MIN_SEARCH_LENGTH - } + ) : SelectLocationUiState +} + +sealed interface FilterChip { + data class Ownership(val ownership: net.mullvad.mullvadvpn.lib.model.Ownership) : FilterChip + + data class Provider(val count: Int) : FilterChip } sealed interface RelayListItem { diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/di/UiModule.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/di/UiModule.kt index f9ab7d53bf0b..8c651dd3ca58 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/di/UiModule.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/di/UiModule.kt @@ -44,6 +44,7 @@ import net.mullvad.mullvadvpn.usecase.VersionNotificationUseCase import net.mullvad.mullvadvpn.usecase.customlists.CustomListActionUseCase import net.mullvad.mullvadvpn.usecase.customlists.CustomListRelayItemsUseCase import net.mullvad.mullvadvpn.usecase.customlists.CustomListsRelayItemUseCase +import net.mullvad.mullvadvpn.usecase.customlists.FilterCustomListsRelayItemUseCase import net.mullvad.mullvadvpn.util.ChangelogDataProvider import net.mullvad.mullvadvpn.util.IChangelogDataProvider import net.mullvad.mullvadvpn.viewmodel.AccountViewModel @@ -134,6 +135,7 @@ val uiModule = module { single { CustomListActionUseCase(get(), get()) } single { SelectedLocationTitleUseCase(get(), get()) } single { AvailableProvidersUseCase(get()) } + single { FilterCustomListsRelayItemUseCase(get(), get()) } single { CustomListsRelayItemUseCase(get(), get()) } single { CustomListRelayItemsUseCase(get(), get()) } single { FilteredRelayListUseCase(get(), get()) } @@ -164,18 +166,7 @@ val uiModule = module { viewModel { ChangelogViewModel(get(), get(), BuildConfig.ALWAYS_SHOW_CHANGELOG) } viewModel { ConnectViewModel( - get(), - get(), - get(), - get(), - get(), - get(), - get(), - get(), - get(), - get(), - IS_PLAY_BUILD - ) + get(), get(), get(), get(), get(), get(), get(), get(), get(), get(), IS_PLAY_BUILD) } viewModel { DeviceListViewModel(get(), get()) } viewModel { DeviceRevokedViewModel(get(), get()) } @@ -183,7 +174,7 @@ val uiModule = module { viewModel { DnsDialogViewModel(get(), get(), get()) } viewModel { LoginViewModel(get(), get(), get()) } viewModel { PrivacyDisclaimerViewModel(get(), IS_PLAY_BUILD) } - viewModel { SelectLocationViewModel(get(), get(), get(), get(), get(), get(), get()) } + viewModel { SelectLocationViewModel(get(), get(), get(), get(), get(), get(), get(), get()) } viewModel { SettingsViewModel(get(), get(), IS_PLAY_BUILD) } viewModel { SplashViewModel(get(), get(), get(), get()) } viewModel { VoucherDialogViewModel(get()) } diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/usecase/customlists/CustomListsRelayItemUseCase.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/usecase/customlists/CustomListsRelayItemUseCase.kt index 93eb914bd0f1..c853c0f4a83e 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/usecase/customlists/CustomListsRelayItemUseCase.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/usecase/customlists/CustomListsRelayItemUseCase.kt @@ -1,8 +1,14 @@ package net.mullvad.mullvadvpn.usecase.customlists import kotlinx.coroutines.flow.combine +import net.mullvad.mullvadvpn.lib.model.Constraint +import net.mullvad.mullvadvpn.lib.model.Ownership +import net.mullvad.mullvadvpn.lib.model.Providers +import net.mullvad.mullvadvpn.lib.model.RelayItem +import net.mullvad.mullvadvpn.relaylist.filterOnOwnershipAndProvider import net.mullvad.mullvadvpn.relaylist.toRelayItemCustomList import net.mullvad.mullvadvpn.repository.CustomListsRepository +import net.mullvad.mullvadvpn.repository.RelayListFilterRepository import net.mullvad.mullvadvpn.repository.RelayListRepository class CustomListsRelayItemUseCase( @@ -17,3 +23,23 @@ class CustomListsRelayItemUseCase( customLists?.map { it.toRelayItemCustomList(relayList) } ?: emptyList() } } + +class FilterCustomListsRelayItemUseCase( + private val customListsRelayItemUseCase: CustomListsRelayItemUseCase, + private val relayListFilterRepository: RelayListFilterRepository +) { + + operator fun invoke() = + combine( + customListsRelayItemUseCase(), + relayListFilterRepository.selectedOwnership, + relayListFilterRepository.selectedProviders, + ) { customLists, selectedOwnership, selectedProviders -> + customLists.filterOnOwnershipAndProvider(selectedOwnership, selectedProviders) + } + + private fun List.filterOnOwnershipAndProvider( + ownership: Constraint, + providers: Constraint + ) = mapNotNull { it.filterOnOwnershipAndProvider(ownership, providers) } +} diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/SelectLocationViewModel.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/SelectLocationViewModel.kt index 814ef2c4697d..cf6837dc48de 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/SelectLocationViewModel.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/SelectLocationViewModel.kt @@ -6,6 +6,7 @@ import arrow.core.raise.either import kotlinx.coroutines.channels.Channel import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.SharingStarted +import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.flow.receiveAsFlow @@ -39,12 +40,13 @@ import net.mullvad.mullvadvpn.usecase.AvailableProvidersUseCase import net.mullvad.mullvadvpn.usecase.FilteredRelayListUseCase import net.mullvad.mullvadvpn.usecase.customlists.CustomListActionUseCase import net.mullvad.mullvadvpn.usecase.customlists.CustomListsRelayItemUseCase -import net.mullvad.mullvadvpn.util.combine +import net.mullvad.mullvadvpn.usecase.customlists.FilterCustomListsRelayItemUseCase class SelectLocationViewModel( private val relayListFilterRepository: RelayListFilterRepository, - availableProvidersUseCase: AvailableProvidersUseCase, - customListsRelayItemUseCase: CustomListsRelayItemUseCase, + private val availableProvidersUseCase: AvailableProvidersUseCase, + private val customListsRelayItemUseCase: CustomListsRelayItemUseCase, + private val filteredCustomListRelayItemsUseCase: FilterCustomListsRelayItemUseCase, private val customListsRepository: CustomListsRepository, private val customListActionUseCase: CustomListActionUseCase, private val filteredRelayListUseCase: FilteredRelayListUseCase, @@ -66,8 +68,7 @@ class SelectLocationViewModel( private val _expandedItems = MutableStateFlow(initialExpand()) fun searchRelayListLocations() = - kotlinx.coroutines.flow - .combine( + combine( _searchTerm, filteredRelayListUseCase(), ) { searchTerm, relayCountries -> @@ -82,68 +83,72 @@ class SelectLocationViewModel( .onEach { _expandedItems.value = it.first } .map { it.second } - @Suppress("DestructuringDeclarationWithTooManyEntries") - val uiState = + fun relayListItems() = combine( - searchRelayListLocations(), - customListsRelayItemUseCase(), - relayListRepository.selectedLocation, - _expandedItems, - _searchTerm, - relayListFilterRepository.selectedOwnership, - availableProvidersUseCase(), - relayListFilterRepository.selectedProviders, - ) { - relayCountries, - customLists, - selectedItem, - expandedItems, - searchTerm, - selectedOwnership, - allProviders, - selectedConstraintProviders -> - val selectRelayItemId = selectedItem.getOrNull() - val selectedOwnershipItem = selectedOwnership.toNullableOwnership() - val selectedProvidersCount = - when (selectedConstraintProviders) { - is Constraint.Any -> null - is Constraint.Only -> - filterSelectedProvidersByOwnership( - selectedConstraintProviders.toSelectedProviders(allProviders), - selectedOwnershipItem, - ) - .size + _searchTerm, + searchRelayListLocations(), + filteredCustomListRelayItemsUseCase(), + relayListRepository.selectedLocation, + _expandedItems, + ) { searchTerm, relayCountries, customLists, selectedItem, expandedItems -> + val filteredCustomLists = customLists.filterOnSearchTerm(searchTerm) + + createRelayListItems( + searchTerm.length >= MIN_SEARCH_LENGTH, + selectedItem.getOrNull(), + filteredCustomLists, + relayCountries, + expandedItems) + .let { + if (it.isEmpty()) { + listOf(RelayListItem.LocationsEmptyText(searchTerm)) + } else { + it } + } + } - val filteredCustomLists = - customLists - .filterOnSearchTerm(searchTerm) - .filterOnOwnershipAndProvider( - ownership = selectedOwnership, - providers = selectedConstraintProviders, - ) + fun filterChips() = + combine( + relayListFilterRepository.selectedOwnership, + relayListFilterRepository.selectedProviders, + availableProvidersUseCase(), + ) { selectedOwnership, selectedConstraintProviders, allProviders, + -> + val selectedOwnershipItem = selectedOwnership.toNullableOwnership() + val selectedProvidersCount = + when (selectedConstraintProviders) { + is Constraint.Any -> null + is Constraint.Only -> + filterSelectedProvidersByOwnership( + selectedConstraintProviders.toSelectedProviders(allProviders), + selectedOwnershipItem, + ) + .size + } + + listOfNotNull( + selectedOwnershipItem?.let { + net.mullvad.mullvadvpn.compose.state.FilterChip.Ownership(it) + }, + selectedProvidersCount?.let { + net.mullvad.mullvadvpn.compose.state.FilterChip.Provider(it) + }, + ) + } + @Suppress("DestructuringDeclarationWithTooManyEntries") + val uiState = + combine(_searchTerm, relayListItems(), filterChips(), customListsRelayItemUseCase()) { + searchTerm, + relayListItems, + filterChips, + customLists -> SelectLocationUiState.Content( searchTerm = searchTerm, - selectedOwnership = selectedOwnershipItem, - selectedProvidersCount = selectedProvidersCount, - relayListItems = - createRelayListItems( - searchTerm.length >= MIN_SEARCH_LENGTH, - selectedItem.getOrNull(), - filteredCustomLists, - relayCountries, - expandedItems) - .let { - if (it.isEmpty()) { - listOf(RelayListItem.LocationsEmptyText(searchTerm)) - } else { - it - } - }, - customLists = customLists, - selectedItem = selectRelayItemId, - ) + filterChips = filterChips, + relayListItems = relayListItems, + customLists = customLists) } .stateIn( viewModelScope, diff --git a/android/app/src/test/kotlin/net/mullvad/mullvadvpn/viewmodel/SelectLocationViewModelTest.kt b/android/app/src/test/kotlin/net/mullvad/mullvadvpn/viewmodel/SelectLocationViewModelTest.kt index 28f52ba26168..bb79b990febc 100644 --- a/android/app/src/test/kotlin/net/mullvad/mullvadvpn/viewmodel/SelectLocationViewModelTest.kt +++ b/android/app/src/test/kotlin/net/mullvad/mullvadvpn/viewmodel/SelectLocationViewModelTest.kt @@ -35,7 +35,7 @@ import net.mullvad.mullvadvpn.repository.RelayListRepository import net.mullvad.mullvadvpn.usecase.AvailableProvidersUseCase import net.mullvad.mullvadvpn.usecase.FilteredRelayListUseCase import net.mullvad.mullvadvpn.usecase.customlists.CustomListActionUseCase -import net.mullvad.mullvadvpn.usecase.customlists.CustomListsRelayItemUseCase +import net.mullvad.mullvadvpn.usecase.customlists.FilteredCustomListRelayItemsUseCase import org.junit.jupiter.api.AfterEach import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test @@ -47,7 +47,7 @@ class SelectLocationViewModelTest { private val mockRelayListFilterRepository: RelayListFilterRepository = mockk() private val mockAvailableProvidersUseCase: AvailableProvidersUseCase = mockk(relaxed = true) private val mockCustomListActionUseCase: CustomListActionUseCase = mockk(relaxed = true) - private val mockCustomListsRelayItemUseCase: CustomListsRelayItemUseCase = mockk() + private val mockFilteredCustomListRelayItemsUseCase: FilteredCustomListRelayItemsUseCase = mockk() private val mockFilteredRelayListUseCase: FilteredRelayListUseCase = mockk() private val mockRelayListRepository: RelayListRepository = mockk() @@ -68,7 +68,7 @@ class SelectLocationViewModelTest { every { mockAvailableProvidersUseCase() } returns allProviders every { mockRelayListRepository.selectedLocation } returns selectedRelayItemFlow every { mockFilteredRelayListUseCase() } returns filteredRelayList - every { mockCustomListsRelayItemUseCase() } returns customRelayListItems + every { mockFilteredCustomListRelayItemsUseCase() } returns customRelayListItems mockkStatic(RELAY_LIST_EXTENSIONS) mockkStatic(RELAY_ITEM_EXTENSIONS) @@ -77,7 +77,7 @@ class SelectLocationViewModelTest { SelectLocationViewModel( relayListFilterRepository = mockRelayListFilterRepository, availableProvidersUseCase = mockAvailableProvidersUseCase, - customListsRelayItemUseCase = mockCustomListsRelayItemUseCase, + filteredCustomListRelayItemsUseCase = mockFilteredCustomListRelayItemsUseCase, customListActionUseCase = mockCustomListActionUseCase, filteredRelayListUseCase = mockFilteredRelayListUseCase, relayListRepository = mockRelayListRepository