From bd745b5254d8f4bc3dd1a9277ab40ee098b13b5e Mon Sep 17 00:00:00 2001 From: kari-ts <135075563+kari-ts@users.noreply.github.com> Date: Tue, 5 Nov 2024 14:55:37 -0800 Subject: [PATCH] android: fix avatar padding (#559) Update Avatar to take isFocusable as a parameter, allowing us to make the avatar focusable in the main view but not in the settings / user switcher view. This fixes the issue where the padding is too big in the settings / user switcher view. Fixes tailscale/corp#24370 Signed-off-by: kari-ts --- .../java/com/tailscale/ipn/ui/view/Avatar.kt | 95 +++++++++---------- .../com/tailscale/ipn/ui/view/MainView.kt | 11 ++- 2 files changed, 54 insertions(+), 52 deletions(-) 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 baae957..6f2a1c3 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 @@ -34,57 +34,52 @@ import com.tailscale.ipn.ui.model.IpnLocal @OptIn(ExperimentalCoilApi::class) @Composable -fun Avatar(profile: IpnLocal.LoginProfile?, size: Int = 50, action: (() -> Unit)? = null) { - var isFocused = remember { mutableStateOf(false) } - val focusManager = LocalFocusManager.current +fun Avatar( + profile: IpnLocal.LoginProfile?, + size: Int = 50, + action: (() -> Unit)? = null, + isFocusable: Boolean = false +) { + 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() - focusManager.clearFocus() // Clear focus after clicking the avatar - } + // Determine the modifier based on whether the avatar is focusable + val outerModifier = + Modifier.then( + if (isFocusable) { + Modifier.padding(4.dp) + } else Modifier) // Add padding if focusable + .size((size * 1.5f).dp) + .clip(CircleShape) + .background(if (isFocused.value) MaterialTheme.colorScheme.surface else Color.Transparent) + .onFocusChanged { focusState -> isFocused.value = focusState.isFocused } + .then(if (isFocusable) Modifier.focusable() else Modifier) // Conditionally add focusable + .clickable( + interactionSource = remember { MutableInteractionSource() }, + indication = ripple(bounded = true), + onClick = { + action?.invoke() + focusManager.clearFocus() // Clear focus after clicking + }) + + // Outer Box for the larger focusable and clickable area + Box(contentAlignment = Alignment.Center, modifier = outerModifier) { + // Inner Box to hold the avatar content (Icon or AsyncImage) + Box(contentAlignment = Alignment.Center, modifier = Modifier.size(size.dp).clip(CircleShape)) { + if (profile?.UserProfile?.ProfilePicURL != null) { + AsyncImage( + model = profile.UserProfile.ProfilePicURL, + modifier = Modifier.size(size.dp).clip(CircleShape), + contentDescription = null) + } else { + 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 ) - ) { - // Inner Box to hold the avatar content (Icon or AsyncImage) - Box( - contentAlignment = Alignment.Center, - modifier = Modifier - .size(size.dp) - .clip(CircleShape) - ) { - if (profile?.UserProfile?.ProfilePicURL != null) { - AsyncImage( - model = profile.UserProfile.ProfilePicURL, - modifier = Modifier.size(size.dp).clip(CircleShape), - contentDescription = null - ) - } else { - 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 - ) - } - } + } } + } } - 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 4bc8188..419ccda 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 @@ -45,8 +45,15 @@ import androidx.compose.material3.Scaffold import androidx.compose.material3.SearchBar import androidx.compose.material3.SearchBarDefaults import androidx.compose.material3.Text -import androidx.compose.runtime.* +import androidx.compose.runtime.Composable +import androidx.compose.runtime.LaunchedEffect +import androidx.compose.runtime.collectAsState +import androidx.compose.runtime.derivedStateOf +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.remember import androidx.compose.runtime.saveable.rememberSaveable +import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.draw.alpha @@ -188,7 +195,7 @@ fun MainView( when (user) { null -> SettingsButton { navigation.onNavigateToSettings() } else -> { - Avatar(profile = user, size = 36) { navigation.onNavigateToSettings() } + Avatar(profile = user, size = 36, { navigation.onNavigateToSettings() }, isFocusable=true) } } }