From 0a1a26f299961405f72bf43e17ff310acc0537ea Mon Sep 17 00:00:00 2001 From: Jonathan Nobels Date: Tue, 26 Nov 2024 14:45:49 -0500 Subject: [PATCH] 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 --- .../src/main/java/com/tailscale/ipn/App.kt | 2 +- .../com/tailscale/ipn/ui/util/ModifierUtil.kt | 18 ++++ .../java/com/tailscale/ipn/ui/view/Avatar.kt | 86 +++++++++---------- .../ipn/ui/viewModel/MainViewModel.kt | 23 +++-- 4 files changed, 78 insertions(+), 51 deletions(-) create mode 100644 android/src/main/java/com/tailscale/ipn/ui/util/ModifierUtil.kt diff --git a/android/src/main/java/com/tailscale/ipn/App.kt b/android/src/main/java/com/tailscale/ipn/App.kt index fde6541fc1..7e49687ecb 100644 --- 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 new file mode 100644 index 0000000000..49d2f198df --- /dev/null +++ b/android/src/main/java/com/tailscale/ipn/ui/util/ModifierUtil.kt @@ -0,0 +1,18 @@ +// Copyright (c) Tailscale Inc & AUTHORS +// SPDX-License-Identifier: BSD-3-Clause + +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 index a7d65bd43c..89ef4d9dbf 100644 --- 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 index fdccab788d..fa92360b78 100644 --- 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 = vpnViewModel.vpnActive + var searchJob: Job? = null + // Icon displayed in the button to present the health view val healthIcon: StateFlow = MutableStateFlow(null) @@ -130,18 +134,25 @@ 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 + } } } viewModelScope.launch { Notifier.netmap.collect { it -> it?.let { netmap -> - peerCategorizer.regenerateGroupedPeers(netmap) - - // Immediately update _peers with the full peer list - _peers.value = peerCategorizer.groupedAndFilteredPeers(searchTerm.value) + searchJob?.cancel() + launch(Dispatchers.Default) { + peerCategorizer.regenerateGroupedPeers(netmap) + val filteredPeers = peerCategorizer.groupedAndFilteredPeers(searchTerm.value) + _peers.value = filteredPeers + } if (netmap.SelfNode.keyDoesNotExpire) { showExpiry.set(false)