fix(node): scroll traceroute/log history detail popups

[beecho01]

Fixes #6701.

Why

A traceroute with many hops scrolls fine in the live/real-time result view, but the same route reopened from history is cut off with no way to see the rest.

Root cause: the shared popup component MeshtasticDialog (core/ui/.../AlertDialogs.kt) only wraps its content in a scrollable column when the dialog has choices (a button list). For a plain text/composableMessage dialog, scrolling is the caller's own responsibility.

• The live traceroute view (TracerouteAlertHandler.kt) already does this correctly - wraps its Text in Column(Modifier.verticalScroll(rememberScrollState())).
• MetricsViewModel.showTracerouteDetail (the history case) did not - just a bare SelectionContainer { Text(...) } - so long content clips inside the dialog's bounded height with no way to reach the rest.
• MetricsViewModel.showLogDetail (used for other detail popups, e.g. neighbor-info logs) had the exact same missing-modifier pattern, one-for-one. Not reported in the issue, but it's the identical mistake in the same file, so it's fixed in the same PR rather than left for someone to report separately.

What changed

• showLogDetail and showTracerouteDetail (feature/node/.../MetricsViewModel.kt) now wrap their content in Column(Modifier.verticalScroll(rememberScrollState())), matching the working TracerouteAlertHandler.kt pattern exactly.
• No changes to MeshtasticDialog/AlertDialogs.kt itself - a shared-component-level fix (always-scrollable text content regardless of choices) was considered but deliberately left out of scope here, since it touches a component used by every dialog in the app and deserves its own separately-reviewed PR.

Test plan

• New test: feature/node/src/jvmTest/.../DetailDialogScrollTest.kt. A static screenshot can't catch this regression - a single frame of "scrolled to top, more content below" looks identical whether or not scrolling actually works. Used a behavioral test instead: performScrollTo() throws when the target node has no scrollable ancestor, so it fails exactly when the verticalScroll wrapper is missing. Confirmed by hand: temporarily reverted the test to the pre-fix (unwrapped) shape and watched it fail with the expected AssertionError, then restored it.
• Local baseline green: spotlessApply spotlessCheck detekt assembleDebug test allTests and kmpSmokeCompile.

Not yet verified on real hardware. I have the fdroid debug build with this fix installed on a Samsung Galaxy S24+, but no nearby node has responded to a traceroute request yet to produce a fresh multi-hop result to test against. Will update here once verified against an actual long route on-device; happy to hold this in draft until then.

Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com

[coderabbitai]

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 2feebbb0-cae7-42ee-b0cf-77c3e650841a

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[jamesarich]

Fix itself looks right, and matching TracerouteAlertHandler's shape exactly is the correct instinct. Catching that showLogDetail had the identical bug and fixing both was a good call, as was leaving MeshtasticDialog alone for a separate PR.

One thing on the test, and I think it's worth sorting before this comes out of draft.

DetailDialogScrollTest builds the fixed shape inline in the test body and then asserts that shape scrolls. It never calls showLogDetail or showTracerouteDetail, so what it actually proves is that a Column(Modifier.verticalScroll(...)) inside MeshtasticDialog scrolls, which is a fact about Compose rather than about our code. If someone reverts the change in MetricsViewModel.kt tomorrow, this test still goes green.

I think that's also what the manual check was telling you: reverting the test to the unwrapped shape and watching it fail confirms the test notices its own copy of the shape, not that the app has it.

The good news is AlertManager makes the real version cheap. It's a plain open class and currentAlert is a StateFlow<AlertData?>, so you can drive the production path directly:

val alertManager = AlertManager()
// ...construct the VM with it, then:
viewModel.showLogDetail(titleRes = Res.string.traceroute, annotatedMessage = longRoute)
val message = alertManager.currentAlert.value?.composableMessage
assertNotNull(message)
setContent { AppTheme { MeshtasticDialog(title = "Traceroute", text = message, onDismiss = {}) } }
onNodeWithText(LAST_HOP_MARKER, substring = true).performScrollTo().assertIsDisplayed()

showLogDetail is the easy one to pin since it has no repository dependency, and it carries the identical shape, so covering just that would satisfy me. showTracerouteDetail needs tracerouteSnapshotRepository stood up, so I would not insist on it if it turns into a fight.

No urgency on the hardware check. Holding in draft until a real multi-hop route comes back is exactly right, and worth noting the test shards have not run yet either since draft PRs skip them.