feat(map): My Location toggle, user-tracking button & distance-based cluster fan-out#2132
Conversation
…luster fan-out Recovers uncommitted follow-up work to the MKMapView map (#1989) and rebases it onto current main. - Add a "My Location" toggle (persisted `enableMapUserLocation`) to Map settings. - MeshMapMK: bind `showUserLocation` to that setting and gate the blue dot on `isMapVisible` so it turns off while the tab is off-screen/backgrounded. - ClusterMapView: add a native MKUserTrackingButton (follow/center), hidden when the user-location dot is off; floor the cluster zoom span (~140 m) so a tight "2" cluster still breaks apart to a readable street level. - MeshMapMK: replace grid-quantized spread grouping with distance-based union-find (16 m) so stacked pins straddling a grid cell or jittering a few meters apart are reliably fanned out into individual tappable circles. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe map adds a persisted “My Location” setting, integrates MapKit’s native tracking button, improves tight-cluster zoom behavior, and replaces coordinate quantization with distance-based node grouping and centroid fan-out. ChangesMap behavior
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant MapSettingsForm
participant MeshMapMK
participant ClusterMapView
User->>MapSettingsForm: Toggle My Location
MapSettingsForm->>MeshMapMK: Update persisted setting
MeshMapMK->>ClusterMapView: Apply location visibility configuration
ClusterMapView->>User: Show or hide tracking control
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Meshtastic/Views/Nodes/Helpers/Map/ClusterMapView.swift`:
- Around line 504-522: Update gestureRecognizer(_:shouldReceive:) to walk from
the touched view up to the MKMapView and reject touches on either
MKAnnotationView or any UIControl, including the tracking, pitch, and compass
buttons. Preserve accepting touches that reach the map without matching those
excluded view types.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e9940bcd-a6c9-4c07-af2e-b0f33d899b02
📒 Files selected for processing (4)
Localizable.xcstringsMeshtastic/Views/Nodes/Helpers/Map/ClusterMapView.swiftMeshtastic/Views/Nodes/Helpers/Map/MapSettingsForm.swiftMeshtastic/Views/Nodes/MeshMapMK.swift
| // Native follow/center control — MapKit owns the user-location camera behavior. Styled to | ||
| // match the pitch button; hidden while the user-location dot is off (see applyConfiguration). | ||
| let tracking = MKUserTrackingButton(mapView: mapView) | ||
| tracking.tintColor = .label | ||
| tracking.backgroundColor = UIColor.tertiarySystemBackground.withAlphaComponent(0.92) | ||
| tracking.layer.cornerRadius = 8 | ||
| tracking.layer.shadowColor = UIColor.black.cgColor | ||
| tracking.layer.shadowOpacity = 0.2 | ||
| tracking.layer.shadowRadius = 2 | ||
| tracking.layer.shadowOffset = CGSize(width: 0, height: 1) | ||
| tracking.translatesAutoresizingMaskIntoConstraints = false | ||
| tracking.isHidden = !(appliedConfiguration?.showsUserLocation ?? true) | ||
| NSLayoutConstraint.activate([ | ||
| tracking.widthAnchor.constraint(equalToConstant: 44), | ||
| tracking.heightAnchor.constraint(equalToConstant: 44) | ||
| ]) | ||
| userTrackingButton = tracking | ||
|
|
||
| let stack = UIStackView(arrangedSubviews: [tracking, pitch, compass]) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
New tracking button sits inside the waypoint-creation gesture zone — long-press can create an unwanted waypoint.
gestureRecognizer(_:shouldReceive:) (below, unchanged) only rejects touches on MKAnnotationView; it doesn't reject touches on this new MKUserTrackingButton (or the existing pitch button). Combined with shouldRecognizeSimultaneouslyWith always returning true, a long-press directly on the tracking button will also fire handleMapLongPress → onMapLongPress → beginNewWaypoint, creating a waypoint under the button.
🐛 Suggested fix — reject touches on any control, not just annotation views (applies at the `gestureRecognizer(_:shouldReceive:)` implementation, ~line 812)
func gestureRecognizer(_ gestureRecognizer: UIGestureRecognizer, shouldReceive touch: UITouch) -> Bool {
guard let mapView = gestureRecognizer.view as? MKMapView else { return true }
var hit = mapView.hitTest(touch.location(in: mapView), with: nil)
while let view = hit, view !== mapView {
if view is MKAnnotationView || view is UIControl { return false }
hit = view.superview
}
return true
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Meshtastic/Views/Nodes/Helpers/Map/ClusterMapView.swift` around lines 504 -
522, Update gestureRecognizer(_:shouldReceive:) to walk from the touched view up
to the MKMapView and reject touches on either MKAnnotationView or any UIControl,
including the tracking, pitch, and compass buttons. Preserve accepting touches
that reach the map without matching those excluded view types.
📄 Docs staleness warningThis PR modifies user-facing Swift source files but does not update any page under Changed source files: What to check:
If this PR does not require a doc update (e.g., internal refactor, bug fix, test change), add the After updating |
1 participant
What & why
Recovers uncommitted follow-up work to the MKMapView map (#1989) that was sitting only in a local working tree on a now-stale branch, and rebases it cleanly onto current
main.My Location control
enableMapUserLocation).MeshMapMKbindsshowUserLocationto that setting and gates the blue dot onisMapVisible, so the user-location dot turns off while the Map tab is off-screen or the app is backgrounded.ClusterMapViewadds a nativeMKUserTrackingButton(follow / center-on-me), styled to match the existing pitch/compass controls and hidden when the dot is off so it isn't a dead button.Cluster fan-out fix
ClusterMapView: floor the cluster zoom span (~140 m) so a tight"2"cluster still zooms to a readable street level instead of an imperceptible nudge.MeshMapMK: replace grid-quantized spread grouping with distance-based union-find (16 m). A fixed grid missed two real cases that produced un-splittable "2" clusters — pairs straddling a cell boundary, and same-site radios whose GPS fixes jitter a few meters apart. Union-find over a coarse neighbor grid keeps it ~O(n) on the density-limited snapshot set.Notes
mainsince this work was branched) and the new "My Location" toggle both live in the same settings group — both preserved.Meshtasticscheme embeds a Watch app requiring the watchOS 26.5 simulator runtime (not installed here), and the repo has no build/test CI. Verified via SwiftLint (clean; the twotype_body_lengthwarnings pre-exist onmain) + symbol-existence checks againstmain's current files. A quick on-device pass of the toggle + tracking button + cluster splitting is worth doing.🤖 Generated with Claude Code
Summary by CodeRabbit