From 4a5d087d56dbd9a21a76993d74556889ce841cf2 Mon Sep 17 00:00:00 2001 From: Jonathan Nobels Date: Mon, 17 Jun 2024 15:44:54 -0400 Subject: [PATCH] android/ui: fix AndroidTV navigation issues (#424) updates tailscale/corp#20930 This addresses several issues with AndroidTV navigation: The search bar is removed until we have a better solution for D-pad navigation. This should be addressed when we switch to a less-customized component. Full replacement of the search functionality is beyond the scope of this change. The back button will now automatically request the focus on AndroidTV devices by default so there is always at least one element focussed. Views with clipboard support are disabled since this was not functional (nothing was getting copied to the clipboard). View with embedded links are removed since these require touch support and a browser. Signed-off-by: Jonathan Nobels --- .../ipn/ui/util/ClipboardValueView.kt | 10 +- .../java/com/tailscale/ipn/ui/util/Lists.kt | 7 +- .../com/tailscale/ipn/ui/view/MainView.kt | 115 +++++++++++------- .../com/tailscale/ipn/ui/view/PeerDetails.kt | 16 ++- .../com/tailscale/ipn/ui/view/SettingsView.kt | 3 +- .../com/tailscale/ipn/ui/view/SharedViews.kt | 24 +++- 6 files changed, 116 insertions(+), 59 deletions(-) diff --git a/android/src/main/java/com/tailscale/ipn/ui/util/ClipboardValueView.kt b/android/src/main/java/com/tailscale/ipn/ui/util/ClipboardValueView.kt index 6dae8f7..79a5c34 100644 --- a/android/src/main/java/com/tailscale/ipn/ui/util/ClipboardValueView.kt +++ b/android/src/main/java/com/tailscale/ipn/ui/util/ClipboardValueView.kt @@ -20,13 +20,21 @@ import androidx.compose.ui.text.AnnotatedString import androidx.compose.ui.unit.dp import com.tailscale.ipn.R import com.tailscale.ipn.ui.theme.titledListItem +import com.tailscale.ipn.ui.util.AndroidTVUtil.isAndroidTV @Composable fun ClipboardValueView(value: String, title: String? = null, subtitle: String? = null) { val localClipboardManager = LocalClipboardManager.current + val modifier = + if (isAndroidTV()) { + Modifier + } else { + Modifier.clickable { localClipboardManager.setText(AnnotatedString(value)) } + } + ListItem( colors = MaterialTheme.colorScheme.titledListItem, - modifier = Modifier.clickable { localClipboardManager.setText(AnnotatedString(value)) }, + modifier = modifier, overlineContent = title?.let { { Text(it, style = MaterialTheme.typography.titleMedium) } }, headlineContent = { Text(text = value, style = MaterialTheme.typography.bodyMedium) }, supportingContent = diff --git a/android/src/main/java/com/tailscale/ipn/ui/util/Lists.kt b/android/src/main/java/com/tailscale/ipn/ui/util/Lists.kt index bf92437..4da762d 100644 --- a/android/src/main/java/com/tailscale/ipn/ui/util/Lists.kt +++ b/android/src/main/java/com/tailscale/ipn/ui/util/Lists.kt @@ -4,6 +4,7 @@ package com.tailscale.ipn.ui.util import androidx.compose.foundation.background +import androidx.compose.foundation.focusable import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.padding @@ -41,7 +42,8 @@ object Lists { title: String, bottomPadding: Dp = 0.dp, style: TextStyle = MaterialTheme.typography.titleMedium, - fontWeight: FontWeight? = null + fontWeight: FontWeight? = null, + focusable: Boolean = false ) { Box( modifier = @@ -50,7 +52,8 @@ object Lists { Text( title, modifier = - Modifier.padding(start = 16.dp, end = 16.dp, top = 8.dp, bottom = bottomPadding), + Modifier.padding(start = 16.dp, end = 16.dp, top = 8.dp, bottom = bottomPadding) + .focusable(focusable), style = style, fontWeight = fontWeight) } 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 909b40b..09dcb2b 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 @@ -6,6 +6,7 @@ package com.tailscale.ipn.ui.view import androidx.compose.foundation.ExperimentalFoundationApi import androidx.compose.foundation.background import androidx.compose.foundation.clickable +import androidx.compose.foundation.focusable import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column @@ -86,6 +87,7 @@ import com.tailscale.ipn.ui.theme.short import com.tailscale.ipn.ui.theme.surfaceContainerListItem import com.tailscale.ipn.ui.theme.warningButton import com.tailscale.ipn.ui.theme.warningListItem +import com.tailscale.ipn.ui.util.AndroidTVUtil.isAndroidTV import com.tailscale.ipn.ui.util.AutoResizingText import com.tailscale.ipn.ui.util.Lists import com.tailscale.ipn.ui.util.LoadingIndicator @@ -316,7 +318,8 @@ fun ExitNodeStatus(navAction: () -> Unit, viewModel: MainViewModel) { NodeState.OFFLINE_MDM -> MaterialTheme.colorScheme.errorButton NodeState.RUNNING_AS_EXIT_NODE -> MaterialTheme.colorScheme.warningButton - NodeState.ACTIVE_NOT_RUNNING -> MaterialTheme.colorScheme.exitNodeToggleButton + NodeState.ACTIVE_NOT_RUNNING -> + MaterialTheme.colorScheme.exitNodeToggleButton else -> MaterialTheme.colorScheme.secondaryButton }, onClick = { @@ -493,49 +496,61 @@ fun PeerList( val focusManager = LocalFocusManager.current var isFocussed by remember { mutableStateOf(false) } - Box(modifier = Modifier.fillMaxWidth().background(color = MaterialTheme.colorScheme.surface)) { - OutlinedTextField( - modifier = - Modifier.fillMaxWidth() - .padding(start = 16.dp, end = 16.dp, top = 16.dp, bottom = 0.dp) - .onFocusChanged { isFocussed = it.isFocused }, - singleLine = true, - shape = MaterialTheme.shapes.extraLarge, - colors = MaterialTheme.colorScheme.searchBarColors, - leadingIcon = { Icon(imageVector = Icons.Outlined.Search, contentDescription = "search") }, - trailingIcon = { - if (isFocussed) { - IconButton( - onClick = { - focusManager.clearFocus() - onSearch("") - }) { - Icon( - imageVector = - if (searchTermStr.isEmpty()) Icons.Outlined.Close - else Icons.Outlined.Clear, - contentDescription = "clear search", - tint = MaterialTheme.colorScheme.onSurfaceVariant) - } - } - }, - placeholder = { - Text( - text = stringResource(id = R.string.search), - style = MaterialTheme.typography.bodyLarge, - maxLines = 1) - }, - value = searchTermStr, - onValueChange = { onSearch(it) }) + var isListFocussed by remember { mutableStateOf(false) } + + val enableSearch = !isAndroidTV() + + if (enableSearch) { + Box(modifier = Modifier.fillMaxWidth().background(color = MaterialTheme.colorScheme.surface)) { + OutlinedTextField( + modifier = + Modifier.fillMaxWidth() + .padding(start = 16.dp, end = 16.dp, top = 16.dp, bottom = 0.dp) + .onFocusChanged { isFocussed = it.isFocused }, + singleLine = true, + shape = MaterialTheme.shapes.extraLarge, + colors = MaterialTheme.colorScheme.searchBarColors, + leadingIcon = { + Icon(imageVector = Icons.Outlined.Search, contentDescription = "search") + }, + trailingIcon = { + if (isFocussed) { + IconButton( + onClick = { + focusManager.clearFocus() + onSearch("") + }) { + Icon( + imageVector = + if (searchTermStr.isEmpty()) Icons.Outlined.Close + else Icons.Outlined.Clear, + contentDescription = "clear search", + tint = MaterialTheme.colorScheme.onSurfaceVariant) + } + } + }, + placeholder = { + Text( + text = stringResource(id = R.string.search), + style = MaterialTheme.typography.bodyLarge, + maxLines = 1) + }, + value = searchTermStr, + onValueChange = { onSearch(it) }) + } } LazyColumn( - modifier = Modifier.fillMaxSize().background(color = MaterialTheme.colorScheme.surface)) { + modifier = + Modifier.fillMaxSize() + .onFocusChanged { isListFocussed = it.isFocused } + .background(color = MaterialTheme.colorScheme.surface)) { if (showNoResults) { item { Spacer( Modifier.height(16.dp) .fillMaxSize() + .focusable(false) .background(color = MaterialTheme.colorScheme.surface)) Lists.LargeTitle( @@ -553,17 +568,11 @@ fun PeerList( } first = false - stickyHeader { - Spacer( - Modifier.height(16.dp) - .fillMaxSize() - .background(color = MaterialTheme.colorScheme.surface)) - - Lists.LargeTitle( - peerSet.user?.DisplayName ?: stringResource(id = R.string.unknown_user), - bottomPadding = 8.dp, - style = MaterialTheme.typography.titleLarge, - fontWeight = FontWeight.SemiBold) + // Sticky headers are a bit broken on Android TV - they hide their content + if (isAndroidTV()) { + item { NodesSectionHeader(peerSet = peerSet) } + } else { + stickyHeader { NodesSectionHeader(peerSet = peerSet) } } itemsWithDividers(peerSet.peers, key = { it.StableID }) { peer -> @@ -595,6 +604,18 @@ fun PeerList( } } +@Composable +fun NodesSectionHeader(peerSet: PeerSet) { + Spacer(Modifier.height(16.dp).fillMaxSize().background(color = MaterialTheme.colorScheme.surface)) + + Lists.LargeTitle( + peerSet.user?.DisplayName ?: stringResource(id = R.string.unknown_user), + bottomPadding = 8.dp, + focusable = isAndroidTV(), + style = MaterialTheme.typography.titleLarge, + fontWeight = FontWeight.SemiBold) +} + @Composable fun ExpiryNotification(netmap: Netmap.NetworkMap?, action: () -> Unit = {}) { if (netmap == null) return diff --git a/android/src/main/java/com/tailscale/ipn/ui/view/PeerDetails.kt b/android/src/main/java/com/tailscale/ipn/ui/view/PeerDetails.kt index b65bb0f..052704c 100644 --- a/android/src/main/java/com/tailscale/ipn/ui/view/PeerDetails.kt +++ b/android/src/main/java/com/tailscale/ipn/ui/view/PeerDetails.kt @@ -5,6 +5,7 @@ package com.tailscale.ipn.ui.view import androidx.compose.foundation.background import androidx.compose.foundation.clickable +import androidx.compose.foundation.focusable import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.Row @@ -33,6 +34,7 @@ import androidx.lifecycle.viewmodel.compose.viewModel import com.tailscale.ipn.R import com.tailscale.ipn.ui.theme.listItem import com.tailscale.ipn.ui.theme.short +import com.tailscale.ipn.ui.util.AndroidTVUtil.isAndroidTV import com.tailscale.ipn.ui.util.Lists import com.tailscale.ipn.ui.util.itemsWithDividers import com.tailscale.ipn.ui.viewModel.PeerDetailsViewModel @@ -101,14 +103,24 @@ fun PeerDetails( fun AddressRow(address: String, type: String) { val localClipboardManager = LocalClipboardManager.current + // Android TV doesn't have a clipboard, nor any way to use the values, so visible only. + val modifier = + if (isAndroidTV()) { + Modifier.focusable(false) + } else { + Modifier.clickable { localClipboardManager.setText(AnnotatedString(address)) } + } + ListItem( - modifier = Modifier.clickable { localClipboardManager.setText(AnnotatedString(address)) }, + modifier = modifier, colors = MaterialTheme.colorScheme.listItem, headlineContent = { Text(text = address) }, supportingContent = { Text(text = type) }, trailingContent = { // TODO: there is some overlap with other uses of clipboard, DRY - Icon(painter = painterResource(id = R.drawable.clipboard), null) + if (!isAndroidTV()) { + Icon(painter = painterResource(id = R.drawable.clipboard), null) + } }) } diff --git a/android/src/main/java/com/tailscale/ipn/ui/view/SettingsView.kt b/android/src/main/java/com/tailscale/ipn/ui/view/SettingsView.kt index b920a38..70b660d 100644 --- a/android/src/main/java/com/tailscale/ipn/ui/view/SettingsView.kt +++ b/android/src/main/java/com/tailscale/ipn/ui/view/SettingsView.kt @@ -32,6 +32,7 @@ import com.tailscale.ipn.mdm.ShowHide import com.tailscale.ipn.ui.Links import com.tailscale.ipn.ui.theme.link import com.tailscale.ipn.ui.theme.listItem +import com.tailscale.ipn.ui.util.AndroidTVUtil.isAndroidTV import com.tailscale.ipn.ui.util.Lists import com.tailscale.ipn.ui.util.set import com.tailscale.ipn.ui.viewModel.SettingsNav @@ -61,7 +62,7 @@ fun SettingsView(settingsNav: SettingsNav, viewModel: SettingsViewModel = viewMo onClick = settingsNav.onNavigateToUserSwitcher) } - if (isAdmin) { + if (isAdmin && !isAndroidTV()) { Lists.ItemDivider() AdminTextView { handler.openUri(Links.ADMIN_URL) } } diff --git a/android/src/main/java/com/tailscale/ipn/ui/view/SharedViews.kt b/android/src/main/java/com/tailscale/ipn/ui/view/SharedViews.kt index 0df763b..de31170 100644 --- a/android/src/main/java/com/tailscale/ipn/ui/view/SharedViews.kt +++ b/android/src/main/java/com/tailscale/ipn/ui/view/SharedViews.kt @@ -22,12 +22,16 @@ import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Text import androidx.compose.material3.TopAppBar import androidx.compose.runtime.Composable +import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.remember import androidx.compose.ui.Modifier +import androidx.compose.ui.focus.FocusRequester +import androidx.compose.ui.focus.focusRequester import androidx.compose.ui.res.stringResource import androidx.compose.ui.unit.dp import com.tailscale.ipn.ui.theme.topAppBar import com.tailscale.ipn.ui.theme.ts_color_light_blue +import com.tailscale.ipn.ui.util.AndroidTVUtil.isAndroidTV typealias BackNavigation = () -> Unit @@ -41,6 +45,12 @@ fun Header( actions: @Composable RowScope.() -> Unit = {}, onBack: (() -> Unit)? = null ) { + val f = FocusRequester() + + if (isAndroidTV()) { + LaunchedEffect(Unit) { f.requestFocus() } + } + TopAppBar( title = { title?.let { title() } @@ -51,21 +61,23 @@ fun Header( }, colors = MaterialTheme.colorScheme.topAppBar, actions = actions, - navigationIcon = { onBack?.let { BackArrow(action = it) } }, + navigationIcon = { onBack?.let { BackArrow(action = it, focusRequester = f) } }, ) } @Composable -fun BackArrow(action: () -> Unit) { +fun BackArrow(action: () -> Unit, focusRequester: FocusRequester) { + Box(modifier = Modifier.padding(start = 8.dp, end = 8.dp)) { Icon( Icons.AutoMirrored.Filled.ArrowBack, contentDescription = "Go back to the previous screen", modifier = - Modifier.clickable( - interactionSource = remember { MutableInteractionSource() }, - indication = rememberRipple(bounded = false), - onClick = { action() })) + Modifier.focusRequester(focusRequester) + .clickable( + interactionSource = remember { MutableInteractionSource() }, + indication = rememberRipple(bounded = false), + onClick = { action() })) } }