android: fix android TV focus requester crash (#580)

Properly attach FocusRequester to the root column of TaildropView so that there is a focusable UI element available to receive the focus Fixes tailscale/corp#25007 Signed-off-by: kari-ts <[email protected]>
tailscale · Dec 5, 2024 · e29cfc5 · e29cfc5
1 parent 38abb03
commit e29cfc5
Show file tree

Hide file tree

Showing 2 changed files with 65 additions and 35 deletions.
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
@@ -5,6 +5,7 @@ package com.tailscale.ipn.ui.view
 
 import androidx.annotation.StringRes
 import androidx.compose.foundation.clickable
+import androidx.compose.foundation.focusable
 import androidx.compose.foundation.interaction.MutableInteractionSource
 import androidx.compose.foundation.layout.Box
 import androidx.compose.foundation.layout.RowScope
@@ -32,9 +33,12 @@ 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
+import com.tailscale.ipn.util.TSLog
 
 typealias BackNavigation = () -> Unit
 
+val TAG = "SharedViews"
+
 // Header view for all secondary screens
 // @see TopAppBar actions for additional actions (usually a row of icons)
 @OptIn(ExperimentalMaterial3Api::class)
@@ -45,10 +49,16 @@ fun Header(
     actions: @Composable RowScope.() -> Unit = {},
     onBack: (() -> Unit)? = null
 ) {
-  val f = FocusRequester()
+  val focusRequester = remember { FocusRequester() }
 
   if (isAndroidTV()) {
-    LaunchedEffect(Unit) { f.requestFocus() }
+    LaunchedEffect(focusRequester) {
+      try {
+        focusRequester.requestFocus()
+      } catch (e: Exception) {
+        TSLog.d(TAG, "Focus request failed")
+      }
+    }
   }
 
   TopAppBar(
@@ -61,23 +71,29 @@ fun Header(
       },
       colors = MaterialTheme.colorScheme.topAppBar,
       actions = actions,
-      navigationIcon = { onBack?.let { BackArrow(action = it, focusRequester = f) } },
+      navigationIcon = { onBack?.let { BackArrow(action = it, focusRequester = focusRequester) } },
   )
 }
 
 @Composable
 fun BackArrow(action: () -> Unit, focusRequester: FocusRequester) {
+  val modifier =
+      if (isAndroidTV()) {
+        Modifier.focusRequester(focusRequester)
+            .focusable() // Ensure the composable can receive focus
+      } else {
+        Modifier
+      }
 
-  Box(modifier = Modifier.padding(start = 8.dp, end = 8.dp)) {
+  Box(modifier = modifier.padding(start = 8.dp, end = 8.dp)) {
     Icon(
         Icons.AutoMirrored.Filled.ArrowBack,
         contentDescription = "Go back to the previous screen",
         modifier =
-            Modifier.focusRequester(focusRequester)
-                .clickable(
-                    interactionSource = remember { MutableInteractionSource() },
-                    indication = ripple(bounded = false),
-                    onClick = { action() }))
+            Modifier.clickable(
+                interactionSource = remember { MutableInteractionSource() },
+                indication = ripple(bounded = true),
+                onClick = action))
   }
 }
 
@@ -96,7 +112,7 @@ fun SimpleActivityIndicator(size: Int = 32) {
 @Composable
 fun ActivityIndicator(progress: Double, size: Int = 32) {
   LinearProgressIndicator(
-      progress = { progress.toFloat() },
+      progress = progress.toFloat(),
       modifier = Modifier.width(size.dp),
       color = ts_color_light_blue,
       trackColor = MaterialTheme.colorScheme.secondary,

diff --git a/android/src/main/java/com/tailscale/ipn/ui/view/TaildropView.kt b/android/src/main/java/com/tailscale/ipn/ui/view/TaildropView.kt
@@ -4,6 +4,7 @@
 package com.tailscale.ipn.ui.view
 
 import android.text.format.Formatter
+import androidx.compose.foundation.focusable
 import androidx.compose.foundation.layout.Arrangement
 import androidx.compose.foundation.layout.Column
 import androidx.compose.foundation.layout.Row
@@ -19,10 +20,14 @@ import androidx.compose.material3.MaterialTheme
 import androidx.compose.material3.Scaffold
 import androidx.compose.material3.Text
 import androidx.compose.runtime.Composable
+import androidx.compose.runtime.LaunchedEffect
 import androidx.compose.runtime.collectAsState
 import androidx.compose.runtime.getValue
+import androidx.compose.runtime.remember
 import androidx.compose.ui.Alignment
 import androidx.compose.ui.Modifier
+import androidx.compose.ui.focus.FocusRequester
+import androidx.compose.ui.focus.focusRequester
 import androidx.compose.ui.platform.LocalContext
 import androidx.compose.ui.res.painterResource
 import androidx.compose.ui.res.stringResource
@@ -36,6 +41,7 @@ import com.tailscale.ipn.ui.util.Lists.SectionDivider
 import com.tailscale.ipn.ui.util.set
 import com.tailscale.ipn.ui.viewModel.TaildropViewModel
 import com.tailscale.ipn.ui.viewModel.TaildropViewModelFactory
+import com.tailscale.ipn.util.TSLog
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.flow.StateFlow
 
@@ -46,36 +52,44 @@ fun TaildropView(
     viewModel: TaildropViewModel =
         viewModel(factory = TaildropViewModelFactory(requestedTransfers, applicationScope))
 ) {
-  Scaffold(
-      contentWindowInsets = WindowInsets.Companion.statusBars,
-      topBar = { Header(R.string.share) }) { paddingInsets ->
-        val showDialog = viewModel.showDialog.collectAsState().value
+  val TAG = "TaildropView"
+  val focusRequester = remember { FocusRequester() }
 
-        // Show the error overlay
-        showDialog?.let { ErrorDialog(type = it, action = { viewModel.showDialog.set(null) }) }
+  // Automatically request focus when the composable is displayed
+  LaunchedEffect(Unit) {
+    try {
+      focusRequester.requestFocus()
+    } catch (e: Exception) {
+      TSLog.w(TAG, "Focus request failed: ${e.message}")
+    }
+  }
 
-        Column(modifier = Modifier.padding(paddingInsets)) {
-          FileShareHeader(
-              fileTransfers = requestedTransfers.collectAsState().value,
-              totalSize = viewModel.totalSize)
+  Scaffold(contentWindowInsets = WindowInsets.statusBars, topBar = { Header(R.string.share) }) {
+      paddingInsets ->
+    Column(modifier = Modifier.focusRequester(focusRequester).focusable().padding(paddingInsets)) {
+      val showDialog = viewModel.showDialog.collectAsState().value
 
-          when (viewModel.state.collectAsState().value) {
-            Ipn.State.Running -> {
-              val peers by viewModel.myPeers.collectAsState()
-              val context = LocalContext.current
-              FileSharePeerList(
-                  peers = peers,
-                  stateViewGenerator = { peerId ->
-                    viewModel.TrailingContentForPeer(peerId = peerId)
-                  },
-                  onShare = { viewModel.share(context, it) })
-            }
-            else -> {
-              FileShareConnectView { viewModel.startVPN() }
-            }
-          }
+      showDialog?.let { ErrorDialog(type = it, action = { viewModel.showDialog.set(null) }) }
+
+      FileShareHeader(
+          fileTransfers = requestedTransfers.collectAsState().value,
+          totalSize = viewModel.totalSize)
+
+      when (viewModel.state.collectAsState().value) {
+        Ipn.State.Running -> {
+          val peers by viewModel.myPeers.collectAsState()
+          val context = LocalContext.current
+          FileSharePeerList(
+              peers = peers,
+              stateViewGenerator = { peerId -> viewModel.TrailingContentForPeer(peerId = peerId) },
+              onShare = { viewModel.share(context, it) })
+        }
+        else -> {
+          FileShareConnectView { viewModel.startVPN() }
         }
       }
+    }
+  }
 }
 
 @Composable