android: use background search and fix avatar padding

fixes tailscale/corp#24847 fixes tailsacle/corp#24848 Search jobs are moved to the default dispatcher so they do not block the UI thread. The avatar boxing is now used only conditionally on AndroidTV. Signed-off-by: Jonathan Nobels <[email protected]>
tailscale · Nov 26, 2024 · f4bbd44 · f4bbd44
1 parent 61c7c3c
commit f4bbd44
Show file tree

Hide file tree

Showing 4 changed files with 75 additions and 48 deletions.
diff --git a/android/src/main/java/com/tailscale/ipn/App.kt b/android/src/main/java/com/tailscale/ipn/App.kt
@@ -163,7 +163,7 @@ class App : UninitializedApp(), libtailscale.AppContext, ViewModelStoreOwner {
     NetworkChangeCallback.monitorDnsChanges(connectivityManager, dns)
     initViewModels()
     applicationScope.launch {
-      Notifier.state.collect { state ->
+      Notifier.state.collect { _ ->
         combine(Notifier.state, MDMSettings.forceEnabled.flow) { state, forceEnabled ->
               Pair(state, forceEnabled)
             }

diff --git a/android/src/main/java/com/tailscale/ipn/ui/util/ModifierUtil.kt b/android/src/main/java/com/tailscale/ipn/ui/util/ModifierUtil.kt
@@ -0,0 +1,15 @@
+package com.tailscale.ipn.ui.util
+
+import androidx.compose.ui.Modifier
+
+/// Applies different modifiers to the receiver based on a condition.
+inline fun Modifier.conditional(
+    condition: Boolean,
+    ifTrue: Modifier.() -> Modifier,
+    ifFalse: Modifier.() -> Modifier = { this },
+): Modifier =
+    if (condition) {
+      then(ifTrue(Modifier))
+    } else {
+      then(ifFalse(Modifier))
+    }
diff --git a/android/src/main/java/com/tailscale/ipn/ui/view/Avatar.kt b/android/src/main/java/com/tailscale/ipn/ui/view/Avatar.kt
@@ -31,6 +31,8 @@ import coil.annotation.ExperimentalCoilApi
 import coil.compose.AsyncImage
 import com.tailscale.ipn.R
 import com.tailscale.ipn.ui.model.IpnLocal
+import com.tailscale.ipn.ui.util.AndroidTVUtil
+import com.tailscale.ipn.ui.util.conditional
 
 @OptIn(ExperimentalCoilApi::class)
 @Composable
@@ -43,53 +45,49 @@ fun Avatar(
   var isFocused = remember { mutableStateOf(false) }
   val focusManager = LocalFocusManager.current
 
-    // Outer Box for the larger focusable and clickable area
-    Box(
-        contentAlignment = Alignment.Center,
-        modifier = Modifier
-            .padding(4.dp) 
-            .size((size * 1.5f).dp) // Focusable area is larger than the avatar
-            .clip(CircleShape) // Ensure both the focus and click area are circular
-            .background(
-                if (isFocused.value) MaterialTheme.colorScheme.surface
-                else Color.Transparent, 
-            )
-            .onFocusChanged { focusState ->
-                isFocused.value = focusState.isFocused 
-            }
-            .focusable() // Make this outer Box focusable (after onFocusChanged)
-            .clickable(
-                interactionSource = remember { MutableInteractionSource() },
-                indication = ripple(bounded = true), // Apply ripple effect inside circular bounds
-                onClick = {
-                    action?.invoke() 
+  // Outer Box for the larger focusable and clickable area
+  Box(
+      contentAlignment = Alignment.Center,
+      modifier =
+          Modifier.conditional(AndroidTVUtil.isAndroidTV(), { padding(4.dp) })
+              .conditional(
+                  AndroidTVUtil.isAndroidTV(),
+                  {
+                    size((size * 1.5f).dp) // Focusable area is larger than the avatar
+                  })
+              .clip(CircleShape) // Ensure both the focus and click area are circular
+              .background(
+                  if (isFocused.value) MaterialTheme.colorScheme.surface else Color.Transparent,
+              )
+              .onFocusChanged { focusState -> isFocused.value = focusState.isFocused }
+              .focusable() // Make this outer Box focusable (after onFocusChanged)
+              .clickable(
+                  interactionSource = remember { MutableInteractionSource() },
+                  indication = ripple(bounded = true), // Apply ripple effect inside circular bounds
+                  onClick = {
+                    action?.invoke()
                     focusManager.clearFocus() // Clear focus after clicking the avatar
-                }
-            )
-    ) {
+                  })) {
         // Inner Box to hold the avatar content (Icon or AsyncImage)
         Box(
             contentAlignment = Alignment.Center,
-            modifier = Modifier
-                .size(size.dp)
-                .clip(CircleShape) 
-        ) { 
-          // Always display the default icon as a background layer
-          Icon(
-              imageVector = Icons.Default.Person,
-              contentDescription = stringResource(R.string.settings_title),
-              modifier =
-                  Modifier.size((size * 0.8f).dp)
-                      .clip(CircleShape) // Icon size slightly smaller than the Box
-              )
+            modifier = Modifier.size(size.dp).clip(CircleShape)) {
+              // Always display the default icon as a background layer
+              Icon(
+                  imageVector = Icons.Default.Person,
+                  contentDescription = stringResource(R.string.settings_title),
+                  modifier =
+                      Modifier.conditional(AndroidTVUtil.isAndroidTV(), { size((size * 0.8f).dp) })
+                          .clip(CircleShape) // Icon size slightly smaller than the Box
+                  )
 
-          // Overlay the profile picture if available
-          profile?.UserProfile?.ProfilePicURL?.let { url ->
-            AsyncImage(
-                model = url,
-                modifier = Modifier.size(size.dp).clip(CircleShape),
-                contentDescription = null)
-          }
-        }
-  }
+              // Overlay the profile picture if available
+              profile?.UserProfile?.ProfilePicURL?.let { url ->
+                AsyncImage(
+                    model = url,
+                    modifier = Modifier.size(size.dp).clip(CircleShape),
+                    contentDescription = null)
+              }
+            }
+      }
 }
diff --git a/android/src/main/java/com/tailscale/ipn/ui/viewModel/MainViewModel.kt b/android/src/main/java/com/tailscale/ipn/ui/viewModel/MainViewModel.kt
@@ -23,7 +23,9 @@ import com.tailscale.ipn.ui.util.PeerCategorizer
 import com.tailscale.ipn.ui.util.PeerSet
 import com.tailscale.ipn.ui.util.TimeUtil
 import com.tailscale.ipn.ui.util.set
+import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.FlowPreview
+import kotlinx.coroutines.Job
 import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.flow.StateFlow
 import kotlinx.coroutines.flow.combine
@@ -77,6 +79,8 @@ class MainViewModel(private val vpnViewModel: VpnViewModel) : IpnViewModel() {
 
   val isVpnActive: StateFlow<Boolean> = vpnViewModel.vpnActive
 
+  var searchJob: Job? = null
+
   // Icon displayed in the button to present the health view
   val healthIcon: StateFlow<Int?> = MutableStateFlow(null)
 
@@ -130,8 +134,13 @@ class MainViewModel(private val vpnViewModel: VpnViewModel) : IpnViewModel() {
 
     viewModelScope.launch {
       _searchTerm.debounce(250L).collect { term ->
-        val filteredPeers = peerCategorizer.groupedAndFilteredPeers(term)
-        _peers.value = filteredPeers
+        // run the search as a background task
+        searchJob?.cancel()
+        searchJob =
+            launch(Dispatchers.Default) {
+              val filteredPeers = peerCategorizer.groupedAndFilteredPeers(term)
+              _peers.value = filteredPeers
+            }
       }
     }
 
@@ -141,7 +150,12 @@ class MainViewModel(private val vpnViewModel: VpnViewModel) : IpnViewModel() {
           peerCategorizer.regenerateGroupedPeers(netmap)
 
           // Immediately update _peers with the full peer list
-          _peers.value = peerCategorizer.groupedAndFilteredPeers(searchTerm.value)
+          searchJob?.cancel()
+          searchJob =
+              launch(Dispatchers.Default) {
+                val filteredPeers = peerCategorizer.groupedAndFilteredPeers(searchTerm.value)
+                _peers.value = filteredPeers
+              }
 
           if (netmap.SelfNode.keyDoesNotExpire) {
             showExpiry.set(false)