Skip to content

fix: Show UDP/RF filters now hide neighbor-info lines (closes #3147)#3148

Merged
Yeraze merged 1 commit into
mainfrom
claude/great-dijkstra-trkPu
May 23, 2026
Merged

fix: Show UDP/RF filters now hide neighbor-info lines (closes #3147)#3148
Yeraze merged 1 commit into
mainfrom
claude/great-dijkstra-trkPu

Conversation

@Yeraze
Copy link
Copy Markdown
Owner

@Yeraze Yeraze commented May 23, 2026

Summary

  • Root cause: visibleNodeNums in App.tsx — the set used to suppress neighbor-info connection lines when a node is hidden — only applied the MQTT filter (via the legacy node.viaMqtt boolean). The RF and UDP filters were absent, so unchecking "Show UDP" hid node markers correctly but left their connection lines visible.
  • Fix: Replace the partial MQTT-only check with nodePassesTransportFilter (the same helper already used by the marker renderer in NodesTab), add showRfNodes / showUdpNodes to the useMapContext() destructuring in App.tsx, and add both to the useMemo dependency array so the set updates reactively.

Changed file: src/App.tsx — 8 insertions, 10 deletions.

Test plan

  • Start MeshMonitor with a source that has a mix of RF, UDP-bridged, and MQTT-bridged nodes visible on the map with neighbor-info lines enabled.
  • Uncheck Show UDP → UDP node markers AND their neighbor-info connection lines should disappear.
  • Re-check Show UDP → nodes and lines return.
  • Repeat for Show MQTT and Show RF to confirm no regression.
  • Confirm traceroute path segments also respect the filter (they share the same visibleNodeNums set).

https://claude.ai/code/session_01DyRSp5mPeD4qyG2gNoiS3o


Generated by Claude Code

visibleNodeNums (used to hide neighbor-info lines when endpoint nodes
are filtered) previously only checked the MQTT filter via the legacy
`node.viaMqtt` boolean, leaving RF and UDP unchecked. This meant that
unchecking "Show UDP" hid the node markers but left their neighbor-info
connection lines visible, making filtered nodes appear to still be on
the map (closes #3147).

Switch to `nodePassesTransportFilter` (same helper used by the marker
renderer) and add `showRfNodes`/`showUdpNodes` to both the
`useMapContext()` destructuring and the `useMemo` dependency array.

https://claude.ai/code/session_01DyRSp5mPeD4qyG2gNoiS3o
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 23, 2026

Claude finished @Yeraze's task —— View job


PR Review in Progress

Reviewing this pull request for:

  • Code quality and best practices
  • Potential bugs or issues
  • Performance considerations
  • Security concerns
  • Test coverage

Review Summary

✅ Excellent Fix - Well Implemented

This PR correctly addresses issue #3147 by fixing the incomplete transport filtering logic for neighbor-info connection lines and traceroute paths.

Code Quality & Best Practices ⭐

Strengths:

  • Perfect reuse of existing utility: Uses nodePassesTransportFilter from src/utils/nodeTransport.ts:48 - the exact same helper already used by NodesTab.tsx for marker filtering
  • Consistent pattern: Now mirrors the filtering logic used in NodesTab.tsx exactly, ensuring UI consistency
  • Clean dependency management: Properly adds showRfNodes, showUdpNodes to the useMemo dependency array so visibleNodeNums updates reactively
  • Improved comments: Better explains the purpose - "Must mirror the per-marker filter in NodesTab so that lines are hidden whenever their endpoint nodes are hidden"
  • Simplified logic: Removes the manual node.viaMqtt check and consolidates all transport filtering into one helper call

Bug Analysis ✅

Root Cause Correctly Identified:
The original code at src/App.tsx:4467 only checked:

if (!showMqttNodes && node.viaMqtt) return false;

This missed RF and UDP filtering, causing connection lines to remain visible when UDP/RF nodes were hidden via the map toggles.

Fix Implementation:

if (!nodePassesTransportFilter(node, { showRfNodes, showUdpNodes, showMqttNodes })) return false;

This correctly applies all three transport filters using the same logic as marker rendering.

Performance Considerations ✅

  • No performance regression: The nodePassesTransportFilter helper is designed to be "small/inlined" for use in large .filter() chains
  • Efficient implementation: Uses switch statement and early returns in classifyNodeTransport
  • Proper memoization: Dependencies are correctly specified for the useMemo hook

Security Concerns ✅

  • No security issues: This is purely a frontend display logic fix with no security implications
  • Safe imports: Only imports existing, tested utilities

Test Coverage 📋

Current Test Coverage:

  • nodePassesTransportFilter has comprehensive unit tests in src/utils/nodeTransport.test.ts
  • ✅ The utility function is well-tested for all transport types (RF, UDP, MQTT)

Recommended Additional Testing:

  • Integration test for visibleNodeNums computation with different transport filter combinations
  • E2E test following the test plan in the PR description to verify neighbor-info lines hide/show correctly

Minor Observations

  1. Import organization: The new import is properly placed alphabetically
  2. Type safety: Uses the existing well-typed interfaces from the nodeTransport utility
  3. Code reduction: Net -2 lines while adding functionality shows good refactoring

Verdict: ✅ APPROVE

This is a high-quality fix that:

  • Correctly addresses the stated issue
  • Reuses existing, tested code patterns
  • Maintains consistency with marker filtering logic
  • Has no negative side effects
  • Follows established coding patterns in the codebase

The fix should resolve the issue where UDP/RF transport filter toggles didn't hide neighbor-info connection lines.


@Yeraze Yeraze merged commit 7f13d1f into main May 23, 2026
19 checks passed
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.

2 participants