From e3001848f598eed7fd9fba73797a93f83791006c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20G=C3=B6ransson?= Date: Thu, 8 Aug 2024 09:39:31 +0200 Subject: [PATCH 1/2] Move to CollectSideEffectWithLifecycle --- ...DeleteApiAccessMethodConfirmationDialog.kt | 4 +- .../DeleteCustomListConfirmationDialog.kt | 4 +- .../mullvadvpn/compose/dialog/DnsDialog.kt | 4 +- .../dialog/EditCustomListNameDialog.kt | 4 +- .../mullvadvpn/compose/dialog/MtuDialog.kt | 4 +- .../dialog/SaveApiAccessMethodDialog.kt | 4 +- .../compose/dialog/payment/PaymentDialog.kt | 4 +- .../compose/screen/AccountScreen.kt | 59 +++++++++---------- .../screen/ApiAccessMethodDetailsScreen.kt | 4 +- .../screen/CustomListLocationsScreen.kt | 4 +- .../compose/screen/DeviceRevokedScreen.kt | 4 +- .../screen/EditApiAccessMethodScreen.kt | 4 +- .../mullvadvpn/compose/screen/FilterScreen.kt | 4 +- .../mullvadvpn/compose/screen/MullvadApp.kt | 43 ++++++++------ .../compose/screen/PrivacyDisclaimerScreen.kt | 4 +- .../compose/screen/ReportProblemScreen.kt | 4 +- .../compose/screen/ServerIpOverridesScreen.kt | 4 +- .../compose/screen/VpnSettingsScreen.kt | 4 +- .../mullvad/mullvadvpn/compose/util/Effect.kt | 9 --- .../viewmodel/CustomListLocationsViewModel.kt | 12 ++-- 20 files changed, 91 insertions(+), 96 deletions(-) diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/dialog/DeleteApiAccessMethodConfirmationDialog.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/dialog/DeleteApiAccessMethodConfirmationDialog.kt index e6605ac5da2e..d829e4650e1d 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/dialog/DeleteApiAccessMethodConfirmationDialog.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/dialog/DeleteApiAccessMethodConfirmationDialog.kt @@ -10,7 +10,7 @@ import com.ramcosta.composedestinations.result.ResultBackNavigator import com.ramcosta.composedestinations.spec.DestinationStyle import net.mullvad.mullvadvpn.R import net.mullvad.mullvadvpn.compose.state.DeleteApiAccessMethodUiState -import net.mullvad.mullvadvpn.compose.util.LaunchedEffectCollect +import net.mullvad.mullvadvpn.compose.util.CollectSideEffectWithLifecycle import net.mullvad.mullvadvpn.lib.model.ApiAccessMethodId import net.mullvad.mullvadvpn.lib.theme.AppTheme import net.mullvad.mullvadvpn.viewmodel.DeleteApiAccessMethodConfirmationSideEffect @@ -34,7 +34,7 @@ fun DeleteApiAccessMethodConfirmation(navigator: ResultBackNavigator) { val viewModel = koinViewModel() val state = viewModel.uiState.collectAsStateWithLifecycle() - LaunchedEffectCollect(viewModel.uiSideEffect) { + CollectSideEffectWithLifecycle(viewModel.uiSideEffect) { when (it) { is DeleteApiAccessMethodConfirmationSideEffect.Deleted -> navigator.navigateBack(result = true) diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/dialog/DeleteCustomListConfirmationDialog.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/dialog/DeleteCustomListConfirmationDialog.kt index d89c04fab714..e8f146f37e8e 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/dialog/DeleteCustomListConfirmationDialog.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/dialog/DeleteCustomListConfirmationDialog.kt @@ -13,7 +13,7 @@ import com.ramcosta.composedestinations.spec.DestinationStyle import net.mullvad.mullvadvpn.R import net.mullvad.mullvadvpn.compose.communication.CustomListActionResultData import net.mullvad.mullvadvpn.compose.state.DeleteCustomListUiState -import net.mullvad.mullvadvpn.compose.util.LaunchedEffectCollect +import net.mullvad.mullvadvpn.compose.util.CollectSideEffectWithLifecycle import net.mullvad.mullvadvpn.lib.model.CustomListId import net.mullvad.mullvadvpn.lib.model.CustomListName import net.mullvad.mullvadvpn.lib.theme.AppTheme @@ -42,7 +42,7 @@ fun DeleteCustomList(navigator: ResultBackNavigator navigator.navigateBack(result = it.result) diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/dialog/DnsDialog.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/dialog/DnsDialog.kt index 242236e619c1..7561747e5306 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/dialog/DnsDialog.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/dialog/DnsDialog.kt @@ -23,7 +23,7 @@ import net.mullvad.mullvadvpn.compose.button.NegativeButton import net.mullvad.mullvadvpn.compose.button.PrimaryButton import net.mullvad.mullvadvpn.compose.communication.DnsDialogResult import net.mullvad.mullvadvpn.compose.textfield.DnsTextField -import net.mullvad.mullvadvpn.compose.util.LaunchedEffectCollect +import net.mullvad.mullvadvpn.compose.util.CollectSideEffectWithLifecycle import net.mullvad.mullvadvpn.lib.theme.AppTheme import net.mullvad.mullvadvpn.lib.theme.Dimens import net.mullvad.mullvadvpn.viewmodel.DnsDialogSideEffect @@ -57,7 +57,7 @@ data class DnsDialogNavArgs(val index: Int? = null, val initialValue: String? = fun Dns(resultNavigator: ResultBackNavigator) { val viewModel = koinViewModel() - LaunchedEffectCollect(viewModel.uiSideEffect) { + CollectSideEffectWithLifecycle(viewModel.uiSideEffect) { when (it) { DnsDialogSideEffect.Complete -> resultNavigator.navigateBack(result = DnsDialogResult.Success) diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/dialog/EditCustomListNameDialog.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/dialog/EditCustomListNameDialog.kt index 5fbcb831a92a..dcece689f437 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/dialog/EditCustomListNameDialog.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/dialog/EditCustomListNameDialog.kt @@ -21,7 +21,7 @@ import net.mullvad.mullvadvpn.compose.communication.CustomListActionResultData import net.mullvad.mullvadvpn.compose.state.EditCustomListNameUiState import net.mullvad.mullvadvpn.compose.test.EDIT_CUSTOM_LIST_DIALOG_INPUT_TEST_TAG import net.mullvad.mullvadvpn.compose.textfield.CustomListNameTextField -import net.mullvad.mullvadvpn.compose.util.LaunchedEffectCollect +import net.mullvad.mullvadvpn.compose.util.CollectSideEffectWithLifecycle import net.mullvad.mullvadvpn.lib.model.CustomListId import net.mullvad.mullvadvpn.lib.model.CustomListName import net.mullvad.mullvadvpn.lib.model.GetCustomListError @@ -53,7 +53,7 @@ fun EditCustomListName( backNavigator: ResultBackNavigator ) { val vm: EditCustomListNameDialogViewModel = koinViewModel() - LaunchedEffectCollect(vm.uiSideEffect) { sideEffect -> + CollectSideEffectWithLifecycle(vm.uiSideEffect) { sideEffect -> when (sideEffect) { is EditCustomListNameDialogSideEffect.ReturnWithResult -> { backNavigator.navigateBack(result = sideEffect.result) diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/dialog/MtuDialog.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/dialog/MtuDialog.kt index d97cfde97295..bbf05d550df5 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/dialog/MtuDialog.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/dialog/MtuDialog.kt @@ -23,7 +23,7 @@ import net.mullvad.mullvadvpn.R import net.mullvad.mullvadvpn.compose.button.NegativeButton import net.mullvad.mullvadvpn.compose.button.PrimaryButton import net.mullvad.mullvadvpn.compose.textfield.MtuTextField -import net.mullvad.mullvadvpn.compose.util.LaunchedEffectCollect +import net.mullvad.mullvadvpn.compose.util.CollectSideEffectWithLifecycle import net.mullvad.mullvadvpn.constant.MTU_MAX_VALUE import net.mullvad.mullvadvpn.constant.MTU_MIN_VALUE import net.mullvad.mullvadvpn.lib.model.Mtu @@ -48,7 +48,7 @@ fun Mtu(navigator: ResultBackNavigator) { val viewModel = koinViewModel() val uiState by viewModel.uiState.collectAsStateWithLifecycle() - LaunchedEffectCollect(viewModel.uiSideEffect) { + CollectSideEffectWithLifecycle(viewModel.uiSideEffect) { when (it) { MtuDialogSideEffect.Complete -> navigator.navigateBack(result = true) MtuDialogSideEffect.Error -> navigator.navigateBack(result = false) diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/dialog/SaveApiAccessMethodDialog.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/dialog/SaveApiAccessMethodDialog.kt index c5fb2d45d673..6182f0eb8194 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/dialog/SaveApiAccessMethodDialog.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/dialog/SaveApiAccessMethodDialog.kt @@ -27,7 +27,7 @@ import net.mullvad.mullvadvpn.compose.state.TestApiAccessMethodState import net.mullvad.mullvadvpn.compose.test.SAVE_API_ACCESS_METHOD_CANCEL_BUTTON_TEST_TAG import net.mullvad.mullvadvpn.compose.test.SAVE_API_ACCESS_METHOD_LOADING_SPINNER_TEST_TAG import net.mullvad.mullvadvpn.compose.test.SAVE_API_ACCESS_METHOD_SAVE_BUTTON_TEST_TAG -import net.mullvad.mullvadvpn.compose.util.LaunchedEffectCollect +import net.mullvad.mullvadvpn.compose.util.CollectSideEffectWithLifecycle import net.mullvad.mullvadvpn.lib.model.ApiAccessMethod import net.mullvad.mullvadvpn.lib.model.ApiAccessMethodId import net.mullvad.mullvadvpn.lib.model.ApiAccessMethodName @@ -59,7 +59,7 @@ data class SaveApiAccessMethodNavArgs( fun SaveApiAccessMethod(backNavigator: ResultBackNavigator) { val viewModel = koinViewModel() - LaunchedEffectCollect(sideEffect = viewModel.uiSideEffect) { + CollectSideEffectWithLifecycle(viewModel.uiSideEffect) { when (it) { SaveApiAccessMethodSideEffect.CouldNotSaveApiAccessMethod -> backNavigator.navigateBack(result = false) diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/dialog/payment/PaymentDialog.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/dialog/payment/PaymentDialog.kt index d1cfcdfebcb8..1001490339e1 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/dialog/payment/PaymentDialog.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/dialog/payment/PaymentDialog.kt @@ -20,7 +20,7 @@ import com.ramcosta.composedestinations.spec.DestinationStyle import net.mullvad.mullvadvpn.R import net.mullvad.mullvadvpn.compose.button.PrimaryButton import net.mullvad.mullvadvpn.compose.component.MullvadCircularProgressIndicatorMedium -import net.mullvad.mullvadvpn.compose.util.LaunchedEffectCollect +import net.mullvad.mullvadvpn.compose.util.CollectSideEffectWithLifecycle import net.mullvad.mullvadvpn.lib.payment.model.ProductId import net.mullvad.mullvadvpn.lib.theme.AppTheme import net.mullvad.mullvadvpn.util.getActivity @@ -125,7 +125,7 @@ fun Payment(productId: ProductId, resultBackNavigator: ResultBackNavigator() val state by vm.uiState.collectAsStateWithLifecycle() - LaunchedEffectCollect(vm.uiSideEffect) { sideEffect -> + CollectSideEffectWithLifecycle(vm.uiSideEffect) { sideEffect -> when (sideEffect) { PaymentUiSideEffect.PaymentCancelled -> resultBackNavigator.navigateBack(result = false) } diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/AccountScreen.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/AccountScreen.kt index d51dddd40b1e..8239646bca43 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/AccountScreen.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/AccountScreen.kt @@ -36,9 +36,6 @@ import com.ramcosta.composedestinations.generated.destinations.VerificationPendi import com.ramcosta.composedestinations.navigation.DestinationsNavigator import com.ramcosta.composedestinations.result.NavResult import com.ramcosta.composedestinations.result.ResultRecipient -import kotlinx.coroutines.flow.Flow -import kotlinx.coroutines.flow.MutableSharedFlow -import kotlinx.coroutines.flow.asSharedFlow import kotlinx.coroutines.launch import net.mullvad.mullvadvpn.R import net.mullvad.mullvadvpn.compose.button.ExternalButton @@ -54,7 +51,7 @@ import net.mullvad.mullvadvpn.compose.extensions.createOpenAccountPageHook import net.mullvad.mullvadvpn.compose.extensions.dropUnlessResumed import net.mullvad.mullvadvpn.compose.state.PaymentState import net.mullvad.mullvadvpn.compose.transitions.SlideInFromBottomTransition -import net.mullvad.mullvadvpn.compose.util.LaunchedEffectCollect +import net.mullvad.mullvadvpn.compose.util.CollectSideEffectWithLifecycle import net.mullvad.mullvadvpn.compose.util.SecureScreenWhileInView import net.mullvad.mullvadvpn.compose.util.createCopyToClipboardHandle import net.mullvad.mullvadvpn.compose.util.showSnackbarImmediately @@ -98,8 +95,7 @@ private fun PreviewAccountScreen() { ), ) ), - ), - uiSideEffect = MutableSharedFlow().asSharedFlow(), + ) ) } } @@ -114,6 +110,29 @@ fun Account( val vm = koinViewModel() val state by vm.uiState.collectAsStateWithLifecycle() + val snackbarHostState = remember { SnackbarHostState() } + val copyTextString = stringResource(id = R.string.copied_mullvad_account_number) + val errorString = stringResource(id = R.string.error_occurred) + val copyToClipboard = createCopyToClipboardHandle(snackbarHostState = snackbarHostState) + val openAccountPage = LocalUriHandler.current.createOpenAccountPageHook() + + CollectSideEffectWithLifecycle(vm.uiSideEffect) { sideEffect -> + when (sideEffect) { + AccountViewModel.UiSideEffect.NavigateToLogin -> { + navigator.navigate(LoginDestination(null)) { + launchSingleTop = true + popUpTo(NavGraphs.root) { inclusive = true } + } + } + is AccountViewModel.UiSideEffect.OpenAccountManagementPageInBrowser -> + openAccountPage(sideEffect.token) + is AccountViewModel.UiSideEffect.CopyAccountNumber -> + launch { copyToClipboard(sideEffect.accountNumber, copyTextString) } + AccountViewModel.UiSideEffect.GenericError -> + snackbarHostState.showSnackbarImmediately(message = errorString) + } + } + playPaymentResultRecipient.onNavResult { when (it) { NavResult.Canceled -> { @@ -125,16 +144,10 @@ fun Account( AccountScreen( state = state, - uiSideEffect = vm.uiSideEffect, + snackbarHostState = snackbarHostState, onRedeemVoucherClick = dropUnlessResumed { navigator.navigate(RedeemVoucherDestination) }, onManageAccountClick = vm::onManageAccountClick, onLogoutClick = vm::onLogoutClick, - navigateToLogin = { - navigator.navigate(LoginDestination(null)) { - launchSingleTop = true - popUpTo(NavGraphs.root) { inclusive = true } - } - }, onCopyAccountNumber = vm::onCopyAccountNumber, onBackClick = dropUnlessResumed { navigator.navigateUp() }, navigateToDeviceInfo = dropUnlessResumed { navigator.navigate(DeviceNameInfoDestination) }, @@ -149,13 +162,12 @@ fun Account( @Composable fun AccountScreen( state: AccountUiState, - uiSideEffect: Flow, + snackbarHostState: SnackbarHostState = remember { SnackbarHostState() }, onCopyAccountNumber: (String) -> Unit = {}, onRedeemVoucherClick: () -> Unit = {}, onManageAccountClick: () -> Unit = {}, onLogoutClick: () -> Unit = {}, onPurchaseBillingProductClick: (productId: ProductId) -> Unit = { _ -> }, - navigateToLogin: () -> Unit = {}, navigateToDeviceInfo: () -> Unit = {}, navigateToVerificationPendingDialog: () -> Unit = {}, onBackClick: () -> Unit = {}, @@ -163,23 +175,6 @@ fun AccountScreen( // This will enable SECURE_FLAG while this screen is visible to preview screenshot SecureScreenWhileInView() - val snackbarHostState = remember { SnackbarHostState() } - val copyTextString = stringResource(id = R.string.copied_mullvad_account_number) - val errorString = stringResource(id = R.string.error_occurred) - val copyToClipboard = createCopyToClipboardHandle(snackbarHostState = snackbarHostState) - val openAccountPage = LocalUriHandler.current.createOpenAccountPageHook() - LaunchedEffectCollect(uiSideEffect) { sideEffect -> - when (sideEffect) { - AccountViewModel.UiSideEffect.NavigateToLogin -> navigateToLogin() - is AccountViewModel.UiSideEffect.OpenAccountManagementPageInBrowser -> - openAccountPage(sideEffect.token) - is AccountViewModel.UiSideEffect.CopyAccountNumber -> - launch { copyToClipboard(sideEffect.accountNumber, copyTextString) } - AccountViewModel.UiSideEffect.GenericError -> - snackbarHostState.showSnackbarImmediately(message = errorString) - } - } - ScaffoldWithMediumTopBar( appBarTitle = stringResource(id = R.string.settings_account), navigationIcon = { NavigateBackDownIconButton(onBackClick) }, diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/ApiAccessMethodDetailsScreen.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/ApiAccessMethodDetailsScreen.kt index f8265389c1a1..3d67c60c076f 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/ApiAccessMethodDetailsScreen.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/ApiAccessMethodDetailsScreen.kt @@ -55,7 +55,7 @@ import net.mullvad.mullvadvpn.compose.test.API_ACCESS_TEST_METHOD_BUTTON import net.mullvad.mullvadvpn.compose.test.API_ACCESS_USE_METHOD_BUTTON import net.mullvad.mullvadvpn.compose.test.DELETE_DROPDOWN_MENU_ITEM_TEST_TAG import net.mullvad.mullvadvpn.compose.transitions.SlideInFromRightTransition -import net.mullvad.mullvadvpn.compose.util.LaunchedEffectCollect +import net.mullvad.mullvadvpn.compose.util.CollectSideEffectWithLifecycle import net.mullvad.mullvadvpn.compose.util.OnNavResultValue import net.mullvad.mullvadvpn.compose.util.showSnackbarImmediately import net.mullvad.mullvadvpn.lib.model.ApiAccessMethodId @@ -94,7 +94,7 @@ fun ApiAccessMethodDetails( val context = LocalContext.current val coroutineScope = rememberCoroutineScope() - LaunchedEffectCollect(sideEffect = viewModel.uiSideEffect) { + CollectSideEffectWithLifecycle(viewModel.uiSideEffect) { when (it) { ApiAccessMethodDetailsSideEffect.GenericError -> launch { diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/CustomListLocationsScreen.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/CustomListLocationsScreen.kt index b7c97f92d6c1..1cab54d0488d 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/CustomListLocationsScreen.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/CustomListLocationsScreen.kt @@ -45,7 +45,7 @@ import net.mullvad.mullvadvpn.compose.test.CIRCULAR_PROGRESS_INDICATOR import net.mullvad.mullvadvpn.compose.test.SAVE_BUTTON_TEST_TAG import net.mullvad.mullvadvpn.compose.textfield.SearchTextField import net.mullvad.mullvadvpn.compose.transitions.SlideInFromRightTransition -import net.mullvad.mullvadvpn.compose.util.LaunchedEffectCollect +import net.mullvad.mullvadvpn.compose.util.CollectSideEffectWithLifecycle import net.mullvad.mullvadvpn.lib.model.CustomListId import net.mullvad.mullvadvpn.lib.model.RelayItem import net.mullvad.mullvadvpn.lib.theme.Dimens @@ -85,7 +85,7 @@ fun CustomListLocations( } } - LaunchedEffectCollect(customListsViewModel.uiSideEffect) { sideEffect -> + CollectSideEffectWithLifecycle(customListsViewModel.uiSideEffect) { sideEffect -> when (sideEffect) { is CustomListLocationsSideEffect.ReturnWithResultData -> backNavigator.navigateBack(result = sideEffect.result) diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/DeviceRevokedScreen.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/DeviceRevokedScreen.kt index 7408e51e5f1e..8cf8f9b23690 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/DeviceRevokedScreen.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/DeviceRevokedScreen.kt @@ -31,7 +31,7 @@ import net.mullvad.mullvadvpn.R import net.mullvad.mullvadvpn.compose.button.DeviceRevokedLoginButton import net.mullvad.mullvadvpn.compose.component.ScaffoldWithTopBar import net.mullvad.mullvadvpn.compose.state.DeviceRevokedUiState -import net.mullvad.mullvadvpn.compose.util.LaunchedEffectCollect +import net.mullvad.mullvadvpn.compose.util.CollectSideEffectWithLifecycle import net.mullvad.mullvadvpn.lib.theme.AppTheme import net.mullvad.mullvadvpn.lib.theme.Dimens import net.mullvad.mullvadvpn.viewmodel.DeviceRevokedSideEffect @@ -51,7 +51,7 @@ fun DeviceRevoked(navigator: DestinationsNavigator) { val state by viewModel.uiState.collectAsStateWithLifecycle() - LaunchedEffectCollect(viewModel.uiSideEffect) { sideEffect -> + CollectSideEffectWithLifecycle(viewModel.uiSideEffect) { sideEffect -> when (sideEffect) { DeviceRevokedSideEffect.NavigateToLogin -> navigator.navigate(LoginDestination()) { diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/EditApiAccessMethodScreen.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/EditApiAccessMethodScreen.kt index 8c6c59e59a42..bab3bbcc5067 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/EditApiAccessMethodScreen.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/EditApiAccessMethodScreen.kt @@ -57,7 +57,7 @@ import net.mullvad.mullvadvpn.compose.test.EDIT_API_ACCESS_NAME_INPUT import net.mullvad.mullvadvpn.compose.textfield.ApiAccessMethodTextField import net.mullvad.mullvadvpn.compose.textfield.apiAccessTextFieldColors import net.mullvad.mullvadvpn.compose.transitions.SlideInFromRightTransition -import net.mullvad.mullvadvpn.compose.util.LaunchedEffectCollect +import net.mullvad.mullvadvpn.compose.util.CollectSideEffectWithLifecycle import net.mullvad.mullvadvpn.compose.util.OnNavResultValue import net.mullvad.mullvadvpn.compose.util.showSnackbarImmediately import net.mullvad.mullvadvpn.lib.model.ApiAccessMethodId @@ -102,7 +102,7 @@ fun EditApiAccessMethod( val context = LocalContext.current val scope = rememberCoroutineScope() - LaunchedEffectCollect(sideEffect = viewModel.uiSideEffect) { + CollectSideEffectWithLifecycle(viewModel.uiSideEffect) { when (it) { is EditApiAccessSideEffect.OpenSaveDialog -> navigator.navigate( diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/FilterScreen.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/FilterScreen.kt index 61f865b9ec14..0ce854a29f25 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/FilterScreen.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/FilterScreen.kt @@ -41,7 +41,7 @@ import net.mullvad.mullvadvpn.compose.extensions.itemWithDivider import net.mullvad.mullvadvpn.compose.extensions.itemsWithDivider import net.mullvad.mullvadvpn.compose.state.RelayFilterState import net.mullvad.mullvadvpn.compose.transitions.SlideInFromRightTransition -import net.mullvad.mullvadvpn.compose.util.LaunchedEffectCollect +import net.mullvad.mullvadvpn.compose.util.CollectSideEffectWithLifecycle import net.mullvad.mullvadvpn.lib.model.Ownership import net.mullvad.mullvadvpn.lib.model.Provider import net.mullvad.mullvadvpn.lib.theme.AppTheme @@ -75,7 +75,7 @@ fun Filter(navigator: DestinationsNavigator) { val viewModel = koinViewModel() val state by viewModel.uiState.collectAsStateWithLifecycle() - LaunchedEffectCollect(viewModel.uiSideEffect) { + CollectSideEffectWithLifecycle(viewModel.uiSideEffect) { when (it) { FilterScreenSideEffect.CloseScreen -> navigator.navigateUp() } diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/MullvadApp.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/MullvadApp.kt index ac54689c35d6..0fb165ea89b9 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/MullvadApp.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/MullvadApp.kt @@ -4,6 +4,7 @@ import androidx.activity.compose.rememberLauncherForActivityResult import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.runtime.Composable import androidx.compose.runtime.DisposableEffect +import androidx.compose.runtime.LaunchedEffect import androidx.compose.ui.ExperimentalComposeUiApi import androidx.compose.ui.Modifier import androidx.compose.ui.semantics.semantics @@ -22,7 +23,6 @@ import com.ramcosta.composedestinations.utils.destination import com.ramcosta.composedestinations.utils.rememberDestinationsNavigator import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.map -import net.mullvad.mullvadvpn.compose.util.LaunchedEffectCollect import net.mullvad.mullvadvpn.compose.util.RequestVpnPermission import net.mullvad.mullvadvpn.viewmodel.ChangelogViewModel import net.mullvad.mullvadvpn.viewmodel.DaemonScreenEvent @@ -55,34 +55,43 @@ fun MullvadApp() { navGraph = NavGraphs.root, ) + // For the following LaunchedEffect we do not use CollectSideEffectWithLifecycle since we + // collect from StateFlow/SharedFlow with replay and don't want to trigger a navigation again. + // Globally handle daemon dropped connection with NoDaemonScreen - LaunchedEffectCollect(serviceVm.uiSideEffect) { - Logger.i { "DaemonScreenEvent: $it" } - when (it) { - DaemonScreenEvent.Show -> - navigator.navigate(NoDaemonDestination) { launchSingleTop = true } - DaemonScreenEvent.Remove -> navigator.popBackStack(NoDaemonDestination, true) + LaunchedEffect(Unit) { + serviceVm.uiSideEffect.collect { + Logger.i { "DaemonScreenEvent: $it" } + when (it) { + DaemonScreenEvent.Show -> + navigator.navigate(NoDaemonDestination) { launchSingleTop = true } + + DaemonScreenEvent.Remove -> navigator.popBackStack(NoDaemonDestination, true) + } } } // Globally show the changelog val changeLogsViewModel = koinViewModel() - LaunchedEffectCollect(changeLogsViewModel.uiSideEffect) { + LaunchedEffect(Unit) { + changeLogsViewModel.uiSideEffect.collect { + // Wait until we are in an acceptable destination + navHostController.currentBackStackEntryFlow + .map { it.destination() } + .first { it in changeLogDestinations } - // Wait until we are in an acceptable destination - navHostController.currentBackStackEntryFlow - .map { it.destination() } - .first { it in changeLogDestinations } - - navigator.navigate(ChangelogDestination(it)) + navigator.navigate(ChangelogDestination(it)) + } } // Ask for VPN Permission val launchVpnPermission = rememberLauncherForActivityResult(RequestVpnPermission()) { _ -> permissionVm.connect() } - LaunchedEffectCollect(permissionVm.uiSideEffect) { - if (it is VpnPermissionSideEffect.ShowDialog) { - launchVpnPermission.launch(Unit) + LaunchedEffect(Unit) { + changeLogsViewModel.uiSideEffect.collect { + if (it is VpnPermissionSideEffect.ShowDialog) { + launchVpnPermission.launch(Unit) + } } } } diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/PrivacyDisclaimerScreen.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/PrivacyDisclaimerScreen.kt index 3094fe17722a..605e789edfa7 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/PrivacyDisclaimerScreen.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/PrivacyDisclaimerScreen.kt @@ -47,7 +47,7 @@ import net.mullvad.mullvadvpn.compose.button.PrimaryButton import net.mullvad.mullvadvpn.compose.component.MullvadCircularProgressIndicatorMedium import net.mullvad.mullvadvpn.compose.component.ScaffoldWithTopBar import net.mullvad.mullvadvpn.compose.component.drawVerticalScrollbar -import net.mullvad.mullvadvpn.compose.util.LaunchedEffectCollect +import net.mullvad.mullvadvpn.compose.util.CollectSideEffectWithLifecycle import net.mullvad.mullvadvpn.compose.util.toDp import net.mullvad.mullvadvpn.constant.DAEMON_READY_TIMEOUT_MS import net.mullvad.mullvadvpn.lib.common.util.openLink @@ -80,7 +80,7 @@ fun PrivacyDisclaimer(navigator: DestinationsNavigator) { val state by viewModel.uiState.collectAsStateWithLifecycle() val context = LocalContext.current - LaunchedEffectCollect(viewModel.uiSideEffect) { + CollectSideEffectWithLifecycle(viewModel.uiSideEffect) { when (it) { PrivacyDisclaimerUiSideEffect.NavigateToLogin -> navigator.navigate(LoginDestination(null)) { diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/ReportProblemScreen.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/ReportProblemScreen.kt index 09cc4e1cdcaa..7f1190a2adee 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/ReportProblemScreen.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/ReportProblemScreen.kt @@ -43,7 +43,7 @@ import net.mullvad.mullvadvpn.compose.component.NavigateBackIconButton import net.mullvad.mullvadvpn.compose.component.ScaffoldWithMediumTopBar import net.mullvad.mullvadvpn.compose.textfield.mullvadWhiteTextFieldColors import net.mullvad.mullvadvpn.compose.transitions.SlideInFromRightTransition -import net.mullvad.mullvadvpn.compose.util.LaunchedEffectCollect +import net.mullvad.mullvadvpn.compose.util.CollectSideEffectWithLifecycle import net.mullvad.mullvadvpn.compose.util.SecureScreenWhileInView import net.mullvad.mullvadvpn.dataproxy.SendProblemReportResult import net.mullvad.mullvadvpn.lib.theme.AppTheme @@ -104,7 +104,7 @@ fun ReportProblem( val vm = koinViewModel() val state by vm.uiState.collectAsStateWithLifecycle() - LaunchedEffectCollect(vm.uiSideEffect) { + CollectSideEffectWithLifecycle(vm.uiSideEffect) { when (it) { is ReportProblemSideEffect.ShowConfirmNoEmail -> navigator.navigate(ReportProblemNoEmailDestination) diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/ServerIpOverridesScreen.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/ServerIpOverridesScreen.kt index 3f3b8312ed5a..88d72072aec6 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/ServerIpOverridesScreen.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/ServerIpOverridesScreen.kt @@ -66,7 +66,7 @@ import net.mullvad.mullvadvpn.compose.test.SERVER_IP_OVERRIDE_INFO_TEST_TAG import net.mullvad.mullvadvpn.compose.test.SERVER_IP_OVERRIDE_MORE_VERT_TEST_TAG import net.mullvad.mullvadvpn.compose.test.SERVER_IP_OVERRIDE_RESET_OVERRIDES_TEST_TAG import net.mullvad.mullvadvpn.compose.transitions.SlideInFromRightLeafTransition -import net.mullvad.mullvadvpn.compose.util.LaunchedEffectCollect +import net.mullvad.mullvadvpn.compose.util.CollectSideEffectWithLifecycle import net.mullvad.mullvadvpn.compose.util.OnNavResultValue import net.mullvad.mullvadvpn.compose.util.showSnackbarImmediately import net.mullvad.mullvadvpn.lib.model.SettingsPatchError @@ -106,7 +106,7 @@ fun ServerIpOverrides( val snackbarHostState = remember { SnackbarHostState() } val context = LocalContext.current - LaunchedEffectCollect(vm.uiSideEffect) { sideEffect -> + CollectSideEffectWithLifecycle(vm.uiSideEffect) { sideEffect -> when (sideEffect) { is ServerIpOverridesUiSideEffect.ImportResult -> launch { diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/VpnSettingsScreen.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/VpnSettingsScreen.kt index 7358d4cd16fc..1bccbaa713c0 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/VpnSettingsScreen.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/VpnSettingsScreen.kt @@ -86,7 +86,7 @@ import net.mullvad.mullvadvpn.compose.test.LAZY_LIST_WIREGUARD_CUSTOM_PORT_NUMBE import net.mullvad.mullvadvpn.compose.test.LAZY_LIST_WIREGUARD_CUSTOM_PORT_TEXT_TEST_TAG import net.mullvad.mullvadvpn.compose.test.LAZY_LIST_WIREGUARD_PORT_ITEM_X_TEST_TAG import net.mullvad.mullvadvpn.compose.transitions.SlideInFromRightTransition -import net.mullvad.mullvadvpn.compose.util.LaunchedEffectCollect +import net.mullvad.mullvadvpn.compose.util.CollectSideEffectWithLifecycle import net.mullvad.mullvadvpn.compose.util.OnNavResultValue import net.mullvad.mullvadvpn.compose.util.showSnackbarImmediately import net.mullvad.mullvadvpn.constant.UDP2TCP_PRESET_PORTS @@ -178,7 +178,7 @@ fun VpnSettings( val snackbarHostState = remember { SnackbarHostState() } val context = LocalContext.current - LaunchedEffectCollect(vm.uiSideEffect) { + CollectSideEffectWithLifecycle(vm.uiSideEffect) { when (it) { is VpnSettingsSideEffect.ShowToast -> launch { snackbarHostState.showSnackbarImmediately(message = it.message(context)) } diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/util/Effect.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/util/Effect.kt index fa0c22eca0df..e1c2a9d5917e 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/util/Effect.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/util/Effect.kt @@ -11,15 +11,6 @@ import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.launch -@Composable -inline fun LaunchedEffectCollect( - sideEffect: Flow, - key: Any = Unit, - crossinline collector: suspend CoroutineScope.(T) -> Unit, -) { - LaunchedEffect(key) { sideEffect.collect { collector(it) } } -} - // This function will restart collection on Start/Stop events, e.g if the user navigates to home // screen collection will stop, and then be restarted when the user opens the app again @Composable diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/CustomListLocationsViewModel.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/CustomListLocationsViewModel.kt index 83b9d7961150..df6037a0b4c9 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/CustomListLocationsViewModel.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/CustomListLocationsViewModel.kt @@ -6,11 +6,12 @@ import androidx.lifecycle.viewModelScope import arrow.core.getOrElse import arrow.core.raise.either import com.ramcosta.composedestinations.generated.destinations.CustomListLocationsDestination -import kotlinx.coroutines.flow.MutableSharedFlow +import kotlinx.coroutines.channels.Channel +import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow -import kotlinx.coroutines.flow.SharedFlow import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.combine +import kotlinx.coroutines.flow.consumeAsFlow import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.onEach @@ -41,9 +42,8 @@ class CustomListLocationsViewModel( ) : ViewModel() { private val navArgs = CustomListLocationsDestination.argsFrom(savedStateHandle = savedStateHandle) - private val _uiSideEffect = - MutableSharedFlow(replay = 1, extraBufferCapacity = 1) - val uiSideEffect: SharedFlow = _uiSideEffect + private val _uiSideEffect = Channel() + val uiSideEffect: Flow = _uiSideEffect.consumeAsFlow() private val _initialLocations = MutableStateFlow>(emptySet()) private val _selectedLocations = MutableStateFlow?>(null) @@ -122,7 +122,7 @@ class CustomListLocationsViewModel( calculateResultData(success, locationsToSave) } .getOrElse { CustomListActionResultData.GenericError } - _uiSideEffect.tryEmit( + _uiSideEffect.send( CustomListLocationsSideEffect.ReturnWithResultData(result = result) ) } From 0cdcfb8c58245654ee4249f100e61ce646cfc7d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20G=C3=B6ransson?= Date: Mon, 2 Sep 2024 14:41:14 +0200 Subject: [PATCH 2/2] Fix tests --- .../compose/screen/AccountScreenTest.kt | 27 ++++--------------- 1 file changed, 5 insertions(+), 22 deletions(-) diff --git a/android/app/src/androidTest/kotlin/net/mullvad/mullvadvpn/compose/screen/AccountScreenTest.kt b/android/app/src/androidTest/kotlin/net/mullvad/mullvadvpn/compose/screen/AccountScreenTest.kt index 3a55dfb9fd51..beb310452d7d 100644 --- a/android/app/src/androidTest/kotlin/net/mullvad/mullvadvpn/compose/screen/AccountScreenTest.kt +++ b/android/app/src/androidTest/kotlin/net/mullvad/mullvadvpn/compose/screen/AccountScreenTest.kt @@ -9,8 +9,6 @@ import io.mockk.MockKAnnotations import io.mockk.every import io.mockk.mockk import io.mockk.verify -import kotlinx.coroutines.flow.MutableSharedFlow -import kotlinx.coroutines.flow.asSharedFlow import net.mullvad.mullvadvpn.compose.createEdgeToEdgeComposeExtension import net.mullvad.mullvadvpn.compose.setContentWithTheme import net.mullvad.mullvadvpn.compose.state.PaymentState @@ -21,7 +19,6 @@ import net.mullvad.mullvadvpn.lib.payment.model.PaymentStatus import net.mullvad.mullvadvpn.lib.payment.model.ProductId import net.mullvad.mullvadvpn.lib.payment.model.ProductPrice import net.mullvad.mullvadvpn.viewmodel.AccountUiState -import net.mullvad.mullvadvpn.viewmodel.AccountViewModel import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test import org.junit.jupiter.api.extension.RegisterExtension @@ -48,8 +45,7 @@ class AccountScreenTest { accountNumber = DUMMY_ACCOUNT_NUMBER, accountExpiry = null, showSitePayment = false, - ), - uiSideEffect = MutableSharedFlow().asSharedFlow(), + ) ) } @@ -72,8 +68,6 @@ class AccountScreenTest { accountNumber = DUMMY_ACCOUNT_NUMBER, accountExpiry = null, ), - uiSideEffect = - MutableSharedFlow().asSharedFlow(), onManageAccountClick = mockedClickHandler, ) } @@ -99,8 +93,6 @@ class AccountScreenTest { accountExpiry = null, showSitePayment = false, ), - uiSideEffect = - MutableSharedFlow().asSharedFlow(), onRedeemVoucherClick = mockedClickHandler, ) } @@ -126,8 +118,6 @@ class AccountScreenTest { accountExpiry = null, showSitePayment = false, ), - uiSideEffect = - MutableSharedFlow().asSharedFlow(), onLogoutClick = mockedClickHandler, ) } @@ -147,8 +137,7 @@ class AccountScreenTest { AccountScreen( state = AccountUiState.default() - .copy(billingPaymentState = PaymentState.Error.Billing), - uiSideEffect = MutableSharedFlow().asSharedFlow(), + .copy(billingPaymentState = PaymentState.Error.Billing) ) } @@ -170,8 +159,7 @@ class AccountScreenTest { .copy( billingPaymentState = PaymentState.PaymentAvailable(listOf(mockPaymentProduct)) - ), - uiSideEffect = MutableSharedFlow().asSharedFlow(), + ) ) } @@ -193,8 +181,7 @@ class AccountScreenTest { .copy( billingPaymentState = PaymentState.PaymentAvailable(listOf(mockPaymentProduct)) - ), - uiSideEffect = MutableSharedFlow().asSharedFlow(), + ) ) } @@ -218,8 +205,6 @@ class AccountScreenTest { billingPaymentState = PaymentState.PaymentAvailable(listOf(mockPaymentProduct)) ), - uiSideEffect = - MutableSharedFlow().asSharedFlow(), navigateToVerificationPendingDialog = mockNavigateToVerificationPending, ) } @@ -245,8 +230,7 @@ class AccountScreenTest { .copy( billingPaymentState = PaymentState.PaymentAvailable(listOf(mockPaymentProduct)) - ), - uiSideEffect = MutableSharedFlow().asSharedFlow(), + ) ) } @@ -272,7 +256,6 @@ class AccountScreenTest { PaymentState.PaymentAvailable(listOf(mockPaymentProduct)) ), onPurchaseBillingProductClick = clickHandler, - uiSideEffect = MutableSharedFlow().asSharedFlow(), ) }