From c3dac5954e60e6220cfcfaded63c904b220b78b4 Mon Sep 17 00:00:00 2001 From: Percy Wegmann Date: Wed, 3 Apr 2024 16:41:23 -0500 Subject: [PATCH] android/ui: permissions styling feedback Updates tailscale/corp#18202 Signed-off-by: Percy Wegmann --- .../com/tailscale/ipn/ui/model/Permissions.kt | 89 ++++++++++++++----- .../com/tailscale/ipn/ui/view/MainView.kt | 27 +++--- .../tailscale/ipn/ui/view/PermissionsView.kt | 22 ++--- .../ipn/ui/viewModel/SettingsViewModel.kt | 5 +- android/src/main/res/values/strings.xml | 1 + 5 files changed, 90 insertions(+), 54 deletions(-) diff --git a/android/src/main/java/com/tailscale/ipn/ui/model/Permissions.kt b/android/src/main/java/com/tailscale/ipn/ui/model/Permissions.kt index 84901e4..8579f0b 100644 --- a/android/src/main/java/com/tailscale/ipn/ui/model/Permissions.kt +++ b/android/src/main/java/com/tailscale/ipn/ui/model/Permissions.kt @@ -5,9 +5,55 @@ package com.tailscale.ipn.ui.model import android.Manifest import android.os.Build +import androidx.compose.runtime.Composable +import androidx.compose.ui.platform.LocalContext +import androidx.core.app.NotificationManagerCompat +import com.google.accompanist.permissions.ExperimentalPermissionsApi +import com.google.accompanist.permissions.PermissionState +import com.google.accompanist.permissions.isGranted +import com.google.accompanist.permissions.rememberMultiplePermissionsState +import com.google.accompanist.permissions.shouldShowRationale import com.tailscale.ipn.R object Permissions { + /** Permissions to prompt for on MainView. */ + @OptIn(ExperimentalPermissionsApi::class) + val prompt: List> + @Composable + get() { + val permissionStates = rememberMultiplePermissionsState(permissions = all.map { it.name }) + return all.zip(permissionStates.permissions).filter { (permission, state) -> + !state.status.isGranted && !state.status.shouldShowRationale + } + } + + /** All permissions with granted status. */ + @OptIn(ExperimentalPermissionsApi::class) + val withGrantedStatus: List> + @Composable + get() { + val permissionStates = rememberMultiplePermissionsState(permissions = all.map { it.name }) + val result = mutableListOf>() + result.addAll( + all.zip(permissionStates.permissions).map { (permission, state) -> + Pair(permission, state.status.isGranted) + }) + if (Build.VERSION.SDK_INT < Build.VERSION_CODES.TIRAMISU) { + // On Android versions prior to 13, we have to programmatically check if notifications are + // being allowed. + val notificationsEnabled = + NotificationManagerCompat.from(LocalContext.current).areNotificationsEnabled() + result.add( + Pair( + Permission( + "", + R.string.permission_post_notifications, + R.string.permission_post_notifications_needed), + notificationsEnabled)) + } + return result + } + /** * All permissions that Tailscale requires. MainView takes care of prompting for permissions, and * PermissionsView provides a list of permissions with corresponding statuses and a link to the @@ -16,26 +62,29 @@ object Permissions { * When new permissions are needed, just add them to this list and the necessary strings to * strings.xml and the rest should take care of itself. */ - val all: List - get() { - val result = mutableListOf() - if (Build.VERSION.SDK_INT < Build.VERSION_CODES.R) { - result.add( - Permission( - Manifest.permission.WRITE_EXTERNAL_STORAGE, - R.string.permission_write_external_storage, - R.string.permission_write_external_storage_needed, - )) - } - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) { - result.add( - Permission( - Manifest.permission.POST_NOTIFICATIONS, - R.string.permission_post_notifications, - R.string.permission_post_notifications_needed)) - } - return result + private val all: List by lazy { + val result = mutableListOf() + if (Build.VERSION.SDK_INT < Build.VERSION_CODES.R) { + result.add( + Permission( + Manifest.permission.WRITE_EXTERNAL_STORAGE, + R.string.permission_write_external_storage, + R.string.permission_write_external_storage_needed, + )) + } + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) { + result.add( + Permission( + Manifest.permission.POST_NOTIFICATIONS, + R.string.permission_post_notifications, + R.string.permission_post_notifications_needed)) } + result + } } -data class Permission(val name: String, val title: Int, val description: Int) +data class Permission( + val name: String, + val title: Int, + val description: Int, +) diff --git a/android/src/main/java/com/tailscale/ipn/ui/view/MainView.kt b/android/src/main/java/com/tailscale/ipn/ui/view/MainView.kt index 1a55e34..e5c3e07 100644 --- a/android/src/main/java/com/tailscale/ipn/ui/view/MainView.kt +++ b/android/src/main/java/com/tailscale/ipn/ui/view/MainView.kt @@ -58,14 +58,10 @@ import androidx.compose.ui.text.style.TextOverflow import androidx.compose.ui.unit.dp import androidx.lifecycle.viewmodel.compose.viewModel import com.google.accompanist.permissions.ExperimentalPermissionsApi -import com.google.accompanist.permissions.isGranted -import com.google.accompanist.permissions.rememberPermissionState -import com.google.accompanist.permissions.shouldShowRationale import com.tailscale.ipn.R import com.tailscale.ipn.ui.model.Ipn import com.tailscale.ipn.ui.model.IpnLocal import com.tailscale.ipn.ui.model.Netmap -import com.tailscale.ipn.ui.model.Permission import com.tailscale.ipn.ui.model.Permissions import com.tailscale.ipn.ui.model.Tailcfg import com.tailscale.ipn.ui.theme.disabled @@ -141,9 +137,9 @@ fun MainView(navigation: MainViewNavigation, viewModel: MainViewModel = viewMode when (state) { Ipn.State.Running -> { - PromptPermissionsIfNecessary(permissions = Permissions.all) + PromptPermissionsIfNecessary() - ExpiryNotificationIfNeccessary( + ExpiryNotificationIfNecessary( netmap = netmap.value, action = { viewModel.login {} }) ExitNodeStatus(navAction = navigation.onNavigateToExitNodes, viewModel = viewModel) @@ -413,7 +409,7 @@ fun PeerList( } @Composable -fun ExpiryNotificationIfNeccessary(netmap: Netmap.NetworkMap?, action: () -> Unit = {}) { +fun ExpiryNotificationIfNecessary(netmap: Netmap.NetworkMap?, action: () -> Unit = {}) { // Key expiry warning shown only if the key is expiring within 24 hours (or has already expired) val networkMap = netmap ?: return if (!TimeUtil.isWithin24Hours(networkMap.SelfNode.KeyExpiry)) { @@ -446,14 +442,13 @@ fun ExpiryNotificationIfNeccessary(netmap: Netmap.NetworkMap?, action: () -> Uni @OptIn(ExperimentalPermissionsApi::class) @Composable -fun PromptPermissionsIfNecessary(permissions: List) { - permissions.forEach { permission -> - val state = rememberPermissionState(permission.name) - if (!state.status.isGranted && !state.status.shouldShowRationale) { - // We don't have the permission and can ask for it - ErrorDialog(title = permission.title, message = permission.description) { - state.launchPermissionRequest() - } - } +fun PromptPermissionsIfNecessary() { + Permissions.prompt.forEach { (permission, state) -> + ErrorDialog( + title = permission.title, + message = permission.description, + buttonText = R.string._continue) { + state.launchPermissionRequest() + } } } diff --git a/android/src/main/java/com/tailscale/ipn/ui/view/PermissionsView.kt b/android/src/main/java/com/tailscale/ipn/ui/view/PermissionsView.kt index fb593b4..37e8667 100644 --- a/android/src/main/java/com/tailscale/ipn/ui/view/PermissionsView.kt +++ b/android/src/main/java/com/tailscale/ipn/ui/view/PermissionsView.kt @@ -18,8 +18,6 @@ import androidx.compose.ui.res.painterResource import androidx.compose.ui.res.stringResource import androidx.compose.ui.unit.dp import com.google.accompanist.permissions.ExperimentalPermissionsApi -import com.google.accompanist.permissions.isGranted -import com.google.accompanist.permissions.rememberMultiplePermissionsState import com.tailscale.ipn.R import com.tailscale.ipn.ui.model.Permissions import com.tailscale.ipn.ui.theme.success @@ -28,31 +26,23 @@ import com.tailscale.ipn.ui.util.itemsWithDividers @OptIn(ExperimentalPermissionsApi::class) @Composable fun PermissionsView(nav: BackNavigation, openApplicationSettings: () -> Unit) { + val permissions = Permissions.withGrantedStatus Scaffold(topBar = { Header(titleRes = R.string.permissions, onBack = nav.onBack) }) { innerPadding -> - val permissions = Permissions.all - val permissionStates = - rememberMultiplePermissionsState(permissions = permissions.map { it.name }) - val permissionsWithStates = permissions.zip(permissionStates.permissions) - LazyColumn(modifier = Modifier.padding(innerPadding)) { - itemsWithDividers(permissionsWithStates) { (permission, state) -> - var modifier: Modifier = Modifier - if (!state.status.isGranted) { - modifier = modifier.clickable { openApplicationSettings() } - } + itemsWithDividers(permissions) { (permission, granted) -> ListItem( - modifier = modifier, + modifier = Modifier.clickable { openApplicationSettings() }, leadingContent = { Icon( - if (state.status.isGranted) painterResource(R.drawable.check_circle) + if (granted) painterResource(R.drawable.check_circle) else painterResource(R.drawable.xmark_circle), tint = - if (state.status.isGranted) MaterialTheme.colorScheme.success + if (granted) MaterialTheme.colorScheme.success else MaterialTheme.colorScheme.onSurfaceVariant, modifier = Modifier.size(24.dp), contentDescription = - stringResource(if (state.status.isGranted) R.string.ok else R.string.warning)) + stringResource(if (granted) R.string.ok else R.string.warning)) }, headlineContent = { Text(stringResource(permission.title), style = MaterialTheme.typography.titleMedium) diff --git a/android/src/main/java/com/tailscale/ipn/ui/viewModel/SettingsViewModel.kt b/android/src/main/java/com/tailscale/ipn/ui/viewModel/SettingsViewModel.kt index 7a309e4..d5a668d 100644 --- a/android/src/main/java/com/tailscale/ipn/ui/viewModel/SettingsViewModel.kt +++ b/android/src/main/java/com/tailscale/ipn/ui/viewModel/SettingsViewModel.kt @@ -26,8 +26,9 @@ enum class SettingType { // enabled: Whether the setting is enabled // value: The value of the setting for textual settings // isOn: The value of the setting for switch settings -// onClick: The action to take when the setting is clicked (typicall for navigation) +// onClick: The action to take when the setting is clicked (typically for navigation) // onToggle: The action to take when the setting is toggled (typically for switches) +// icon: An optional Composable that draws a trailing icon to display with nav settings // // Behavior is undefined if you mix the types here. Switch settings should supply an // isOn and onToggle, while navigation settings should supply an onClick and an optional @@ -40,7 +41,7 @@ data class Setting( val enabled: StateFlow = MutableStateFlow(true), val isOn: StateFlow? = null, val onClick: () -> Unit = {}, - val onToggle: (Boolean) -> Unit = {} + val onToggle: (Boolean) -> Unit = {}, ) data class SettingsNav( diff --git a/android/src/main/res/values/strings.xml b/android/src/main/res/values/strings.xml index efd86bd..4c50b5e 100644 --- a/android/src/main/res/values/strings.xml +++ b/android/src/main/res/values/strings.xml @@ -15,6 +15,7 @@ Selected Offline OK + Continue Warning Search\n