From 060ed9c72af1d29d065e2e7d6d68b33823a90fb9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20G=C3=B6ransson?= Date: Thu, 7 Mar 2024 08:09:56 +0100 Subject: [PATCH] Fix long method lints --- .../compose/screen/ConnectScreen.kt | 261 +++++++++++------- .../mullvadvpn/compose/screen/FilterScreen.kt | 115 +++++--- .../mullvadvpn/compose/screen/LoginScreen.kt | 160 ++++++----- .../compose/screen/OutOfTimeScreen.kt | 106 ++++--- .../compose/screen/PrivacyDisclaimerScreen.kt | 161 +++++------ .../compose/screen/SettingsScreen.kt | 196 +++++++------ 6 files changed, 558 insertions(+), 441 deletions(-) diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/ConnectScreen.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/ConnectScreen.kt index 98ac3404ca0c..91b583a70b4a 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/ConnectScreen.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/ConnectScreen.kt @@ -5,8 +5,11 @@ import android.net.Uri import androidx.compose.animation.animateContentSize import androidx.compose.animation.core.animateFloatAsState import androidx.compose.animation.core.tween +import androidx.compose.foundation.ScrollState import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.ColumnScope +import androidx.compose.foundation.layout.PaddingValues import androidx.compose.foundation.layout.Spacer import androidx.compose.foundation.layout.defaultMinSize import androidx.compose.foundation.layout.fillMaxHeight @@ -158,7 +161,6 @@ fun Connect(navigator: DestinationsNavigator) { ) } -@Suppress("LongMethod") @Composable fun ConnectScreen( state: ConnectUiState, @@ -175,15 +177,6 @@ fun ConnectScreen( ) { val scrollState = rememberScrollState() - var lastConnectionActionTimestamp by remember { mutableLongStateOf(0L) } - - fun handleThrottledAction(action: () -> Unit) { - val currentTime = System.currentTimeMillis() - if ((currentTime - lastConnectionActionTimestamp) > CONNECT_BUTTON_THROTTLE_MILLIS) { - lastConnectionActionTimestamp = currentTime - action.invoke() - } - } ScaffoldWithTopBarAndDeviceName( topBarColor = state.tunnelUiState.topBarColor(), @@ -195,45 +188,11 @@ fun ConnectScreen( ) { var progressIndicatorBias by remember { mutableFloatStateOf(0f) } - // Distance to marker when secure/unsecure - val baseZoom = - animateFloatAsState( - targetValue = - if (state.tunnelRealState is TunnelState.Connected) SECURE_ZOOM - else UNSECURE_ZOOM, - animationSpec = tween(SECURE_ZOOM_ANIMATION_MILLIS), - label = "baseZoom" - ) - - val markers = - state.tunnelRealState.toMarker(state.location)?.let { listOf(it) } ?: emptyList() - - AnimatedMap( - modifier = Modifier.padding(top = it.calculateTopPadding()), - cameraLocation = state.location?.toLatLong() ?: fallbackLatLong, - cameraBaseZoom = baseZoom.value, - cameraVerticalBias = progressIndicatorBias, - markers = markers, - globeColors = - GlobeColors( - landColor = MaterialTheme.colorScheme.primary, - oceanColor = MaterialTheme.colorScheme.secondary, - ) - ) - - Column( - verticalArrangement = Arrangement.Bottom, - horizontalAlignment = Alignment.Start, - modifier = - Modifier.animateContentSize() - .padding(top = it.calculateTopPadding()) - .fillMaxHeight() - .drawVerticalScrollbar( - scrollState, - color = MaterialTheme.colorScheme.onPrimary.copy(alpha = AlphaScrollbar) - ) - .verticalScroll(scrollState) - .testTag(SCROLLABLE_COLUMN_TEST_TAG) + MapColumn( + state, + it, + progressIndicatorBias, + scrollState, ) { Spacer(modifier = Modifier.defaultMinSize(minHeight = Dimens.mediumPadding).weight(1f)) MullvadCircularProgressIndicatorLarge( @@ -260,66 +219,19 @@ fun ConnectScreen( } ) Spacer(modifier = Modifier.defaultMinSize(minHeight = Dimens.mediumPadding).weight(1f)) - ConnectionStatusText( - state = state.tunnelRealState, - modifier = Modifier.padding(horizontal = Dimens.sideMargin) - ) - Text( - text = state.location?.country ?: "", - style = MaterialTheme.typography.headlineLarge, - color = MaterialTheme.colorScheme.onPrimary, - modifier = Modifier.padding(horizontal = Dimens.sideMargin) - ) - Text( - text = state.location?.city ?: "", - style = MaterialTheme.typography.headlineLarge, - color = MaterialTheme.colorScheme.onPrimary, - modifier = Modifier.padding(horizontal = Dimens.sideMargin) - ) - var expanded by rememberSaveable { mutableStateOf(false) } - LocationInfo( - onToggleTunnelInfo = { expanded = !expanded }, - isVisible = state.showLocationInfo, - isExpanded = expanded, - location = state.location, - inAddress = state.inAddress, - outAddress = state.outAddress, - modifier = - Modifier.fillMaxWidth() - .padding(horizontal = Dimens.sideMargin) - .testTag(LOCATION_INFO_TEST_TAG) - ) - Spacer(modifier = Modifier.height(Dimens.buttonSpacing)) - SwitchLocationButton( - modifier = - Modifier.fillMaxWidth() - .padding(horizontal = Dimens.sideMargin) - .testTag(SELECT_LOCATION_BUTTON_TEST_TAG), - onClick = onSwitchLocationClick, - showChevron = state.showLocation, - text = - if (state.showLocation && state.selectedRelayItem != null) { - state.selectedRelayItem.locationName - } else { - stringResource(id = R.string.switch_location) - } - ) + + ConnectionInfo(state = state) + Spacer(modifier = Modifier.height(Dimens.buttonSpacing)) - ConnectionButton( - state = state.tunnelUiState, - modifier = - Modifier.padding(horizontal = Dimens.sideMargin) - .padding(bottom = Dimens.screenVerticalMargin) - .testTag(CONNECT_BUTTON_TEST_TAG), - disconnectClick = onDisconnectClick, - reconnectClick = { handleThrottledAction(onReconnectClick) }, - cancelClick = onCancelClick, - connectClick = { handleThrottledAction(onConnectClick) }, - reconnectButtonTestTag = RECONNECT_BUTTON_TEST_TAG + + ButtonPanel( + state, + onSwitchLocationClick, + onDisconnectClick, + onReconnectClick, + onCancelClick, + onConnectClick, ) - // We need to manually add this padding so we align size with the map - // component and marker with the progress indicator. - Spacer(modifier = Modifier.height(it.calculateBottomPadding())) } NotificationBanner( @@ -333,6 +245,141 @@ fun ConnectScreen( } } +@Composable +private fun MapColumn( + state: ConnectUiState, + it: PaddingValues, + progressIndicatorBias: Float, + scrollState: ScrollState, + content: @Composable ColumnScope.() -> Unit +) { + + // Distance to marker when secure/unsecure + val baseZoom = + animateFloatAsState( + targetValue = + if (state.tunnelRealState is TunnelState.Connected) SECURE_ZOOM else UNSECURE_ZOOM, + animationSpec = tween(SECURE_ZOOM_ANIMATION_MILLIS), + label = "baseZoom" + ) + + val markers = state.tunnelRealState.toMarker(state.location)?.let { listOf(it) } ?: emptyList() + + AnimatedMap( + modifier = Modifier.padding(top = it.calculateTopPadding()), + cameraLocation = state.location?.toLatLong() ?: fallbackLatLong, + cameraBaseZoom = baseZoom.value, + cameraVerticalBias = progressIndicatorBias, + markers = markers, + globeColors = + GlobeColors( + landColor = MaterialTheme.colorScheme.primary, + oceanColor = MaterialTheme.colorScheme.secondary, + ) + ) + + Column( + verticalArrangement = Arrangement.Bottom, + horizontalAlignment = Alignment.Start, + modifier = + Modifier.animateContentSize() + .padding(top = it.calculateTopPadding()) + .fillMaxHeight() + .drawVerticalScrollbar( + scrollState, + color = MaterialTheme.colorScheme.onPrimary.copy(alpha = AlphaScrollbar) + ) + .verticalScroll(scrollState) + .testTag(SCROLLABLE_COLUMN_TEST_TAG) + ) { + content() + // We need to manually add this padding so we align size with the map + // component and marker with the progress indicator. + Spacer(modifier = Modifier.height(it.calculateBottomPadding())) + } +} + +@Composable +private fun ConnectionInfo(state: ConnectUiState) { + ConnectionStatusText( + state = state.tunnelRealState, + modifier = Modifier.padding(horizontal = Dimens.sideMargin) + ) + Text( + text = state.location?.country ?: "", + style = MaterialTheme.typography.headlineLarge, + color = MaterialTheme.colorScheme.onPrimary, + modifier = Modifier.padding(horizontal = Dimens.sideMargin) + ) + Text( + text = state.location?.city ?: "", + style = MaterialTheme.typography.headlineLarge, + color = MaterialTheme.colorScheme.onPrimary, + modifier = Modifier.padding(horizontal = Dimens.sideMargin) + ) + var expanded by rememberSaveable { mutableStateOf(false) } + LocationInfo( + onToggleTunnelInfo = { expanded = !expanded }, + isVisible = state.showLocationInfo, + isExpanded = expanded, + location = state.location, + inAddress = state.inAddress, + outAddress = state.outAddress, + modifier = + Modifier.fillMaxWidth() + .padding(horizontal = Dimens.sideMargin) + .testTag(LOCATION_INFO_TEST_TAG) + ) +} + +@Composable +private fun ButtonPanel( + state: ConnectUiState, + onSwitchLocationClick: () -> Unit, + onDisconnectClick: () -> Unit, + onReconnectClick: () -> Unit, + onCancelClick: () -> Unit, + onConnectClick: () -> Unit, +) { + var lastConnectionActionTimestamp by remember { mutableLongStateOf(0L) } + + fun handleThrottledAction(action: () -> Unit) { + val currentTime = System.currentTimeMillis() + if ((currentTime - lastConnectionActionTimestamp) > CONNECT_BUTTON_THROTTLE_MILLIS) { + lastConnectionActionTimestamp = currentTime + action.invoke() + } + } + + SwitchLocationButton( + modifier = + Modifier.fillMaxWidth() + .padding(horizontal = Dimens.sideMargin) + .testTag(SELECT_LOCATION_BUTTON_TEST_TAG), + onClick = onSwitchLocationClick, + showChevron = state.showLocation, + text = + if (state.showLocation && state.selectedRelayItem != null) { + state.selectedRelayItem.locationName + } else { + stringResource(id = R.string.switch_location) + } + ) + Spacer(modifier = Modifier.height(Dimens.buttonSpacing)) + ConnectionButton( + state = state.tunnelUiState, + modifier = + Modifier.padding(horizontal = Dimens.sideMargin) + .padding(bottom = Dimens.screenVerticalMargin) + .testTag(CONNECT_BUTTON_TEST_TAG), + disconnectClick = onDisconnectClick, + reconnectClick = { handleThrottledAction(onReconnectClick) }, + cancelClick = onCancelClick, + connectClick = { handleThrottledAction(onConnectClick) }, + reconnectButtonTestTag = RECONNECT_BUTTON_TEST_TAG + ) +} + @Composable fun TunnelState.toMarker(location: GeoIpLocation?): Marker? { if (location == null) return null 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 9a7eda2db207..b67807ad282e 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 @@ -89,7 +89,6 @@ fun FilterScreen(navigator: DestinationsNavigator) { ) } -@Suppress("LongMethod") @Composable fun FilterScreen( state: RelayFilterState, @@ -149,63 +148,105 @@ fun FilterScreen( LazyColumn(modifier = Modifier.padding(contentPadding).fillMaxSize()) { item { Divider() - ExpandableComposeCell( - title = stringResource(R.string.ownership), - isExpanded = ownershipExpanded, - isEnabled = true, - onInfoClicked = null, - onCellClicked = { ownershipExpanded = !ownershipExpanded } - ) + OwnershipHeader(ownershipExpanded, { ownershipExpanded = it }) } if (ownershipExpanded) { - item { - SelectableCell( - title = stringResource(id = R.string.any), - isSelected = state.selectedOwnership == null, - onCellClicked = { onSelectedOwnership(null) } - ) - } + item { AnyOwnership(state, onSelectedOwnership) } items(state.filteredOwnershipByProviders) { ownership -> Divider() - SelectableCell( - title = stringResource(id = ownership.stringResource()), - isSelected = ownership == state.selectedOwnership, - onCellClicked = { onSelectedOwnership(ownership) } - ) + Ownership(ownership, state, onSelectedOwnership) } } item { Divider() - ExpandableComposeCell( - title = stringResource(R.string.providers), - isExpanded = providerExpanded, - isEnabled = true, - onInfoClicked = null, - onCellClicked = { providerExpanded = !providerExpanded } - ) + ProvidersHeader(providerExpanded, { providerExpanded = it }) } if (providerExpanded) { item { Divider() - CheckboxCell( - providerName = stringResource(R.string.all_providers), - checked = state.isAllProvidersChecked, - onCheckedChange = { isChecked -> onAllProviderCheckChange(isChecked) } - ) + AllProviders(state, onAllProviderCheckChange) } items(state.filteredProvidersByOwnership) { provider -> Divider() - CheckboxCell( - providerName = provider.name, - checked = provider in state.selectedProviders, - onCheckedChange = { checked -> onSelectedProvider(checked, provider) } - ) + Provider(provider, state, onSelectedProvider) } } } } } +@Composable +private fun OwnershipHeader(expanded: Boolean, onToggleExpanded: (Boolean) -> Unit) { + ExpandableComposeCell( + title = stringResource(R.string.ownership), + isExpanded = expanded, + isEnabled = true, + onInfoClicked = null, + onCellClicked = { onToggleExpanded(!expanded) } + ) +} + +@Composable +private fun AnyOwnership( + state: RelayFilterState, + onSelectedOwnership: (ownership: Ownership?) -> Unit +) { + SelectableCell( + title = stringResource(id = R.string.any), + isSelected = state.selectedOwnership == null, + onCellClicked = { onSelectedOwnership(null) } + ) +} + +@Composable +private fun Ownership( + ownership: Ownership, + state: RelayFilterState, + onSelectedOwnership: (ownership: Ownership?) -> Unit +) { + SelectableCell( + title = stringResource(id = ownership.stringResource()), + isSelected = ownership == state.selectedOwnership, + onCellClicked = { onSelectedOwnership(ownership) } + ) +} + +@Composable +private fun ProvidersHeader(expanded: Boolean, onToggleExpanded: (Boolean) -> Unit) { + ExpandableComposeCell( + title = stringResource(R.string.providers), + isExpanded = expanded, + isEnabled = true, + onInfoClicked = null, + onCellClicked = { onToggleExpanded(!expanded) } + ) +} + +@Composable +private fun AllProviders( + state: RelayFilterState, + onAllProviderCheckChange: (isChecked: Boolean) -> Unit +) { + CheckboxCell( + providerName = stringResource(R.string.all_providers), + checked = state.isAllProvidersChecked, + onCheckedChange = { isChecked -> onAllProviderCheckChange(isChecked) } + ) +} + +@Composable +private fun Provider( + provider: Provider, + state: RelayFilterState, + onSelectedProvider: (checked: Boolean, provider: Provider) -> Unit +) { + CheckboxCell( + providerName = provider.name, + checked = provider in state.selectedProviders, + onCheckedChange = { checked -> onSelectedProvider(checked, provider) } + ) +} + private fun Ownership.stringResource(): Int = when (this) { Ownership.MullvadOwned -> R.string.mullvad_owned_only diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/LoginScreen.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/LoginScreen.kt index f21e185ce9a7..7d84be162b2e 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/LoginScreen.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/LoginScreen.kt @@ -6,6 +6,7 @@ import androidx.compose.foundation.background import androidx.compose.foundation.clickable import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.ColumnScope import androidx.compose.foundation.layout.IntrinsicSize import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.Spacer @@ -205,9 +206,7 @@ private fun LoginScreen( } } -@Suppress("LongMethod") @Composable -@OptIn(ExperimentalComposeUiApi::class) private fun LoginContent( state: LoginUiState, onAccountNumberChange: (String) -> Unit, @@ -225,88 +224,99 @@ private fun LoginContent( .padding(bottom = Dimens.smallPadding) ) - var tfFocusState: FocusState? by remember { mutableStateOf(null) } - var ddFocusState: FocusState? by remember { mutableStateOf(null) } - val expandedDropdown = tfFocusState?.hasFocus ?: false || ddFocusState?.hasFocus ?: false + LoginInput(state, onLoginClick, onAccountNumberChange, onDeleteHistoryClick) - Text( - modifier = Modifier.padding(bottom = Dimens.smallPadding), - text = state.loginState.supportingText() ?: "", - style = MaterialTheme.typography.labelMedium, - color = - if (state.loginState.isError()) { - MaterialTheme.colorScheme.error - } else { - MaterialTheme.colorScheme.onPrimary - }, + Spacer(modifier = Modifier.size(Dimens.largePadding)) + VariantButton( + isEnabled = state.loginButtonEnabled, + onClick = { onLoginClick(state.accountNumberInput) }, + text = stringResource(id = R.string.login_title), + modifier = Modifier.padding(bottom = Dimens.mediumPadding) ) + } +} - TextField( - modifier = - // Fix for DPad navigation - Modifier.onFocusChanged { tfFocusState = it } - .focusProperties { - left = FocusRequester.Cancel - right = FocusRequester.Cancel - } - .fillMaxWidth() - .testTag(LOGIN_INPUT_TEST_TAG) - .let { - if (!expandedDropdown || state.lastUsedAccount == null) { - it.clip(MaterialTheme.shapes.small) - } else { - it - } - }, - value = state.accountNumberInput, - label = { - Text( - text = stringResource(id = R.string.login_description), - color = Color.Unspecified, - maxLines = 1, - overflow = TextOverflow.Ellipsis - ) - }, - keyboardActions = KeyboardActions(onDone = { onLoginClick(state.accountNumberInput) }), - keyboardOptions = - KeyboardOptions( - imeAction = if (state.loginButtonEnabled) ImeAction.Done else ImeAction.None, - keyboardType = KeyboardType.NumberPassword - ), - onValueChange = onAccountNumberChange, - singleLine = true, - maxLines = 1, - visualTransformation = accountTokenVisualTransformation(), - enabled = state.loginState is Idle, - colors = mullvadWhiteTextFieldColors(), - isError = state.loginState.isError(), - ) +@Composable +@OptIn(ExperimentalComposeUiApi::class) +private fun ColumnScope.LoginInput( + state: LoginUiState, + onLoginClick: (String) -> Unit, + onAccountNumberChange: (String) -> Unit, + onDeleteHistoryClick: () -> Unit +) { + var tfFocusState: FocusState? by remember { mutableStateOf(null) } + var ddFocusState: FocusState? by remember { mutableStateOf(null) } + val expandedDropdown = tfFocusState?.hasFocus ?: false || ddFocusState?.hasFocus ?: false - AnimatedVisibility(visible = state.lastUsedAccount != null && expandedDropdown) { - val token = state.lastUsedAccount?.value.orEmpty() - val accountTransformation = remember { accountTokenVisualTransformation() } - val transformedText = - remember(token) { accountTransformation.filter(AnnotatedString(token)).text } + Text( + modifier = Modifier.padding(bottom = Dimens.smallPadding), + text = state.loginState.supportingText() ?: "", + style = MaterialTheme.typography.labelMedium, + color = + if (state.loginState.isError()) { + MaterialTheme.colorScheme.error + } else { + MaterialTheme.colorScheme.onPrimary + }, + ) - AccountDropDownItem( - modifier = Modifier.onFocusChanged { ddFocusState = it }, - accountToken = transformedText.toString(), - onClick = { - state.lastUsedAccount?.let { - onAccountNumberChange(it.value) - onLoginClick(it.value) + TextField( + modifier = + // Fix for DPad navigation + Modifier.onFocusChanged { tfFocusState = it } + .focusProperties { + left = FocusRequester.Cancel + right = FocusRequester.Cancel + } + .fillMaxWidth() + .testTag(LOGIN_INPUT_TEST_TAG) + .let { + if (!expandedDropdown || state.lastUsedAccount == null) { + it.clip(MaterialTheme.shapes.small) + } else { + it } }, - onDeleteClick = onDeleteHistoryClick + value = state.accountNumberInput, + label = { + Text( + text = stringResource(id = R.string.login_description), + color = Color.Unspecified, + maxLines = 1, + overflow = TextOverflow.Ellipsis ) - } + }, + keyboardActions = KeyboardActions(onDone = { onLoginClick(state.accountNumberInput) }), + keyboardOptions = + KeyboardOptions( + imeAction = if (state.loginButtonEnabled) ImeAction.Done else ImeAction.None, + keyboardType = KeyboardType.NumberPassword + ), + onValueChange = onAccountNumberChange, + singleLine = true, + maxLines = 1, + visualTransformation = accountTokenVisualTransformation(), + enabled = state.loginState is Idle, + colors = mullvadWhiteTextFieldColors(), + isError = state.loginState.isError(), + ) - Spacer(modifier = Modifier.size(Dimens.largePadding)) - VariantButton( - isEnabled = state.loginButtonEnabled, - onClick = { onLoginClick(state.accountNumberInput) }, - text = stringResource(id = R.string.login_title), - modifier = Modifier.padding(bottom = Dimens.mediumPadding) + AnimatedVisibility(visible = state.lastUsedAccount != null && expandedDropdown) { + val token = state.lastUsedAccount?.value.orEmpty() + val accountTransformation = remember { accountTokenVisualTransformation() } + val transformedText = + remember(token) { accountTransformation.filter(AnnotatedString(token)).text } + + AccountDropDownItem( + modifier = Modifier.onFocusChanged { ddFocusState = it }, + accountToken = transformedText.toString(), + onClick = { + state.lastUsedAccount?.let { + onAccountNumberChange(it.value) + onLoginClick(it.value) + } + }, + onDeleteClick = onDeleteHistoryClick ) } } diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/OutOfTimeScreen.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/OutOfTimeScreen.kt index 69949f49c9ef..7aa1f9024261 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/OutOfTimeScreen.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/OutOfTimeScreen.kt @@ -168,7 +168,6 @@ fun OutOfTime( ) } -@Suppress("LongMethod") @Composable fun OutOfTimeScreen( state: OutOfTimeUiState, @@ -248,57 +247,80 @@ fun OutOfTimeScreen( ) Spacer(modifier = Modifier.weight(1f).defaultMinSize(minHeight = Dimens.verticalSpace)) // Button area - if (state.tunnelState.showDisconnectButton()) { - NegativeButton( - onClick = onDisconnectClick, - text = stringResource(id = R.string.disconnect), - modifier = - Modifier.padding( - start = Dimens.sideMargin, - end = Dimens.sideMargin, - bottom = Dimens.buttonSpacing - ) - ) - } - state.billingPaymentState?.let { - PlayPayment( - billingPaymentState = state.billingPaymentState, - onPurchaseBillingProductClick = { productId -> - onPurchaseBillingProductClick(productId) - }, - onInfoClick = navigateToVerificationPendingDialog, - modifier = - Modifier.padding( - start = Dimens.sideMargin, - end = Dimens.sideMargin, - bottom = Dimens.buttonSpacing - ) - .align(Alignment.CenterHorizontally) - ) - } - if (state.showSitePayment) { - SitePaymentButton( - onClick = onSitePaymentClick, - isEnabled = state.tunnelState.enableSitePaymentButton(), - modifier = - Modifier.padding( + + ButtonPanel( + state = state, + onDisconnectClick = onDisconnectClick, + onPurchaseBillingProductClick = onPurchaseBillingProductClick, + onRedeemVoucherClick = onRedeemVoucherClick, + onSitePaymentClick = onSitePaymentClick, + navigateToVerificationPendingDialog = navigateToVerificationPendingDialog + ) + } + } +} + +@Composable +private fun ButtonPanel( + state: OutOfTimeUiState, + onDisconnectClick: () -> Unit, + onPurchaseBillingProductClick: (ProductId) -> Unit, + onRedeemVoucherClick: () -> Unit, + onSitePaymentClick: () -> Unit, + navigateToVerificationPendingDialog: () -> Unit +) { + + Column { + if (state.tunnelState.showDisconnectButton()) { + NegativeButton( + onClick = onDisconnectClick, + text = stringResource(id = R.string.disconnect), + modifier = + Modifier.padding( + start = Dimens.sideMargin, + end = Dimens.sideMargin, + bottom = Dimens.buttonSpacing + ) + ) + } + state.billingPaymentState?.let { + PlayPayment( + billingPaymentState = state.billingPaymentState, + onPurchaseBillingProductClick = { productId -> + onPurchaseBillingProductClick(productId) + }, + onInfoClick = navigateToVerificationPendingDialog, + modifier = + Modifier.padding( start = Dimens.sideMargin, end = Dimens.sideMargin, bottom = Dimens.buttonSpacing ) - ) - } - RedeemVoucherButton( - onClick = onRedeemVoucherClick, + .align(Alignment.CenterHorizontally) + ) + } + if (state.showSitePayment) { + SitePaymentButton( + onClick = onSitePaymentClick, + isEnabled = state.tunnelState.enableSitePaymentButton(), modifier = Modifier.padding( start = Dimens.sideMargin, end = Dimens.sideMargin, - bottom = Dimens.screenVerticalMargin - ), - isEnabled = state.tunnelState.enableRedeemButton() + bottom = Dimens.buttonSpacing + ) ) } + RedeemVoucherButton( + onClick = onRedeemVoucherClick, + modifier = + Modifier.padding( + start = Dimens.sideMargin, + end = Dimens.sideMargin, + bottom = Dimens.screenVerticalMargin + ), + isEnabled = state.tunnelState.enableRedeemButton() + ) } } 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 934cc646965b..a7a7f3bce690 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 @@ -2,10 +2,12 @@ package net.mullvad.mullvadvpn.compose.screen import androidx.compose.foundation.Image import androidx.compose.foundation.background +import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.Spacer import androidx.compose.foundation.layout.fillMaxSize +import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.height import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.width @@ -31,8 +33,6 @@ import androidx.compose.ui.text.style.TextDecoration import androidx.compose.ui.tooling.preview.Preview import androidx.compose.ui.unit.dp import androidx.compose.ui.unit.sp -import androidx.constraintlayout.compose.ConstraintLayout -import androidx.constraintlayout.compose.Dimension import androidx.lifecycle.compose.collectAsStateWithLifecycle import com.ramcosta.composedestinations.annotation.Destination import com.ramcosta.composedestinations.navigation.DestinationsNavigator @@ -109,7 +109,6 @@ fun PrivacyDisclaimer( PrivacyDisclaimerScreen(state, {}, viewModel::setPrivacyDisclosureAccepted) } -@Suppress("LongMethod") @Composable fun PrivacyDisclaimerScreen( state: PrivacyDisclaimerViewState, @@ -118,101 +117,87 @@ fun PrivacyDisclaimerScreen( ) { val topColor = MaterialTheme.colorScheme.primary ScaffoldWithTopBar(topBarColor = topColor, onAccountClicked = null, onSettingsClicked = null) { - ConstraintLayout( - modifier = - Modifier.padding(it) - .fillMaxSize() - .background(color = MaterialTheme.colorScheme.background) + val scrollState = rememberScrollState() + Column( + Modifier.padding(it) + .padding(horizontal = Dimens.sideMargin, vertical = Dimens.screenVerticalMargin) + .fillMaxSize() + .background(color = MaterialTheme.colorScheme.background) + .verticalScroll(scrollState) + .drawVerticalScrollbar( + state = scrollState, + color = MaterialTheme.colorScheme.onPrimary.copy(alpha = AlphaScrollbar) + ), + verticalArrangement = Arrangement.SpaceBetween ) { - val (body, actionButtons) = createRefs() - val sideMargin = Dimens.sideMargin - val scrollState = rememberScrollState() + Content(onPrivacyPolicyLinkClicked) - Column( - modifier = - Modifier.constrainAs(body) { - top.linkTo(parent.top) - start.linkTo(parent.start) - end.linkTo(parent.end) - bottom.linkTo(actionButtons.top) - width = Dimension.fillToConstraints - height = Dimension.fillToConstraints - } - .drawVerticalScrollbar( - state = scrollState, - color = MaterialTheme.colorScheme.onPrimary.copy(alpha = AlphaScrollbar) - ) - .verticalScroll(scrollState) - .padding(sideMargin), - ) { - Text( - text = stringResource(id = R.string.privacy_disclaimer_title), - style = MaterialTheme.typography.headlineSmall, - color = MaterialTheme.colorScheme.onBackground, - fontWeight = FontWeight.Bold - ) + ButtonPanel(state.isStartingService, onAcceptClicked) + } + } +} - val fontSize = 14.sp - Text( - text = stringResource(id = R.string.privacy_disclaimer_body_first_paragraph), - fontSize = fontSize, - color = MaterialTheme.colorScheme.onBackground, - modifier = Modifier.padding(top = 10.dp) - ) +@Composable +private fun Content(onPrivacyPolicyLinkClicked: () -> Unit) { + Column { + Text( + text = stringResource(id = R.string.privacy_disclaimer_title), + style = MaterialTheme.typography.headlineSmall, + color = MaterialTheme.colorScheme.onBackground, + fontWeight = FontWeight.Bold + ) - Spacer(modifier = Modifier.height(fontSize.toDp() + Dimens.smallPadding)) + val fontSize = 14.sp + Text( + text = stringResource(id = R.string.privacy_disclaimer_body_first_paragraph), + fontSize = fontSize, + color = MaterialTheme.colorScheme.onBackground, + modifier = Modifier.padding(top = 10.dp) + ) - Text( - text = stringResource(id = R.string.privacy_disclaimer_body_second_paragraph), - fontSize = fontSize, - color = MaterialTheme.colorScheme.onBackground, - ) + Spacer(modifier = Modifier.height(fontSize.toDp() + Dimens.smallPadding)) - Row(modifier = Modifier.padding(top = 10.dp)) { - ClickableText( - text = AnnotatedString(stringResource(id = R.string.privacy_policy_label)), - onClick = { onPrivacyPolicyLinkClicked() }, - style = - TextStyle( - fontSize = 12.sp, - color = Color.White, - textDecoration = TextDecoration.Underline - ) - ) + Text( + text = stringResource(id = R.string.privacy_disclaimer_body_second_paragraph), + fontSize = fontSize, + color = MaterialTheme.colorScheme.onBackground, + ) - Image( - painter = painterResource(id = R.drawable.icon_extlink), - contentDescription = null, - modifier = - Modifier.align(Alignment.CenterVertically) - .padding(start = 2.dp, top = 2.dp) - .width(10.dp) - .height(10.dp) + Row(modifier = Modifier.padding(top = 10.dp)) { + ClickableText( + text = AnnotatedString(stringResource(id = R.string.privacy_policy_label)), + onClick = { onPrivacyPolicyLinkClicked() }, + style = + TextStyle( + fontSize = 12.sp, + color = Color.White, + textDecoration = TextDecoration.Underline ) - } - } + ) - Column( + Image( + painter = painterResource(id = R.drawable.icon_extlink), + contentDescription = null, modifier = - Modifier.constrainAs(actionButtons) { - top.linkTo(body.bottom, margin = sideMargin) - start.linkTo(parent.start, margin = sideMargin) - end.linkTo(parent.end, margin = sideMargin) - bottom.linkTo(parent.bottom, margin = sideMargin) - width = Dimension.fillToConstraints - height = Dimension.preferredWrapContent - }, - horizontalAlignment = Alignment.CenterHorizontally - ) { - if (state.isStartingService) { - MullvadCircularProgressIndicatorMedium() - } else { - PrimaryButton( - text = stringResource(id = R.string.agree_and_continue), - onClick = onAcceptClicked::invoke - ) - } - } + Modifier.align(Alignment.CenterVertically) + .padding(start = 2.dp, top = 2.dp) + .width(10.dp) + .height(10.dp) + ) + } + } +} + +@Composable +private fun ButtonPanel(isStartingService: Boolean, onAcceptClicked: () -> Unit) { + Column(Modifier.fillMaxWidth(), horizontalAlignment = Alignment.CenterHorizontally) { + if (isStartingService) { + MullvadCircularProgressIndicatorMedium() + } else { + PrimaryButton( + text = stringResource(id = R.string.agree_and_continue), + onClick = onAcceptClicked::invoke + ) } } } diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/SettingsScreen.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/SettingsScreen.kt index f73119d71fc0..611f2e29ae9a 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/SettingsScreen.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/SettingsScreen.kt @@ -1,5 +1,6 @@ package net.mullvad.mullvadvpn.compose.screen +import android.content.Context import android.net.Uri import androidx.compose.animation.animateContentSize import androidx.compose.foundation.background @@ -78,7 +79,6 @@ fun Settings(navigator: DestinationsNavigator) { ) } -@Suppress("LongMethod") @ExperimentalMaterial3Api @Composable fun SettingsScreen( @@ -103,109 +103,121 @@ fun SettingsScreen( item { NavigationComposeCell( title = stringResource(id = R.string.settings_vpn), - onClick = { onVpnSettingCellClick() } + onClick = onVpnSettingCellClick ) } + item { Spacer(modifier = Modifier.height(Dimens.cellVerticalSpacing)) } + item { SplitTunneling(onSplitTunnelingCellClick) } + item { Spacer(modifier = Modifier.height(Dimens.cellVerticalSpacing)) } + } - item { - Spacer(modifier = Modifier.height(Dimens.cellVerticalSpacing)) - NavigationComposeCell( - title = stringResource(id = R.string.split_tunneling), - onClick = { onSplitTunnelingCellClick() } - ) - Spacer(modifier = Modifier.height(Dimens.cellVerticalSpacing)) - } + item { AppVersion(context, state) } + + item { Spacer(modifier = Modifier.height(Dimens.cellVerticalSpacing)) } + + itemWithDivider { ReportProblem(onReportProblemCellClick) } + + if (!state.isPlayBuild) { + itemWithDivider { FaqAndGuides(context) } } - item { - NavigationComposeCell( - title = stringResource(id = R.string.app_version), - onClick = { - context.openLink( - Uri.parse( - context.resources - .getString(R.string.download_url) - .appendHideNavOnPlayBuild(state.isPlayBuild) - ) - ) - }, - bodyView = - @Composable { - if (!state.isPlayBuild) { - NavigationCellBody( - content = state.appVersion, - contentBodyDescription = - stringResource(id = R.string.app_version), - isExternalLink = true, - ) - } else { - Text( - text = state.appVersion, - style = MaterialTheme.typography.labelMedium, - color = MaterialTheme.colorScheme.onSecondary - ) - } - }, - showWarning = state.isUpdateAvailable, - isRowEnabled = !state.isPlayBuild + + itemWithDivider { PrivacyPolicy(context, state) } + } + } +} + +@Composable +private fun SplitTunneling(onSplitTunnelingCellClick: () -> Unit) { + NavigationComposeCell( + title = stringResource(id = R.string.split_tunneling), + onClick = onSplitTunnelingCellClick + ) +} + +@Composable +private fun AppVersion(context: Context, state: SettingsUiState) { + NavigationComposeCell( + title = stringResource(id = R.string.app_version), + onClick = { + context.openLink( + Uri.parse( + context.resources + .getString(R.string.download_url) + .appendHideNavOnPlayBuild(state.isPlayBuild) ) - } - if (state.isUpdateAvailable) { - item { + ) + }, + bodyView = + @Composable { + if (!state.isPlayBuild) { + NavigationCellBody( + content = state.appVersion, + contentBodyDescription = stringResource(id = R.string.app_version), + isExternalLink = true, + ) + } else { Text( - text = stringResource(id = R.string.update_available_footer), + text = state.appVersion, style = MaterialTheme.typography.labelMedium, - color = MaterialTheme.colorScheme.onSecondary, - modifier = - Modifier.background(MaterialTheme.colorScheme.secondary) - .padding( - start = Dimens.cellStartPadding, - top = Dimens.cellTopPadding, - end = Dimens.cellStartPadding, - bottom = Dimens.cellLabelVerticalPadding, - ) + color = MaterialTheme.colorScheme.onSecondary ) } - } - - itemWithDivider { - Spacer(modifier = Modifier.height(Dimens.cellVerticalSpacing)) - NavigationComposeCell( - title = stringResource(id = R.string.report_a_problem), - onClick = { onReportProblemCellClick() } - ) - } + }, + showWarning = state.isUpdateAvailable, + isRowEnabled = !state.isPlayBuild + ) - if (!state.isPlayBuild) { - itemWithDivider { - val faqGuideLabel = stringResource(id = R.string.faqs_and_guides) - NavigationComposeCell( - title = faqGuideLabel, - bodyView = @Composable { DefaultExternalLinkView(faqGuideLabel) }, - onClick = { - context.openLink( - Uri.parse(context.resources.getString(R.string.faqs_and_guides_url)) - ) - } + if (state.isUpdateAvailable) { + Text( + text = stringResource(id = R.string.update_available_footer), + style = MaterialTheme.typography.labelMedium, + color = MaterialTheme.colorScheme.onSecondary, + modifier = + Modifier.background(MaterialTheme.colorScheme.secondary) + .padding( + start = Dimens.cellStartPadding, + top = Dimens.cellTopPadding, + end = Dimens.cellStartPadding, + bottom = Dimens.cellLabelVerticalPadding, ) - } - } + ) + } +} + +@Composable +private fun ReportProblem(onReportProblemCellClick: () -> Unit) { + NavigationComposeCell( + title = stringResource(id = R.string.report_a_problem), + onClick = { onReportProblemCellClick() } + ) +} + +@Composable +private fun FaqAndGuides(context: Context) { + val faqGuideLabel = stringResource(id = R.string.faqs_and_guides) + NavigationComposeCell( + title = faqGuideLabel, + bodyView = @Composable { DefaultExternalLinkView(faqGuideLabel) }, + onClick = { + context.openLink(Uri.parse(context.resources.getString(R.string.faqs_and_guides_url))) + } + ) +} - itemWithDivider { - val privacyPolicyLabel = stringResource(id = R.string.privacy_policy_label) - NavigationComposeCell( - title = privacyPolicyLabel, - bodyView = @Composable { DefaultExternalLinkView(privacyPolicyLabel) }, - onClick = { - context.openLink( - Uri.parse( - context.resources - .getString(R.string.privacy_policy_url) - .appendHideNavOnPlayBuild(state.isPlayBuild) - ) - ) - } +@Composable +private fun PrivacyPolicy(context: Context, state: SettingsUiState) { + val privacyPolicyLabel = stringResource(id = R.string.privacy_policy_label) + NavigationComposeCell( + title = privacyPolicyLabel, + bodyView = @Composable { DefaultExternalLinkView(privacyPolicyLabel) }, + onClick = { + context.openLink( + Uri.parse( + context.resources + .getString(R.string.privacy_policy_url) + .appendHideNavOnPlayBuild(state.isPlayBuild) ) - } + ) } - } + ) }