Skip to content

feat: custom marker clustering and viewport-aware overlay sync - #18

Merged
jkasprzyk17 merged 3 commits into
mainfrom
feat/marker-clustering-and-viewport-filter
Jun 26, 2026
Merged

feat: custom marker clustering and viewport-aware overlay sync#18
jkasprzyk17 merged 3 commits into
mainfrom
feat/marker-clustering-and-viewport-filter

Conversation

@jkasprzyk17

Copy link
Copy Markdown
Contributor

Summary

  • Replace platform-native clustering (MapKit clusteringIdentifier / Android ClusterManager) with a shared grid-based MarkerClusterEngine on iOS and Android — viewport-aware, computed off the main thread from descriptor data only.
  • Add MarkerViewportFilter and MarkerSpatialIndex so large marker sets only sync visible candidates to native, keeping per-frame map work bounded.
  • Refactor overlay collection: stable OverlayType identifiers (fixes monorepo module-path reference equality), improved useCollectedOverlays diffing, and dedicated native annotation/marker views (NitroPinAnnotationView, NitroClusterAnnotationView, ClusterIconFactory).
  • Expand the example app with a Poland stress-test scenario (thousands of markers, Gaussian city distribution) and Reanimated setup for the demo UI.

Native changes

Area iOS Android
Clustering MarkerClusterEngine, MapClusterAnnotation, NitroClusterAnnotationView MarkerClusterEngine, ClusterIconFactory, ClusterBadgeMetrics
Viewport culling MarkerViewportFilter, MarkerSpatialIndex same
Overlay controller diff-based annotation sync diff-based marker sync
Removed NitroClusterItem (maps-utils cluster manager)

JS / TS changes

  • useCollectedOverlays — more reliable child collection and native sync
  • overlayType.ts — stable overlay type tags on Marker, Polyline, Polygon, Circle
  • MapView / overlay components — wire-through for clustering and overlay metadata
  • docs/architecture.md — updated clustering docs

Other

  • .gitignore — ignore .cursor/
  • example/ — Reanimated + metro config, expanded App.tsx demo

Test plan

  • iOS: open example → Advanced Features → verify clusters form/split on zoom, tap cluster fires onClusterPress
  • iOS: markers with clusterable={false} always render individually
  • iOS: pan/zoom with 1k+ markers — map stays responsive, no annotation flicker
  • Android: same clustering + cluster press checks
  • Android: viewport filter — off-screen markers not rendered natively
  • Both: polyline / polygon / circle overlays still render and respond to onPress
  • bun run typecheck and bun run lint pass

Made with Cursor

@coderabbitai

coderabbitai Bot commented Jun 26, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 3078d6d6-d75a-4ab3-b486-b8c9f619a40f

📥 Commits

Reviewing files that changed from the base of the PR and between 0b20046 and e66ef91.

📒 Files selected for processing (2)
  • package/android/src/main/java/com/margelo/nitro/nitromaps/MarkerSpatialIndex.kt
  • package/ios/MarkerSpatialIndex.swift
🚧 Files skipped from review as they are similar to previous changes (2)
  • package/ios/MarkerSpatialIndex.swift
  • package/android/src/main/java/com/margelo/nitro/nitromaps/MarkerSpatialIndex.kt

📝 Walkthrough

Summary by CodeRabbit

  • New Features
    • Added custom, grid-based clustering with viewport-aware updates for large marker sets on both platforms.
    • Added bulk overlay props plus marker/overlay press and drag callbacks (markers, polylines, polygons, circles).
    • Improved built-in map scale appearance styling when enabled.
    • Refreshed the example app’s scenario/status UI with smoother, animated controls.
  • Bug Fixes
    • Improved marker/overlay interaction handling consistency across clustered and non-clustered modes (including marker drag end tagging).
  • Documentation
    • Updated advanced map/clustering docs to reflect the new custom grid clustering approach.

Walkthrough

