From 419d3ec3355d233e51d63e4ce2c6219a8a5d14ee Mon Sep 17 00:00:00 2001
From: Jeremiah K <17190268+jeremiah-k@users.noreply.github.com>
Date: Sun, 21 Jun 2026 18:46:51 -0500
Subject: [PATCH] fix(ui): show Wi-Fi unavailable banner only during active
 network scan (#5892)

---
 .../core/ui/util/NetworkTransportInfo.kt      | 23 ++++---
 .../core/ui/util/NetworkTransportTest.kt      | 61 +++++++++++++------
 .../connections/ui/ConnectionsScreen.kt       |  8 +--
 3 files changed, 58 insertions(+), 34 deletions(-)

diff --git a/core/ui/src/commonMain/kotlin/org/meshtastic/core/ui/util/NetworkTransportInfo.kt b/core/ui/src/commonMain/kotlin/org/meshtastic/core/ui/util/NetworkTransportInfo.kt
index a4f673918..79a2b567d 100644
--- a/core/ui/src/commonMain/kotlin/org/meshtastic/core/ui/util/NetworkTransportInfo.kt
+++ b/core/ui/src/commonMain/kotlin/org/meshtastic/core/ui/util/NetworkTransportInfo.kt
@@ -46,19 +46,22 @@ internal fun anyNetworkScanTransportAvailable(networks: List<NetworkTransportInf
 /**
  * Returns `true` when the "Wi-Fi unavailable" recovery banner should render in `ConnectionsScreen`.
  *
- * The banner surfaces "no usable transport for NSD/mDNS scan" only while the user is actually trying to use network
- * discovery: when the Network section is visible OR an active scan is in progress.
+ * Banner shows only while a network scan is actively running, local-network permission is granted, WiFi is unavailable,
+ * and the scan has not yet produced any discovered TCP nodes. The auto-scan case is covered because `isNetworkScanning`
+ * is true during auto-scan regardless of the user's transport-chip preference.
  *
- * The previous gate (`showNetworkTransport &&` alone) missed the auto-scan case — `LifecycleStartEffect` keys
- * `startNetworkScan()` off `networkAutoScan + permission`, not the section-visibility chip, so a user with the Network
- * filter chip toggled off but auto-scan on gets a running scan that can't find anything with the recovery banner
- * suppressed. Widening to `showNetworkTransport || isNetworkScanning` surfaces the hint whenever the user is actually
- * interacting with network discovery, without overlapping the permission-request flow on the scan toggle (still gated
- * by [localNetworkPermissionGranted]).
+ * Gating on the scan state (rather than the `showNetworkTransport` chip preference, which defaults to on) keeps the
+ * banner silent while discovery is idle — the user only needs the recovery hint at the moment a scan cannot find a
+ * usable transport. The [localNetworkPermissionGranted] guard keeps the banner from overlapping the permission-request
+ * flow on the scan toggle.
+ *
+ * The banner is a recovery hint for the case where the user has started a network scan but no nodes have been
+ * discovered yet. Once the scan produces results, the user has found what they were looking for and the hint is no
+ * longer useful — so the banner is suppressed when the discovered-TCP list is non-empty.
  */
 fun shouldShowWifiUnavailableBanner(
-    showNetworkTransport: Boolean,
     isNetworkScanning: Boolean,
     localNetworkPermissionGranted: Boolean,
     wifiUnavailable: Boolean,
-): Boolean = (showNetworkTransport || isNetworkScanning) && localNetworkPermissionGranted && wifiUnavailable
+    discoveredTcpDevicesEmpty: Boolean,
+): Boolean = isNetworkScanning && localNetworkPermissionGranted && wifiUnavailable && discoveredTcpDevicesEmpty
diff --git a/core/ui/src/commonTest/kotlin/org/meshtastic/core/ui/util/NetworkTransportTest.kt b/core/ui/src/commonTest/kotlin/org/meshtastic/core/ui/util/NetworkTransportTest.kt
index 6f0c8e55a..ca26d502a 100644
--- a/core/ui/src/commonTest/kotlin/org/meshtastic/core/ui/util/NetworkTransportTest.kt
+++ b/core/ui/src/commonTest/kotlin/org/meshtastic/core/ui/util/NetworkTransportTest.kt
@@ -177,9 +177,10 @@ class NetworkTransportTest {
 
 /**
  * Coverage for the [shouldShowWifiUnavailableBanner] gate consumed by `ConnectionsScreen`. The banner surfaces "no
- * usable transport for NSD/mDNS scan" only while the user is actually trying to use network discovery. Each case
- * mirrors one of the four reported combinations of (section-visible, scan-active) plus the permission and
- * transport-availability guards.
+ * usable transport for NSD/mDNS scan" only while a network scan is actively running, and is suppressed once the scan
+ * has produced discovered TCP nodes — at that point the user has found what they were looking for and the recovery hint
+ * is no longer useful. Each case mirrors one of the reported combinations of (scan-active, results-empty) plus the
+ * permission and transport-availability guards.
  */
 class WifiUnavailableBannerTest {
     private fun transports(wifi: Boolean = false) =
@@ -187,41 +188,46 @@ class WifiUnavailableBannerTest {
         listOf(NetworkTransportInfo(hasWifi = wifi, hasEthernet = false, hasVpn = false))
 
     @Test
-    fun section_visible_permission_granted_cellular_only_then_banner_shows() {
-        // Baseline: Network chip visible, permission granted, cellular-only — banner shows.
-        assertTrue(
+    fun scan_inactive_permission_granted_cellular_only_then_banner_hidden() {
+        // Pins the user-reported bug: no scan is running, permission granted, cellular-only
+        // transport. The banner must NOT render — the user is not actively trying to discover, so
+        // the recovery hint is noise.
+        assertFalse(
             shouldShowWifiUnavailableBanner(
-                showNetworkTransport = true,
                 isNetworkScanning = false,
                 localNetworkPermissionGranted = true,
                 wifiUnavailable = !anyNetworkScanTransportAvailable(transports()),
+                discoveredTcpDevicesEmpty = true,
             ),
         )
     }
 
     @Test
-    fun scan_active_section_hidden_cellular_only_then_banner_shows() {
-        // The user-reported regression: Network chip toggled off but auto-scan running in the background,
-        // cellular-only transport. The pre-fix gate (`showNetworkTransport &&` alone) suppressed this case.
+    fun scan_active_permission_granted_cellular_only_then_banner_shows() {
+        // The auto-scan case: scan running in the background with cellular-only transport. Chip
+        // visibility no longer gates this surface — `isNetworkScanning` is the only activity
+        // signal — so the banner surfaces the missing-transport hint to the user.
         assertTrue(
             shouldShowWifiUnavailableBanner(
-                showNetworkTransport = false,
                 isNetworkScanning = true,
                 localNetworkPermissionGranted = true,
                 wifiUnavailable = !anyNetworkScanTransportAvailable(transports()),
+                discoveredTcpDevicesEmpty = true,
             ),
         )
     }
 
     @Test
-    fun section_hidden_and_scan_inactive_then_banner_hidden() {
-        // User is not interacting with network discovery at all — banner suppressed even on cellular-only.
-        assertFalse(
+    fun scan_active_permission_granted_wifi_unavailable_then_banner_shows() {
+        // Clean positive case: scan actively running, permission granted, WiFi unavailable, no
+        // discovered nodes yet — banner fires. This is the only combination the banner is meant
+        // to surface.
+        assertTrue(
             shouldShowWifiUnavailableBanner(
-                showNetworkTransport = false,
-                isNetworkScanning = false,
+                isNetworkScanning = true,
                 localNetworkPermissionGranted = true,
-                wifiUnavailable = !anyNetworkScanTransportAvailable(transports()),
+                wifiUnavailable = true,
+                discoveredTcpDevicesEmpty = true,
             ),
         )
     }
@@ -231,10 +237,10 @@ class WifiUnavailableBannerTest {
         // Permission-recovery flow on the scan toggle owns this surface; banner must not overlap.
         assertFalse(
             shouldShowWifiUnavailableBanner(
-                showNetworkTransport = true,
                 isNetworkScanning = true,
                 localNetworkPermissionGranted = false,
                 wifiUnavailable = !anyNetworkScanTransportAvailable(transports()),
+                discoveredTcpDevicesEmpty = true,
             ),
         )
     }
@@ -244,10 +250,10 @@ class WifiUnavailableBannerTest {
         // VPN off + Wi-Fi on + scanning → no banner (preserves the existing "transport available" outcome).
         assertFalse(
             shouldShowWifiUnavailableBanner(
-                showNetworkTransport = true,
                 isNetworkScanning = true,
                 localNetworkPermissionGranted = true,
                 wifiUnavailable = !anyNetworkScanTransportAvailable(transports(wifi = true)),
+                discoveredTcpDevicesEmpty = true,
             ),
         )
     }
@@ -258,10 +264,25 @@ class WifiUnavailableBannerTest {
         val vpnOnly = listOf(NetworkTransportInfo(hasWifi = false, hasEthernet = false, hasVpn = true))
         assertFalse(
             shouldShowWifiUnavailableBanner(
-                showNetworkTransport = true,
                 isNetworkScanning = true,
                 localNetworkPermissionGranted = true,
                 wifiUnavailable = !anyNetworkScanTransportAvailable(vpnOnly),
+                discoveredTcpDevicesEmpty = true,
+            ),
+        )
+    }
+
+    @Test
+    fun scan_active_permission_granted_wifi_unavailable_results_found_then_banner_hidden() {
+        // Once the live scan has produced discovered TCP nodes, the recovery hint is no longer
+        // useful — the user has found what they were looking for. Banner stays hidden even though
+        // scan is active, permission is granted, and WiFi is reported unavailable.
+        assertFalse(
+            shouldShowWifiUnavailableBanner(
+                isNetworkScanning = true,
+                localNetworkPermissionGranted = true,
+                wifiUnavailable = true,
+                discoveredTcpDevicesEmpty = false,
             ),
         )
     }
diff --git a/feature/connections/src/commonMain/kotlin/org/meshtastic/feature/connections/ui/ConnectionsScreen.kt b/feature/connections/src/commonMain/kotlin/org/meshtastic/feature/connections/ui/ConnectionsScreen.kt
index dac08e1c4..d05fb22fb 100644
--- a/feature/connections/src/commonMain/kotlin/org/meshtastic/feature/connections/ui/ConnectionsScreen.kt
+++ b/feature/connections/src/commonMain/kotlin/org/meshtastic/feature/connections/ui/ConnectionsScreen.kt
@@ -300,9 +300,9 @@ fun ConnectionsScreen(
 
                         // Adapter-off hints: shown only when the relevant permission is granted but the radio/network
                         // is unavailable, so they don't overlap the permission-recovery flow on the scan toggles.
-                        // The Wi-Fi banner gate includes `isNetworkScanning` because `LifecycleStartEffect` keys the
-                        // auto-scan off `permission status`, not the section-visibility chip — a user with the Network
-                        // filter off but auto-scan on still has a running scan that needs the hint.
+                        // The WiFi-unavailable banner only renders while a network scan is actively running —
+                        // discovery is the only moment the user needs to know WiFi is missing. The auto-scan case is
+                        // covered because `isNetworkScanning` is true during auto-scan regardless of chip state.
                         if (showBleTransport && bluetoothPermission.isGranted && bluetoothDisabled) {
                             RecoveryCard(
                                 message = stringResource(Res.string.bluetooth_disabled),
@@ -313,10 +313,10 @@ fun ConnectionsScreen(
                         }
                         if (
                             shouldShowWifiUnavailableBanner(
-                                showNetworkTransport = showNetworkTransport,
                                 isNetworkScanning = isNetworkScanning,
                                 localNetworkPermissionGranted = localNetworkPermission.isGranted,
                                 wifiUnavailable = wifiUnavailable,
+                                discoveredTcpDevicesEmpty = discoveredTcpDevices.isEmpty(),
                             )
                         ) {
                             RecoveryCard(