Skip to content

fix(map): show direct-heard SNR in node hover tooltip (#3925)#3929

Merged
Yeraze merged 1 commit into
mainfrom
fix/3925-map-hover-snr
Jul 5, 2026
Merged

fix(map): show direct-heard SNR in node hover tooltip (#3925)#3929
Yeraze merged 1 commit into
mainfrom
fix/3925-map-hover-snr

Conversation

@Yeraze

@Yeraze Yeraze commented Jul 5, 2026

Copy link
Copy Markdown
Owner

Summary

Fixes #3925. The map node-circle hover tooltip never displayed SNR, even when a node was heard directly (0 hops) and an SNR value was available. It only showed the node name and hop count.

This was a different code path from the position-history point tooltip (fixed in #3590) and inconsistent with the Nodes list card view, which both already show direct-heard SNR.

Changes

  • src/utils/nodeHops.ts — new pure helper getMapHoverTooltipMeta(effectiveHops, snr) returning { hops, showSnr, snr }. SNR is shown only when effectiveHops === 0 && snr != null, mirroring the Nodes card view and the position-history tooltip.
  • src/components/NodesTab.tsx — the map marker hover <Tooltip> now uses the helper and renders a N hops · 📶 X.XdB line for directly-heard nodes with known SNR. Unknown-hop nodes with no SNR still render nothing (unchanged behavior).
  • src/utils/nodeHops.test.ts — new unit tests for the helper: 0 hops with SNR, SNR of exactly 0, null/undefined SNR, multi-hop (SNR hidden), and unknown hops.

Behavior

Node state Before After
0 hops, SNR known 0 hops 0 hops · 📶 X.XdB
0 hops, SNR null 0 hops 0 hops
N hops (>0) N hops N hops (unchanged)
unknown hops, no SNR (nothing) (nothing)

Testing

  • New nodeHops.test.ts: 7 tests, all pass.
  • Full Vitest suite: 7948 passed, 0 failed, 0 suite failures (8570 total including skipped).

🤖 Generated with Claude Code

https://claude.ai/code/session_01AphLfgUJWaFNAgU5UXdJ4n

The map node-circle hover tooltip only rendered the node name and hop
count; it never surfaced SNR, even when the node was heard directly
(0 hops) and an SNR value was available. This was inconsistent with:

- the Nodes list card view (`hopsAway === 0 && snr != null` -> 📶 X.XdB)
- the position-history point tooltip fixed under #3590

Extract a pure `getMapHoverTooltipMeta(effectiveHops, snr)` helper in
`nodeHops.ts` that decides whether to show the direct-heard SNR, and use
it in the map hover tooltip so a "N hops · 📶 X.XdB" line is rendered
when the node is heard directly and SNR is known.

Adds `nodeHops.test.ts` covering the show/hide conditions (0 hops with
SNR, SNR of 0, null/undefined SNR, multi-hop, unknown hops).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AphLfgUJWaFNAgU5UXdJ4n
@github-actions

github-actions Bot commented Jul 5, 2026

Copy link
Copy Markdown

Claude finished @Yeraze's task —— View job


PR Review: fix(map): show direct-heard SNR in node hover tooltip (#3925)

Tasks:

  • Gather context and read changed files
  • Review code quality and best practices
  • Check for potential bugs
  • Evaluate test coverage
  • Post final review

Overall this is a clean, well-scoped fix. A few observations:

Code Quality

src/utils/nodeHops.ts

The helper is well-factored — pure function, narrow interface, good JSDoc. One minor style note: line 105 uses a non-null assertion (snr!) after the showSnr guard, which is safe but slightly redundant. TypeScript can't narrow through the object literal pattern here, so the ! is required, but it's worth a brief comment to avoid future confusion. Not blocking.

// line 105
return { hops, showSnr, snr: showSnr ? snr! : null };
// `snr!` is safe here because showSnr === true implies snr != null

src/components/NodesTab.tsx (lines 2285–2301)

The IIFE pattern {(() => { ... })()} works but is a bit heavy for what's essentially a conditional render. The pre-existing code already uses this pattern in this file, so staying consistent is the right call. No change needed.

Line 2296 uses snr!.toFixed(1) — the ! assertion is guarded by showSnr, which comes from getMapHoverTooltipMeta returning snr: null when showSnr is false. This is safe, but since snr is typed as number | null from the destructured meta, a TypeScript-safer alternative would be snr?.toFixed(1) (though it would never be undefined here). Not a bug, just a minor observation.

Potential Bugs

None found. The snr != null check (line 104 of nodeHops.ts) correctly handles both null and undefined, including the falsy-but-valid case of SNR === 0 — which is explicitly tested. The early-exit condition at line 2288 (if (hops === null && !showSnr) return null) correctly preserves the prior behavior of rendering nothing for unknown-hop nodes.

One edge case to be aware of (not introduced by this PR): node.snr in DeviceInfo is typed as snr?: number, meaning it's only populated when the node was heard directly by the receiving node. Multi-hop nodes may still have a stale snr value in the DB from a prior direct-heard event. The guard effectiveHops === 0 correctly prevents showing a stale SNR for multi-hop nodes, so this is handled properly.

Performance

No concerns. getMapHoverTooltipMeta is called inside the tooltip render (only when the user hovers a marker) and does O(1) work.

Security

No concerns.

Test Coverage

The 7 tests in nodeHops.test.ts cover the important cases well:

  • ✅ 0 hops + SNR known (positive SNR)
  • ✅ 0 hops + SNR = 0 (falsy but valid)
  • ✅ 0 hops + SNR null
  • ✅ 0 hops + SNR undefined
  • ✅ multi-hop with SNR present (SNR hidden)
  • ✅ unknown hops (999) with SNR
  • ✅ known multi-hop, no SNR

One case not tested: effectiveHops > 999 (e.g. 1000). The current implementation treats only >= 999 as unknown (effectiveHops < 999 ? effectiveHops : null), so 1000 would be treated as unknown hops too. This matches the sentinel convention in the codebase (999 = unknown). A test for > 999 would make this explicit, but it's a low-priority gap given the sentinel is already tested.

The component-level rendering (NodesTab.tsx) lacks a snapshot/integration test for the tooltip HTML, but that's consistent with the rest of the test suite and isn't a blocker.

Summary

Solid fix with good test coverage and clean separation of concerns. The new pure helper is reusable and well-documented. The behavior table in the PR description matches the implementation. Approved.

@Yeraze Yeraze merged commit 95b7573 into main Jul 5, 2026
19 checks passed
@Yeraze Yeraze deleted the fix/3925-map-hover-snr branch July 5, 2026 16:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Map node hover tooltip: no SNR shown for directly-heard nodes (0 hops)

1 participant