refactor: Migrate Node dropdown menu to Compose (#1386)

app/src/main/java/com/geeksville/mesh/ui/NodeItem.kt

@@ -13,6 +13,7 @@ import androidx.compose.animation.core.repeatable
 import androidx.compose.animation.core.tween
 import androidx.compose.foundation.background
 import androidx.compose.foundation.layout.Arrangement
+import androidx.compose.foundation.layout.Box
 import androidx.compose.foundation.layout.Column
 import androidx.compose.foundation.layout.IntrinsicSize
 import androidx.compose.foundation.layout.Row
@@ -23,6 +24,7 @@ import androidx.compose.foundation.layout.height
 import androidx.compose.foundation.layout.padding
 import androidx.compose.foundation.layout.size
 import androidx.compose.foundation.layout.width
+import androidx.compose.foundation.layout.wrapContentSize
 import androidx.compose.foundation.text.selection.DisableSelection
 import androidx.compose.foundation.text.selection.SelectionContainer
 import androidx.compose.material.Card
@@ -56,7 +58,11 @@ import com.geeksville.mesh.ConfigProtos.Config.DisplayConfig
 import com.geeksville.mesh.MeshProtos
 import com.geeksville.mesh.R
 import com.geeksville.mesh.database.entity.NodeEntity
+import com.geeksville.mesh.service.MeshService
+import com.geeksville.mesh.service.MeshService.ConnectionState
+import com.geeksville.mesh.ui.components.MenuItemAction
 import com.geeksville.mesh.ui.components.NodeKeyStatusIcon
+import com.geeksville.mesh.ui.components.NodeMenu
 import com.geeksville.mesh.ui.components.SimpleAlertDialog
 import com.geeksville.mesh.ui.compose.ElevationInfo
 import com.geeksville.mesh.ui.compose.SatelliteCountInfo
@@ -73,11 +79,12 @@ fun NodeItem(
     gpsFormat: Int,
     distanceUnits: Int,
     tempInFahrenheit: Boolean,
-    isIgnored: Boolean = false,
-    chipClicked: () -> Unit = {},
+    ignoreIncomingList: List<Int> = emptyList(),
+    menuItemActionClicked: (MenuItemAction) -> Unit = {},
     blinking: Boolean = false,
     expanded: Boolean = false,
     currentTimeMillis: Long,
+    connectionState: MeshService.ConnectionState? = ConnectionState.DISCONNECTED,
 ) {
     val isUnknownUser = thatNode.isUnknownUser
     val unknownShortName = stringResource(id = R.string.unknown_node_short_name)
@@ -154,26 +161,44 @@ fun NodeItem(
                             .fillMaxWidth(),
                         verticalAlignment = Alignment.CenterVertically,
                     ) {
-                        Chip(
-                            modifier = Modifier
-                                .width(IntrinsicSize.Min)
-                                .defaultMinSize(minHeight = 32.dp, minWidth = 72.dp),
-                            colors = ChipDefaults.chipColors(
-                                backgroundColor = Color(nodeColor),
-                                contentColor = Color(textColor)
-                            ),
-                            onClick = { chipClicked() },
-                            content = {
-                                Text(
-                                    modifier = Modifier.fillMaxWidth(),
-                                    text = thatNode.user.shortName.ifEmpty { unknownShortName },
-                                    fontWeight = FontWeight.Normal,
-                                    fontSize = MaterialTheme.typography.button.fontSize,
-                                    textDecoration = TextDecoration.LineThrough.takeIf { isIgnored },
-                                    textAlign = TextAlign.Center,
-                                )
-                            },
-                        )
+                        var menuExpanded by remember { mutableStateOf(false) }
+                        Box(
+                            modifier = Modifier.wrapContentSize(Alignment.TopStart)
+                        ) {
+                            Chip(
+                                modifier = Modifier
+                                    .width(IntrinsicSize.Min)
+                                    .defaultMinSize(minHeight = 32.dp, minWidth = 72.dp),
+                                colors = ChipDefaults.chipColors(
+                                    backgroundColor = Color(nodeColor),
+                                    contentColor = Color(textColor)
+                                ),
+                                onClick = {
+                                    menuExpanded = !menuExpanded
+                                },
+                                content = {
+                                    Text(
+                                        modifier = Modifier.fillMaxWidth(),
+                                        text = thatNode.user.shortName.ifEmpty { unknownShortName },
+                                        fontWeight = FontWeight.Normal,
+                                        fontSize = MaterialTheme.typography.button.fontSize,
+                                        textDecoration = TextDecoration.LineThrough.takeIf {
+                                            ignoreIncomingList.contains(thatNode.num)
+                                        },
+                                        textAlign = TextAlign.Center,
+                                    )
+                                },
+                            )
+                            NodeMenu(
+                                node = thatNode,
+                                ignoreIncomingList = ignoreIncomingList,
+                                isThisNode = isThisNode,
+                                onMenuItemAction = menuItemActionClicked,
+                                expanded = menuExpanded,
+                                onDismissRequest = { menuExpanded = false },
+                                isConnected = connectionState == ConnectionState.CONNECTED,
+                            )
+                        }
                         NodeKeyStatusIcon(
                             hasPKC = thatNode.hasPKC,
                             mismatchKey = thatNode.mismatchKey,
@@ -183,7 +208,11 @@ fun NodeItem(
                             modifier = Modifier.weight(1f),
                             text = longName,
                             style = style,
-                            textDecoration = TextDecoration.LineThrough.takeIf { isIgnored },
+                            textDecoration = TextDecoration.LineThrough.takeIf {
+                                ignoreIncomingList.contains(
+                                    thatNode.num
+                                )
+                            },
                             softWrap = true,
                         )
 

app/src/main/java/com/geeksville/mesh/ui/NodeMenu.kt

@@ -1,58 +0,0 @@
-package com.geeksville.mesh.ui
-
-import android.view.Gravity
-import android.view.MenuItem
-import android.view.View
-import androidx.appcompat.widget.PopupMenu
-import com.geeksville.mesh.R
-import com.geeksville.mesh.database.entity.NodeEntity
-import com.google.android.material.dialog.MaterialAlertDialogBuilder
-
-internal fun View.nodeMenu(
-    node: NodeEntity,
-    ignoreIncomingList: List<Int>,
-    isOurNode: Boolean = false,
-    onMenuItemAction: MenuItem.() -> Unit,
-) = PopupMenu(context, this, Gravity.NO_GRAVITY, R.attr.actionOverflowMenuStyle, 0).apply {
-    val isIgnored = ignoreIncomingList.contains(node.num)
-
-    inflate(R.menu.menu_nodes)
-    menu.apply {
-        setGroupVisible(R.id.group_remote, !isOurNode)
-        findItem(R.id.ignore).apply {
-            isEnabled = isIgnored || ignoreIncomingList.size < 3
-            isChecked = isIgnored
-        }
-    }
-    setOnMenuItemClickListener { item ->
-        when (item.itemId) {
-            R.id.remove -> {
-                MaterialAlertDialogBuilder(context)
-                    .setTitle(R.string.remove)
-                    .setMessage(R.string.remove_node_text)
-                    .setNeutralButton(R.string.cancel) { _, _ -> }
-                    .setPositiveButton(R.string.send) { _, _ ->
-                        item.onMenuItemAction()
-                    }
-                    .show()
-            }
-
-            R.id.ignore -> {
-                val message = if (isIgnored) R.string.ignore_remove else R.string.ignore_add
-                MaterialAlertDialogBuilder(context)
-                    .setTitle(R.string.ignore)
-                    .setMessage(context.getString(message, node.user.longName))
-                    .setNeutralButton(R.string.cancel) { _, _ -> }
-                    .setPositiveButton(R.string.send) { _, _ ->
-                        item.onMenuItemAction()
-                    }
-                    .show()
-                item.isChecked = !item.isChecked
-            }
-
-            else -> item.onMenuItemAction()
-        }
-        true
-    }
-    show()
-}

app/src/main/java/com/geeksville/mesh/ui/UsersFragment.kt

@@ -14,19 +14,20 @@ import androidx.compose.foundation.lazy.rememberLazyListState
 import androidx.compose.runtime.Composable
 import androidx.compose.runtime.LaunchedEffect
 import androidx.compose.runtime.getValue
+import androidx.compose.runtime.livedata.observeAsState
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.platform.ComposeView
-import androidx.compose.ui.platform.LocalFocusManager
 import androidx.compose.ui.platform.ViewCompositionStrategy
 import androidx.compose.ui.unit.dp
 import androidx.fragment.app.activityViewModels
 import androidx.hilt.navigation.compose.hiltViewModel
 import androidx.lifecycle.compose.collectAsStateWithLifecycle
+import ar.com.hjg.pngj.PngHelperInternal.debug
 import com.geeksville.mesh.DataPacket
-import com.geeksville.mesh.R
 import com.geeksville.mesh.android.Logging
 import com.geeksville.mesh.database.entity.NodeEntity
 import com.geeksville.mesh.model.UIViewModel
+import com.geeksville.mesh.ui.components.MenuItemAction
 import com.geeksville.mesh.ui.components.NodeFilterTextField
 import com.geeksville.mesh.ui.components.rememberTimeTickWithLifecycle
 import com.geeksville.mesh.ui.theme.AppTheme
@@ -37,56 +38,6 @@ class UsersFragment : ScreenFragment("Users"), Logging {
 
     private val model: UIViewModel by activityViewModels()
 
-    private fun popup(node: NodeEntity) {
-        if (!model.isConnected()) return
-        val isOurNode = node.num == model.myNodeNum
-        val ignoreIncomingList = model.ignoreIncomingList
-
-        requireView().nodeMenu(
-            node = node,
-            ignoreIncomingList = ignoreIncomingList,
-            isOurNode = isOurNode,
-        ) {
-            when (itemId) {
-                R.id.direct_message -> {
-                    navigateToMessages(node)
-                }
-
-                R.id.request_position -> {
-                    model.requestPosition(node.num)
-                }
-
-                R.id.traceroute -> {
-                    model.requestTraceroute(node.num)
-                }
-
-                R.id.remove -> {
-                    model.removeNode(node.num)
-                }
-
-                R.id.ignore -> {
-                    model.ignoreIncomingList = ignoreIncomingList.toMutableList().apply {
-                        if (contains(node.num)) {
-                            debug("removed '${node.num}' from ignore list")
-                            remove(node.num)
-                        } else {
-                            debug("added '${node.num}' to ignore list")
-                            add(node.num)
-                        }
-                    }
-                }
-
-                R.id.more_details -> {
-                    navigateToRadioConfig(node.num)
-                }
-
-                R.id.request_userinfo -> {
-                    model.requestUserInfo(node.num)
-                }
-            }
-        }
-    }
-
     private fun navigateToMessages(node: NodeEntity) = node.user.let { user ->
         val hasPKC = model.ourNodeInfo.value?.hasPKC == true && node.hasPKC // TODO use meta.hasPKC
         val channel = if (hasPKC) DataPacket.PKC_CHANNEL_INDEX else node.channel
@@ -95,7 +46,7 @@ class UsersFragment : ScreenFragment("Users"), Logging {
         parentFragmentManager.navigateToMessages(contactKey, user.longName)
     }
 
-    private fun navigateToRadioConfig(nodeNum: Int) {
+    private fun navigateToNodeDetails(nodeNum: Int) {
         info("calling NodeDetails --> destNum: $nodeNum")
         parentFragmentManager.navigateToNavGraph(nodeNum, "NodeDetails")
     }
@@ -109,7 +60,11 @@ class UsersFragment : ScreenFragment("Users"), Logging {
             setViewCompositionStrategy(ViewCompositionStrategy.DisposeOnViewTreeLifecycleDestroyed)
             setContent {
                 AppTheme {
-                    NodesScreen(model = model, chipClicked = ::popup)
+                    NodesScreen(
+                        model = model,
+                        navigateToMessages = ::navigateToMessages,
+                        navigateToNodeDetails = ::navigateToNodeDetails,
+                    )
                 }
             }
         }
@@ -118,11 +73,12 @@ class UsersFragment : ScreenFragment("Users"), Logging {
 
 @OptIn(ExperimentalFoundationApi::class)
 @Composable
+@Suppress("LongMethod", "CyclomaticComplexMethod")
 fun NodesScreen(
     model: UIViewModel = hiltViewModel(),
-    chipClicked: (NodeEntity) -> Unit,
+    navigateToMessages: (NodeEntity) -> Unit,
+    navigateToNodeDetails: (Int) -> Unit,
 ) {
-    val focusManager = LocalFocusManager.current
     val state by model.nodesUiState.collectAsStateWithLifecycle()
 
     val nodes by model.nodeList.collectAsStateWithLifecycle()
@@ -163,20 +119,60 @@ fun NodesScreen(
         }
 
         items(nodes, key = { it.num }) { node ->
+            val isIgnored = state.ignoreIncomingList.contains(node.num)
+            val connectionState by model.connectionState.observeAsState()
             NodeItem(
                 thisNode = ourNode,
                 thatNode = node,
                 gpsFormat = state.gpsFormat,
                 distanceUnits = state.distanceUnits,
                 tempInFahrenheit = state.tempInFahrenheit,
-                isIgnored = state.ignoreIncomingList.contains(node.num),
-                chipClicked = {
-                    focusManager.clearFocus()
-                    chipClicked(node)
+                ignoreIncomingList = state.ignoreIncomingList,
+                menuItemActionClicked = { menuItem ->
+                    when (menuItem) {
+                        MenuItemAction.Remove -> {
+                            model.removeNode(node.num)
+                            debug("removing node ${node.num}")
+                        }
+
+                        MenuItemAction.Ignore -> {
+                            model.ignoreIncomingList =
+                                state.ignoreIncomingList.toMutableList().apply {
+                                    if (isIgnored) {
+                                        remove(node.num)
+                                        debug("removing node ${node.num} from ignore list")
+                                    } else {
+                                        add(node.num)
+                                        debug("adding node ${node.num} to ignore list")
+                                    }
+                                }
+                        }
+
+                        MenuItemAction.DirectMessage -> {
+                            navigateToMessages(node)
+                        }
+
+                        MenuItemAction.RequestUserInfo -> {
+                            model.requestUserInfo(node.num)
+                        }
+
+                        MenuItemAction.RequestPosition -> {
+                            model.requestPosition(node.num)
+                        }
+
+                        MenuItemAction.TraceRoute -> {
+                            model.requestTraceroute(node.num)
+                        }
+
+                        MenuItemAction.MoreDetails -> {
+                            navigateToNodeDetails(node.num)
+                        }
+                    }
                 },
                 blinking = node == focusedNode,
                 expanded = state.showDetails,
                 currentTimeMillis = currentTimeMillis,
+                connectionState = connectionState,
             )
         }
     }

app/src/main/java/com/geeksville/mesh/ui/components/NodeMenu.kt

@@ -0,0 +1,149 @@
+package com.geeksville.mesh.ui.components
+
+import androidx.compose.foundation.background
+import androidx.compose.material.Checkbox
+import androidx.compose.material.CheckboxDefaults
+import androidx.compose.material.Divider
+import androidx.compose.material.DropdownMenu
+import androidx.compose.material.DropdownMenuItem
+import androidx.compose.material.MaterialTheme
+import androidx.compose.material.Text
+import androidx.compose.runtime.Composable
+import androidx.compose.runtime.getValue
+import androidx.compose.runtime.mutableStateOf
+import androidx.compose.runtime.remember
+import androidx.compose.runtime.setValue
+import androidx.compose.ui.Modifier
+import androidx.compose.ui.res.stringResource
+import com.geeksville.mesh.R
+import com.geeksville.mesh.database.entity.NodeEntity
+
+@Suppress("LongMethod")
+@Composable
+fun NodeMenu(
+    node: NodeEntity,
+    ignoreIncomingList: List<Int>,
+    isThisNode: Boolean = false,
+    onMenuItemAction: (MenuItemAction) -> Unit,
+    onDismissRequest: () -> Unit,
+    expanded: Boolean = false,
+    isConnected: Boolean = false,
+) {
+    val isIgnored = ignoreIncomingList.contains(node.num)
+    var displayIgnoreDialog by remember { mutableStateOf(false) }
+    var displayRemoveDialog by remember { mutableStateOf(false) }
+    if (displayIgnoreDialog) {
+        SimpleAlertDialog(
+            title = R.string.ignore,
+            text = stringResource(
+                id = if (isIgnored) R.string.ignore_remove else R.string.ignore_add,
+                node.user.longName
+            ),
+            onConfirm = {
+                displayIgnoreDialog = false
+                onMenuItemAction(MenuItemAction.Ignore)
+            },
+            onDismiss = {
+                displayIgnoreDialog = false
+            }
+        )
+    }
+    if (displayRemoveDialog) {
+        SimpleAlertDialog(
+            title = R.string.remove,
+            text = R.string.remove_node_text,
+            onConfirm = {
+                displayRemoveDialog = false
+                onMenuItemAction(MenuItemAction.Remove)
+            },
+            onDismiss = {
+                displayRemoveDialog = false
+            }
+        )
+    }
+    DropdownMenu(
+        modifier = Modifier.background(MaterialTheme.colors.background.copy(alpha = 1f)),
+        expanded = expanded,
+        onDismissRequest = onDismissRequest,
+    ) {
+
+        if (!isThisNode && isConnected) {
+            DropdownMenuItem(
+                onClick = {
+                    onDismissRequest()
+                    onMenuItemAction(MenuItemAction.DirectMessage)
+                },
+                content = { Text(stringResource(R.string.direct_message)) }
+            )
+            Divider()
+            DropdownMenuItem(
+                onClick = {
+                    onDismissRequest()
+                    onMenuItemAction(MenuItemAction.RequestUserInfo)
+                },
+                content = { Text(stringResource(R.string.request_userinfo)) }
+            )
+            Divider()
+            DropdownMenuItem(
+                onClick = {
+                    onDismissRequest()
+                    onMenuItemAction(MenuItemAction.RequestPosition)
+                },
+                content = { Text(stringResource(R.string.request_position)) }
+            )
+            Divider()
+            DropdownMenuItem(
+                onClick = {
+                    onDismissRequest()
+                    onMenuItemAction(MenuItemAction.TraceRoute)
+                },
+                content = { Text(stringResource(R.string.traceroute)) }
+            )
+            Divider()
+            DropdownMenuItem(
+                onClick = {
+                    onDismissRequest()
+                    displayRemoveDialog = true
+                },
+                content = { Text(stringResource(R.string.remove)) },
+            )
+            Divider()
+            DropdownMenuItem(
+                onClick = {
+                    onDismissRequest()
+                    displayIgnoreDialog = true
+                },
+                content = {
+                    Text(stringResource(R.string.ignore))
+                    Checkbox(
+                        colors = CheckboxDefaults.colors(checkedColor = MaterialTheme.colors.primary),
+                        checked = isIgnored,
+                        onCheckedChange = {
+                            onDismissRequest()
+                            displayIgnoreDialog = true
+                        },
+                    )
+                },
+                enabled = ignoreIncomingList.size < 3 || isIgnored
+            )
+            Divider()
+        }
+        DropdownMenuItem(
+            onClick = {
+                onDismissRequest()
+                onMenuItemAction(MenuItemAction.MoreDetails)
+            },
+            content = { Text(stringResource(R.string.more_details)) }
+        )
+    }
+}
+
+enum class MenuItemAction {
+    Remove,
+    Ignore,
+    DirectMessage,
+    RequestUserInfo,
+    RequestPosition,
+    TraceRoute,
+    MoreDetails
+}

app/src/main/java/com/geeksville/mesh/ui/components/SimpleAlertDialog.kt

@@ -1,7 +1,6 @@
 package com.geeksville.mesh.ui.components
 
 import androidx.annotation.StringRes
-import androidx.compose.foundation.background
 import androidx.compose.foundation.layout.fillMaxWidth
 import androidx.compose.foundation.layout.padding
 import androidx.compose.foundation.shape.RoundedCornerShape
@@ -23,10 +22,11 @@ import com.geeksville.mesh.ui.theme.AppTheme
 fun SimpleAlertDialog(
     @StringRes title: Int,
     text: @Composable (() -> Unit)? = null,
+    onConfirm: (() -> Unit)? = null,
     onDismiss: () -> Unit = {},
 ) = AlertDialog(
     onDismissRequest = onDismiss,
-    confirmButton = {
+    dismissButton = {
         TextButton(
             onClick = onDismiss,
             modifier = Modifier
@@ -36,6 +36,18 @@ fun SimpleAlertDialog(
             ),
         ) { Text(text = stringResource(id = R.string.close)) }
     },
+    confirmButton = {
+        onConfirm?.let {
+        TextButton(
+            onClick = onConfirm,
+            modifier = Modifier
+                .padding(horizontal = 16.dp),
+            colors = ButtonDefaults.textButtonColors(
+                contentColor = MaterialTheme.colors.onSurface,
+            ),
+        ) { Text(text = stringResource(id = R.string.okay)) }
+        }
+    },
     title = {
         Text(
             text = stringResource(id = title),
@@ -52,8 +64,10 @@ fun SimpleAlertDialog(
 fun SimpleAlertDialog(
     @StringRes title: Int,
     @StringRes text: Int,
+    onConfirm: (() -> Unit)? = null,
     onDismiss: () -> Unit = {},
 ) = SimpleAlertDialog(
+    onConfirm = onConfirm,
     onDismiss = onDismiss,
     title = title,
     text = {
@@ -65,6 +79,25 @@ fun SimpleAlertDialog(
     },
 )
 
+@Composable
+fun SimpleAlertDialog(
+    @StringRes title: Int,
+    text: String,
+    onConfirm: (() -> Unit)? = null,
+    onDismiss: () -> Unit = {},
+) = SimpleAlertDialog(
+    onConfirm = onConfirm,
+    onDismiss = onDismiss,
+    title = title,
+    text = {
+        Text(
+            text = text,
+            modifier = Modifier.fillMaxWidth(),
+            textAlign = TextAlign.Center,
+        )
+    },
+)
+
 @PreviewLightDark
 @Composable
 private fun SimpleAlertDialogPreview() {

app/src/main/res/menu/menu_nodes.xml

@@ -1,37 +0,0 @@
-<?xml version="1.0" encoding="utf-8"?>
-<menu xmlns:android="http://schemas.android.com/apk/res/android"
-    xmlns:app="http://schemas.android.com/apk/res-auto">
-    <group android:id="@+id/group_remote">
-        <item
-            android:id="@+id/direct_message"
-            android:title="@string/direct_message"
-            app:showAsAction="withText" />
-        <item
-            android:id="@+id/request_userinfo"
-            android:title="@string/request_userinfo"
-            app:showAsAction="withText" />
-        <item
-            android:id="@+id/request_position"
-            android:title="@string/request_position"
-            app:showAsAction="withText" />
-        <item
-            android:id="@+id/traceroute"
-            android:title="@string/traceroute"
-            app:showAsAction="withText" />
-        <item
-            android:id="@+id/ignore"
-            android:checkable="true"
-            android:checked="false"
-            android:title="@string/ignore" />
-        <item
-            android:id="@+id/remove"
-            android:title="@string/remove"
-            app:showAsAction="withText" />
-    </group>
-    <group android:id="@+id/group_both">
-        <item
-            android:id="@+id/more_details"
-            android:title="@string/more_details"
-            app:showAsAction="withText" />
-    </group>
-</menu>