The PR adds stable overlay tags and bulk overlay props/callbacks, then replaces native marker clustering with viewport-based clustering and custom rendering on Android and iOS. The example app, docs, CI, and runtime setup are updated to match.

Changes

Overlay collection and viewport clustering

Layer / File(s) Summary
Overlay contract
package/src/overlays/overlayType.ts, package/src/components/{Marker,Circle,Polyline,Polygon}.tsx, package/src/types/map.ts, docs/architecture.md, example/examples/types.ts
Defines stable overlay type tags, adds bulk overlay descriptor arrays and interaction callbacks to MapViewProps, and updates the architecture docs description of clusteringEnabled.
Overlay collection and dispatch
package/src/hooks/useCollectedOverlays.ts, package/src/components/MapView.tsx
Collects overlay children through typed collectors, keys callbacks by overlay type and id, and forwards bulk overlay callbacks through MapView to native props.
Android clustering
package/android/src/main/java/com/margelo/nitro/nitromaps/*.kt
Builds the Android viewport clustering pipeline, cached cluster badge rendering, marker indexing and filtering, diff application, and updated map view event routing.
iOS clustering
package/ios/*.swift
Builds the iOS grid clustering pipeline, custom annotation views, live refresh handling, scale appearance styling, and delegate-driven cluster selection and region updates.
Example app and setup
example/App.tsx, example/examples/advancedFeatures.ts, example/{index.js,metro.config.js,package.json}, .gitignore, .github/workflows/ci.yml
Updates the example app UI, scenario data, example runtime setup, package dependencies, CI codegen, and ignore rules.

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~90+ minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 21.97% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: custom clustering and viewport-aware overlay syncing.
Description check ✅ Passed The description is clearly aligned with the changeset and explains the clustering, viewport filtering, and overlay refactor work.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 14

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
example/App.tsx (1)

535-540: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Use the merged padding for fitToCoordinates.

MapView receives mapPadding, but the ready-time fit still uses scenario.advanced.mapPadding, so scale/compass padding added by mergeMapPadding can be ignored.

Proposed fix
       mapRef.current?.fitToCoordinates(
         scenario.markers.map((marker) => marker.coordinate),
-        scenario.advanced.mapPadding,
+        mapPadding,
         true,
       );
@@
-  }, [scenario]);
+  }, [mapPadding, scenario]);
🤖 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 `@example/App.tsx` around lines 535 - 540, The ready-time fit in the scenario
setup is using only scenario.advanced.mapPadding, so the extra padding from
mergeMapPadding is being skipped. Update the fitToCoordinates call in App.tsx to
use the same merged padding value that MapView receives, and keep the logic tied
to scenario.advanced?.fitToCoordinatesOnReady and the marker coordinate list so
both map layout and initial fit stay consistent.
package/ios/HybridMapViewDelegate.swift (1)

151-156: 🎯 Functional Correctness | 🟠 Major

Don’t deselect markers immediately after selection. canShowCallout is enabled for markers with a title/subtitle, and this call to deselectAnnotation(_:animated:) closes the callout right away, so the user never gets a chance to read it.

Proposed fix
     parent?.onMarkerPress?(marker.id)
-    mapView.deselectAnnotation(view.annotation, animated: true)
+    if marker.title == nil && marker.subtitle == nil {
+      mapView.deselectAnnotation(view.annotation, animated: 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 `@package/ios/HybridMapViewDelegate.swift` around lines 151 - 156, The marker
tap handler in HybridMapViewDelegate’s mapView(_:didSelect:) is immediately
closing the callout by calling deselectAnnotation(_:animated:) after
onMarkerPress. Remove that deselect call for MapMarkerAnnotation selections so
markers with canShowCallout stay open long enough for the title/subtitle to be
read. Keep the press callback on marker.id and only change the selection state
if there is a separate, intentional dismissal path elsewhere.
🤖 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 `@example/App.tsx`:
- Around line 495-499: Selecting a scenario in selectScenario currently resets
map readiness even when the same scenario is already active, which can leave
MapView stuck because onMapReady may not fire again. Update selectScenario in
App.tsx to no-op when the passed index matches the current scenarioIndex, and
only call setMapReady(false) and setStatus(MAP_SCENARIOS[index].name) when the
scenario actually changes.

In
`@package/android/src/main/java/com/margelo/nitro/nitromaps/ClusterIconFactory.kt`:
- Around line 67-69: The cluster count label formatting in formatCount should
use a fixed locale instead of the device default so the abbreviated count stays
consistent across locales. Update the String.format call inside
ClusterIconFactory.formatCount to pass an explicit locale such as Locale.US or
Locale.ROOT, and keep the existing count>=1000 logic unchanged.

In
`@package/android/src/main/java/com/margelo/nitro/nitromaps/MapOverlayController.kt`:
- Around line 145-162: The viewport diff logic in MapOverlayController’s
refresh/applyDiff path only tracks added and removed keys, so retained elements
with the same diffKey never refresh their native Marker or cluster state. Update
the diff building to keep a retained set alongside added/removed, and pass those
retained elements into applyDiff so existing markers are updated instead of
skipped. In applyDiff and the related retained-element handling, refresh single
markers with position/title/snippet/draggable, refresh clusters with
position/icon, and make sure clusterByKey[key] is reassigned for retained
clusters.
- Around line 127-139: The viewport recompute in the overlay refresh path can
run before the map has a valid size, causing clustering to use a 0x0 viewport
and collapse markers into one cluster. Update the refresh logic around
viewWidthPx, viewHeightPx, and the computeExecutor.execute block so it skips or
defers recomputation until both dimensions are greater than zero. Keep the
existing generation/candidate flow intact, but only call
MarkerClusterEngine.clusters and related merge/projection work once the map has
a real size.

In
`@package/android/src/main/java/com/margelo/nitro/nitromaps/MarkerClusterEngine.kt`:
- Around line 83-132: The clustering logic in MarkerClusterEngine currently uses
raw longitude values, which breaks anti-meridian cases and can produce incorrect
buckets, centroids, and bounds. Update the bucketing and cluster construction in
the loop that builds buckets and in the ClusterElement.Cluster creation path to
use wrapped longitude normalization instead of direct longitude math. Also
ensure mergeOverlapping() receives longitude-safe bounds handling so clusters
spanning 180/-180 are merged and bounded correctly.

In
`@package/android/src/main/java/com/margelo/nitro/nitromaps/MarkerSpatialIndex.kt`:
- Around line 64-85: The longitude handling in `MarkerSpatialIndex.candidates()`
does not account for anti-meridian wrapping, so bounds that cross ±180° can
produce an inverted `lonSpan` and empty results. Update the `candidates()` logic
to normalize wrapped longitudes or split the query into two longitude ranges
before computing `colStart`/`colEnd`, while keeping the existing latitude and
row logic intact. Use the `clampedColumn`, `clampedRow`, and `candidates()` flow
as the main place to adjust the bounds handling.

In `@package/ios/HybridMapView.swift`:
- Around line 249-266: The live clustering timer created in startLiveClustering
is not torn down when the view is recycled, so it can remain active and block
reuse; update HybridMapView’s lifecycle cleanup by invalidating liveClusterTimer
in prepareForRecycle() and ensuring any teardown path (including deinit) calls
stopLiveClustering() or equivalent. Use the existing startLiveClustering(),
stopLiveClustering(), and prepareForRecycle() methods to locate the change and
make sure the timer is always cleared before the view can be reused.
- Around line 254-259: The live clustering timer created in `HybridMapView` is
tied to the default run-loop mode and is not cleaned up during reuse, so it can
pause during map gestures and leave stale state behind. Update the
`liveClusterTimer` setup to add the timer to `.common` run-loop modes so it
continues firing during pan/zoom, and make `prepareForRecycle()` explicitly
invalidate and clear `liveClusterTimer` before the view is recycled.

In `@package/ios/MapOverlayController.swift`:
- Around line 185-227: The viewport diff logic in computeDiff and applyDiff only
handles added and removed annotations, so existing markers and clusters are
never refreshed when their underlying data changes. Update the diffing flow to
detect when a displayed annotation with the same diffKey needs content/position
changes, then call MapMarkerAnnotation.update(from:) for markers and the
equivalent cluster refresh path before reusing the annotation. Keep the existing
add/remove behavior, but extend MarkerDiff and applyDiff so stale annotations
are updated in place instead of left unchanged.
- Around line 91-96: The reapplyMarkers path in MapOverlayController leaves
queued viewport work alive when switching from usesViewportPipeline to the sync
path, so stale computeQueue results can overwrite the freshly reconciled
markers. Update reapplyMarkers to cancel any pending viewport work item and
advance refreshGeneration before calling
reconcileMarkersSync(allMarkerDescriptors), using the existing computeQueue and
refreshGeneration state so old async callbacks can no longer win.

In `@package/ios/MarkerSpatialIndex.swift`:
- Around line 58-87: The longitude range logic in
MarkerSpatialIndex.candidates(in:padding:) treats the viewport as a single
min-to-max interval, so regions crossing the date line miss markers on the
wrapped side. Update the lookup to handle wrapped longitudes by either splitting
the query into two longitude ranges or normalizing longitudes before calling
clampedColumn, and make sure the candidates collection still includes cells from
both sides of the antimeridian.

In `@package/ios/MarkerViewportFilter.swift`:
- Around line 88-101: The MKCoordinateRegion.contains(_:, padding:) check in
MarkerViewportFilter is not date-line aware, so longitude bounds fail when the
region wraps past ±180°. Update the longitude portion of this method to use
wrap-aware normalization or interval handling while keeping the latitude logic
unchanged, and make sure the fix preserves valid coordinates near the
antimeridian before subsampling/clustering.

In `@package/src/components/MapView.tsx`:
- Around line 60-75: The bulk overlay selection in MapView is treating explicit
empty arrays as absent, so controlled props like markers, polylines, polygons,
and circles cannot intentionally render nothing. Update the overlay resolution
logic in MapView so that the prop value is used whenever it is provided, even if
it is an empty array, and only fall back to collectedMarkers,
collectedPolylines, collectedPolygons, and collectedCircles when the prop is
actually undefined/null. Keep the behavior consistent across all four overlay
variables.

In `@package/src/hooks/useCollectedOverlays.ts`:
- Around line 67-197: The callback registry in useCollectedOverlays is keyed
only by bare overlay id, so different overlay types can overwrite each other
when they reuse the same id. Update the registry keying in overlayCollectors and
OverlayCollectorState so it is namespaced by overlay type (for example via a
shared helper used by both writers and readers), and make MapView.tsx use the
same key when looking up callbacks for each type-specific overlay event handler.

---

Outside diff comments:
In `@example/App.tsx`:
- Around line 535-540: The ready-time fit in the scenario setup is using only
scenario.advanced.mapPadding, so the extra padding from mergeMapPadding is being
skipped. Update the fitToCoordinates call in App.tsx to use the same merged
padding value that MapView receives, and keep the logic tied to
scenario.advanced?.fitToCoordinatesOnReady and the marker coordinate list so
both map layout and initial fit stay consistent.

In `@package/ios/HybridMapViewDelegate.swift`:
- Around line 151-156: The marker tap handler in HybridMapViewDelegate’s
mapView(_:didSelect:) is immediately closing the callout by calling
deselectAnnotation(_:animated:) after onMarkerPress. Remove that deselect call
for MapMarkerAnnotation selections so markers with canShowCallout stay open long
enough for the title/subtitle to be read. Keep the press callback on marker.id
and only change the selection state if there is a separate, intentional
dismissal path elsewhere.
🪄 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

Run ID: feeb1508-0619-41be-9db9-118b6984ed67

📥 Commits

Reviewing files that changed from the base of the PR and between 601f796 and e8f472b.

⛔ Files ignored due to path filters (1)
  • bun.lock is excluded by !**/*.lock
