Skip to content

fix(ui): show Wi-Fi unavailable banner only during active network scan#5892

Merged
jamesarich merged 2 commits into
meshtastic:mainfrom
jeremiah-k:bugfix/wifi-banner-scan-only
Jun 21, 2026
Merged

fix(ui): show Wi-Fi unavailable banner only during active network scan#5892
jamesarich merged 2 commits into
meshtastic:mainfrom
jeremiah-k:bugfix/wifi-banner-scan-only

Conversation

@jeremiah-k

@jeremiah-k jeremiah-k commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

Overview

This PR fixes Connections screen Wi-Fi recovery banner behavior so the banner appears only when it is useful: during an active network scan, after local-network permission has been granted, when no Wi-Fi/VPN/Ethernet scan transport is available, and before any TCP devices have been discovered. This prevents the banner from appearing just because the Network section is visible, and hides it once discovery has produced usable results.

This PR is related to the scan lifecycle coordination PR (#5887) and should land after it. That PR makes isNetworkScanning a more reliable runtime signal; this PR then uses that signal as the banner’s user-intent gate. Because both PRs touch nearby ConnectionsScreen.kt code, this branch may need a small rebase after the scan lifecycle PR lands.

Key Changes

Fixes

  • Banner no longer displays when no network scan is actively running, eliminating false-positive idle states.
  • Banner automatically hides once discovered TCP devices appear in the list.
  • Auto-scan scenarios still show the banner when appropriate because the gate uses isNetworkScanning, not the Network section visibility chip.
  • Permission-denied, Wi-Fi-available, and VPN-available states continue to suppress the banner.

Refactors

  • Updated shouldShowWifiUnavailableBanner() to use scan/result state instead of transport chip visibility:
    • Removed showNetworkTransport.
    • Added isNetworkScanning.
    • Added discoveredTcpDevicesEmpty.
    • Retained localNetworkPermissionGranted and wifiUnavailable.
  • Simplified the ConnectionsScreen call site to pass active scan state, permission state, transport availability, and whether the discovered TCP list is empty.
  • Removed dependence on the Network section visibility chip, which defaults to visible and is not a reliable signal of user discovery intent.
  • Updated predicate documentation to describe the recovery-hint behavior and result-based suppression.

Testing

  • Expanded common test coverage for:
    • scan inactive + cellular-only transport => banner hidden.
    • scan active + cellular-only transport => banner shown.
    • scan active + permission denied => banner hidden.
    • scan active + Wi-Fi available => banner hidden.
    • scan active + VPN available => banner hidden.
    • scan active + Wi-Fi unavailable + TCP results found => banner hidden.
  • Static call-site review confirms the old showNetworkTransport predicate argument is removed from touched files.

Breaking Changes

The internal shouldShowWifiUnavailableBanner() helper signature changed.

Removed:

  • showNetworkTransport: Boolean

Added:

  • isNetworkScanning: Boolean
  • discoveredTcpDevicesEmpty: Boolean

Retained:

  • localNetworkPermissionGranted: Boolean
  • wifiUnavailable: Boolean

No user-facing migration is required.

@github-actions github-actions Bot added the bugfix PR tag label Jun 21, 2026
@github-actions

github-actions Bot commented Jun 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

📄 Docs staleness check — advisory

This PR modifies user-facing UI source files but does not update any page under docs/en/user/ or docs/en/developer/.

⚠️ Doc changes propagate to 3 consumers: in-app docs browser, Jekyll site (GitHub Pages), and meshtastic.org (Docusaurus sync). Updating a page in docs/en/ automatically flows to all three.

Changed source files:

core/ui/src/commonMain/kotlin/org/meshtastic/core/ui/util/NetworkTransportInfo.kt
feature/connections/src/commonMain/kotlin/org/meshtastic/feature/connections/ui/ConnectionsScreen.kt

What to check:

Changed area Likely doc page
feature/messaging/ docs/en/user/messages-and-channels.md
feature/node/ docs/en/user/nodes.md or docs/en/user/node-metrics.md
feature/map/ docs/en/user/map-and-waypoints.md
feature/connections/ docs/en/user/connections.md
feature/settings/ docs/en/user/settings-radio-user.md or docs/en/user/settings-module-admin.md
feature/firmware/ docs/en/user/firmware.md
feature/intro/ docs/en/user/onboarding.md
feature/discovery/ docs/en/user/discovery.md
feature/docs/ Internal docs infrastructure
core/ui/ docs/en/developer/codebase.md or component-specific user pages

New page checklist (if adding a new doc page):

  1. Create the .md file in docs/en/user/ or docs/en/developer/ with last_updated frontmatter
  2. Register in DocBundleLoader.kt with string resources (in-app browser)
  3. Jekyll and Docusaurus sync pick up new pages automatically — no config change needed

If this PR does not require a doc update (e.g., internal refactor, bug fix, test change), add the skip-docs-check label to dismiss this check.

Cross-platform note: This check is advisory while doc coverage matures. Both Android and Apple repos use the same skip-docs-check label and advisory severity. See meshtastic/design standards for shared conventions.

@github-actions

github-actions Bot commented Jun 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🖼️ Preview staleness check — advisory

This PR modifies UI composables but does not update any *Previews.kt files.

Previews power screenshot tests and in-app docs screenshots. Keeping them current ensures visual regression coverage stays accurate.

Changed UI files:

feature/connections/src/commonMain/kotlin/org/meshtastic/feature/connections/ui/ConnectionsScreen.kt

What to check:

Pattern Preview file convention
feature/{name}/…/ui/ or component/ feature/{name}/…/*Previews.kt
core/ui/…/ core/ui/…/ (previews colocated)

Adding previews checklist:

  1. Create or update a *Previews.kt file in the same module with @PreviewLightDark
  2. Add @Suppress("PreviewPublic") if the preview is consumed by screenshot-tests
  3. Add corresponding @PreviewTest function in screenshot-tests/src/screenshotTest/
  4. Run ./gradlew :screenshot-tests:updateDebugScreenshotTest to generate reference images

If this PR does not require preview updates (e.g., logic-only change, non-visual refactor), add the skip-preview-check label to dismiss.

This was referenced Jun 21, 2026
Merged
Closed
@jeremiah-k jeremiah-k force-pushed the bugfix/wifi-banner-scan-only branch from cce6db6 to 42dbe8e Compare June 21, 2026 22:55
@jeremiah-k
fix(ui): only show WiFi-unavailable banner while network scan is active
99e1582 
The banner predicate previously OR'd `showNetworkTransport` (a default-on
user preference for the Network chip) with `isNetworkScanning`, so it
fired whenever the chip was enabled — even with no scan running. Drop
the chip term so the banner only renders while a scan is actively
running.

The auto-scan case (chip off but auto-scan pref on) is still covered
because `isNetworkScanning` is true during auto-scan regardless of chip
state.

The unused `showNetworkTransport` parameter is removed from the
predicate signature and its single call site.

Tests: the prior scenario asserting "section visible plus scan idle
yields banner shown" is flipped to assert the banner is hidden, pinning
the reported bug. A new positive case
(scan_active_permission_granted_wifi_unavailable_then_banner_shows)
locks in the desired behavior. The remaining existing scenarios retain
their semantics under the narrower predicate.
@jeremiah-k
fix(ui): hide WiFi-unavailable banner once network scan finds results
0d9c080 
The WiFi-unavailable recovery banner in ConnectionsScreen is a hint for the
case where the user has started a network scan but the device is on a
transport that cannot carry NSD/mDNS traffic to a Meshtastic node
(cellular-only, no Wi-Fi/Ethernet/VPN). It points the user at Wi-Fi
settings as the recovery path.

Once the live scan has actually surfaced discovered TCP nodes, the user
has already found what the hint was trying to help them reach. Keeping
the banner visible past that point is noise — it pulls attention away
from the freshly populated device list and implies recovery is still
needed when, from the user's intent, it is not.

Extend the `shouldShowWifiUnavailableBanner` predicate with a fourth
AND-term, `discoveredTcpDevicesEmpty`, sourced from the already-collected
`discoveredTcpDevices` state. The banner now requires all four conditions:
scan active, local-network permission granted, WiFi unavailable, and the
discovered-TCP list still empty. Naming mirrors the negative phrasing of
`wifiUnavailable` so the predicate reads symmetrically.

The predicate stays a pure boolean function with no coupling to
feature-specific types, so existing commonTest coverage extends cleanly:
the six prior WifiUnavailableBannerTest cases gain the new argument
pinned to `true` (preserving the empty-list banner-shows behavior), and
one new case pins the results-found suppression path.
@jeremiah-k jeremiah-k force-pushed the bugfix/wifi-banner-scan-only branch from 42dbe8e to 0d9c080 Compare June 21, 2026 23:04
@jeremiah-k jeremiah-k marked this pull request as ready for review June 21, 2026 23:27
@jamesarich jamesarich added this pull request to the merge queue Jun 21, 2026
Merged via the queue into meshtastic:main with commit 419d3ec Jun 21, 2026
22 checks passed
@jeremiah-k jeremiah-k deleted the bugfix/wifi-banner-scan-only branch June 22, 2026 00:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jamesarich jamesarich jamesarich approved these changes

Assignees

No one assigned

Labels

bugfix PR tag

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jeremiah-k @jamesarich