Skip to content

Commit

Permalink
android/ui: fix AndroidTV navigation issues (#424)
Browse files Browse the repository at this point in the history
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 <jonathan@tailscale.com>
  • Loading branch information
barnstar committed Jun 17, 2024
1 parent 634d51c commit 0ff6be6
Show file tree
Hide file tree
Showing 6 changed files with 116 additions and 59 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down
7 changes: 5 additions & 2 deletions android/src/main/java/com/tailscale/ipn/ui/util/Lists.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 =
Expand All @@ -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)
}
Expand Down
115 changes: 68 additions & 47 deletions android/src/main/java/com/tailscale/ipn/ui/view/MainView.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 = {
Expand Down Expand Up @@ -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(
Expand All @@ -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 ->
Expand Down Expand Up @@ -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
Expand Down
16 changes: 14 additions & 2 deletions android/src/main/java/com/tailscale/ipn/ui/view/PeerDetails.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
})
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) }
}
Expand Down
24 changes: 18 additions & 6 deletions android/src/main/java/com/tailscale/ipn/ui/view/SharedViews.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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() }
Expand All @@ -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() }))
}
}

Expand Down

0 comments on commit 0ff6be6

Please sign in to comment.