📒 Files selected for processing (37)
  • .gitignore
  • docs/architecture.md
  • example/App.tsx
  • example/examples/advancedFeatures.ts
  • example/examples/types.ts
  • example/index.js
  • example/metro.config.js
  • example/package.json
  • package/android/src/main/java/com/margelo/nitro/nitromaps/ClusterBadgeMetrics.kt
  • package/android/src/main/java/com/margelo/nitro/nitromaps/ClusterIconFactory.kt
  • package/android/src/main/java/com/margelo/nitro/nitromaps/HybridMapView.kt
  • package/android/src/main/java/com/margelo/nitro/nitromaps/MapOverlayController.kt
  • package/android/src/main/java/com/margelo/nitro/nitromaps/MarkerClusterEngine.kt
  • package/android/src/main/java/com/margelo/nitro/nitromaps/MarkerDescriptor+MarkerOptions.kt
  • package/android/src/main/java/com/margelo/nitro/nitromaps/MarkerSpatialIndex.kt
  • package/android/src/main/java/com/margelo/nitro/nitromaps/MarkerViewportFilter.kt
  • package/android/src/main/java/com/margelo/nitro/nitromaps/NitroClusterItem.kt
  • package/ios/ClusterBadgeMetrics.swift
  • package/ios/HybridMapView.swift
  • package/ios/HybridMapViewDelegate.swift
  • package/ios/MKMapView+ScaleAppearance.swift
  • package/ios/MapClusterAnnotation.swift
  • package/ios/MapOverlayController.swift
  • package/ios/MarkerClusterEngine.swift
  • package/ios/MarkerSpatialIndex.swift
  • package/ios/MarkerViewportFilter.swift
  • package/ios/NitroClusterAnnotationView.swift
  • package/ios/NitroMKMapView.swift
  • package/ios/NitroPinAnnotationView.swift
  • package/src/components/Circle.tsx
  • package/src/components/MapView.tsx
  • package/src/components/Marker.tsx
  • package/src/components/Polygon.tsx
  • package/src/components/Polyline.tsx
  • package/src/hooks/useCollectedOverlays.ts
  • package/src/overlays/overlayType.ts
  • package/src/types/map.ts
💤 Files with no reviewable changes (1)
  • package/android/src/main/java/com/margelo/nitro/nitromaps/NitroClusterItem.kt

Comment thread example/App.tsx Outdated
Comment thread package/android/src/main/java/com/margelo/nitro/nitromaps/MapOverlayController.kt Outdated
Comment thread package/ios/MapOverlayController.swift
Comment thread package/ios/MarkerSpatialIndex.swift
Comment thread package/ios/MarkerViewportFilter.swift
Comment thread package/src/components/MapView.tsx Outdated
Comment on lines +67 to +197
interface OverlayCollectorState {
registry: Map<string, OverlayCallbacks>;
markers: MarkerDescriptor[];
polylines: PolylineDescriptor[];
polygons: PolygonDescriptor[];
circles: CircleDescriptor[];
markerIndex: number;
polylineIndex: number;
polygonIndex: number;
circleIndex: number;
hasMarkerPress: boolean;
hasMarkerDragEnd: boolean;
hasPolylinePress: boolean;
hasPolygonPress: boolean;
hasCirclePress: boolean;
}

interface OverlayCollector {
overlayType: OverlayTypeName;
component: unknown;
idPrefix: string;
collect: (child: ReactElement, state: OverlayCollectorState) => void;
}

function tappableFromPress(
onPress: (() => void) | undefined,
tappable: boolean | undefined,
): boolean | undefined {
return onPress != null ? (tappable ?? true) : tappable;
}

const overlayCollectors: OverlayCollector[] = [
{
overlayType: OverlayType.Marker,
component: Marker,
idPrefix: 'marker',
collect: (child, state) => {
const props = child.props as MarkerProps;
const id = resolveOverlayId(props.id, 'marker', state.markerIndex);
state.markerIndex += 1;

state.markers.push({
id,
coordinate: props.coordinate,
title: props.title,
subtitle: props.subtitle,
draggable: props.draggable,
clusterable: props.clusterable,
});
state.registry.set(id, {
onPress: props.onPress,
onDragEnd: props.onDragEnd,
});
if (props.onPress != null) {
state.hasMarkerPress = true;
}
if (props.onDragEnd != null) {
state.hasMarkerDragEnd = true;
}
},
},
{
overlayType: OverlayType.Polyline,
component: Polyline,
idPrefix: 'polyline',
collect: (child, state) => {
const props = child.props as PolylineProps;
const id = resolveOverlayId(props.id, 'polyline', state.polylineIndex);
state.polylineIndex += 1;

state.polylines.push({
id,
coordinates: props.coordinates,
strokeColor: props.strokeColor,
strokeWidth: props.strokeWidth,
tappable: tappableFromPress(props.onPress, props.tappable),
});
state.registry.set(id, { onPress: props.onPress });
if (props.onPress != null) {
state.hasPolylinePress = true;
}
},
},
{
overlayType: OverlayType.Polygon,
component: Polygon,
idPrefix: 'polygon',
collect: (child, state) => {
const props = child.props as PolygonProps;
const id = resolveOverlayId(props.id, 'polygon', state.polygonIndex);
state.polygonIndex += 1;

state.polygons.push({
id,
coordinates: props.coordinates,
fillColor: props.fillColor,
strokeColor: props.strokeColor,
strokeWidth: props.strokeWidth,
tappable: tappableFromPress(props.onPress, props.tappable),
});
state.registry.set(id, { onPress: props.onPress });
if (props.onPress != null) {
state.hasPolygonPress = true;
}
},
},
{
overlayType: OverlayType.Circle,
component: Circle,
idPrefix: 'circle',
collect: (child, state) => {
const props = child.props as CircleProps;
const id = resolveOverlayId(props.id, 'circle', state.circleIndex);
state.circleIndex += 1;

state.circles.push({
id,
center: props.center,
radius: props.radius,
fillColor: props.fillColor,
strokeColor: props.strokeColor,
strokeWidth: props.strokeWidth,
tappable: tappableFromPress(props.onPress, props.tappable),
});
state.registry.set(id, { onPress: props.onPress });
if (props.onPress != null) {
state.hasCirclePress = true;
}
},
},
];

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Namespace the callback registry by overlay type.

Lines 116, 144, 167, and 191 write every overlay callback into one Map<string, OverlayCallbacks> keyed by bare id. The descriptor contracts only promise uniqueness per overlay kind, so a marker and polygon that both use id="foo" will stomp each other here. package/src/components/MapView.tsx then does callbackRegistry.current.get(id) for every overlay event, so the wrong JS callback fires deterministically.

Suggested direction
+function registryKey(type: OverlayTypeName, id: string): string {
+  return `${type}:${id}`;
+}
+
 interface OverlayCollectorState {
   registry: Map<string, OverlayCallbacks>;
   ...
 }
 ...
-      state.registry.set(id, {
+      state.registry.set(registryKey(OverlayType.Marker, id), {
         onPress: props.onPress,
         onDragEnd: props.onDragEnd,
       });
...
-      state.registry.set(id, { onPress: props.onPress });
+      state.registry.set(registryKey(OverlayType.Polyline, id), {
+        onPress: props.onPress,
+      });
...
-      state.registry.set(id, { onPress: props.onPress });
+      state.registry.set(registryKey(OverlayType.Polygon, id), {
+        onPress: props.onPress,
+      });
...
-      state.registry.set(id, { onPress: props.onPress });
+      state.registry.set(registryKey(OverlayType.Circle, id), {
+        onPress: props.onPress,
+      });

MapView should use the same helper when reading the registry in each type-specific handler.

🤖 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 `@package/src/hooks/useCollectedOverlays.ts` around lines 67 - 197, The
callback registry in useCollectedOverlays is keyed only by bare overlay id, so
different overlay types can overwrite each other when they reuse the same id.
Update the registry keying in overlayCollectors and OverlayCollectorState so it
is namespaced by overlay type (for example via a shared helper used by both
writers and readers), and make MapView.tsx use the same key when looking up
callbacks for each type-specific overlay event handler.

…gic, and enhance overlay handling in MapView component.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
`@package/android/src/main/java/com/margelo/nitro/nitromaps/MarkerSpatialIndex.kt`:
- Around line 75-100: In MarkerSpatialIndex, the padded longitude window should
be normalized before calling longitudeColumns so date-line overflow is handled
correctly. Update the query path that computes minLonQ/maxLonQ to wrap values
crossing ±180° into the wrapped range, then let longitudeColumns operate on
normalized bounds so cases like [170, 181] include both sides of the date line
instead of only clamping the upper bound.
🪄 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

Run ID: 6e137487-4af1-46aa-b9ac-987fdb372c43

📥 Commits

Reviewing files that changed from the base of the PR and between e8f472b and 0b20046.

⛔ Files ignored due to path filters (1)
  • example/ios/Podfile.lock is excluded by !**/*.lock
📒 Files selected for processing (18)
  • .github/workflows/ci.yml
  • example/App.tsx
  • package/android/src/main/java/com/margelo/nitro/nitromaps/ClusterIconFactory.kt
  • package/android/src/main/java/com/margelo/nitro/nitromaps/MapOverlayController.kt
  • package/android/src/main/java/com/margelo/nitro/nitromaps/MarkerClusterEngine.kt
  • package/android/src/main/java/com/margelo/nitro/nitromaps/MarkerSpatialIndex.kt
  • package/ios/HybridMapView.swift
  • package/ios/HybridMapViewDelegate.swift
  • package/ios/MapClusterAnnotation.swift
  • package/ios/MapOverlayController.swift
  • package/ios/MarkerClusterEngine.swift
  • package/ios/MarkerSpatialIndex.swift
  • package/ios/MarkerViewportFilter.swift
  • package/ios/NitroPinAnnotationView.swift
  • package/src/components/MapView.tsx
  • package/src/hooks/useCollectedOverlays.ts
  • package/src/overlays/overlayType.ts
  • package/src/types/map.ts
🚧 Files skipped from review as they are similar to previous changes (14)
  • package/src/overlays/overlayType.ts
  • package/android/src/main/java/com/margelo/nitro/nitromaps/ClusterIconFactory.kt
  • package/ios/MarkerClusterEngine.swift
  • package/src/types/map.ts
  • package/ios/HybridMapViewDelegate.swift
  • package/src/components/MapView.tsx
  • package/android/src/main/java/com/margelo/nitro/nitromaps/MapOverlayController.kt
  • package/android/src/main/java/com/margelo/nitro/nitromaps/MarkerClusterEngine.kt
  • package/ios/MarkerViewportFilter.swift
  • package/ios/HybridMapView.swift
  • example/App.tsx
  • package/ios/MarkerSpatialIndex.swift
  • package/src/hooks/useCollectedOverlays.ts
  • package/ios/MapOverlayController.swift

… better handling of longitude wrapping and edge cases.
@jkasprzyk17
jkasprzyk17 merged commit 893f71c into main Jun 26, 2026
2 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.

1 participant