Skip to content

Commit ced77ca

Browse files
fix: address overlay animation review feedback
1 parent 33405d4 commit ced77ca

10 files changed

Lines changed: 296 additions & 59 deletions

README.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,8 @@ When no animation prop is set, the default is `system`: each provider keeps its
164164

165165
Explicit configs use milliseconds. `duration` defaults to `180`, `delay` defaults to `0`, and both values are clamped to `0..3000` before they reach the native provider. `reduceMotion` defaults to `system`, which disables explicit animations when the platform Reduced Motion setting asks for it; use `never` only when the app intentionally ignores that setting for this overlay.
166166

167+
On iOS with `provider="google"`, marker and cluster entering animations can reduce UI-thread frame rate when a large viewport refresh adds many markers at once. The provider caps animated markers per refresh and may show the remaining markers immediately to preserve map gesture performance. For very large marker sets, prefer clustering, shorter durations, or `markerEnteringAnimation={false}` / `clusterEnteringAnimation={false}` when smooth gestures are more important than entrance motion.
168+
167169
### Capability matrix
168170

169171
| Capability | `apple` iOS | `google` iOS | `google` Android | Future providers |

docs/architecture.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,8 @@ Map and overlay callbacks are wired through Nitro listeners on the HybridView. C
103103

104104
Marker and marker-cluster entering animations follow the same descriptor model. The public API accepts `false`, `system`, or a serializable preset config; the React wrapper normalizes that into native descriptors. Native provider adapters execute the animation when a marker render element appears in the render diff. Updating animation config for an already retained marker does not restart the animation; the new config is used the next time that marker is added again.
105105

106+
Google Maps on iOS is more sensitive to marker animation churn than MapKit. Large viewport refreshes can add many `GMSMarker` instances on the main thread, so the Google provider limits how many markers animate per refresh and reveals the rest immediately. This keeps gestures responsive, but very large marker sets may still need clustering, disabled entering animations, or a future provider-specific animation strategy.
107+
106108
## Data flow (target state)
107109

108110
```

example/App.tsx

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ import {
1616
StyleSheet,
1717
Text,
1818
View,
19+
type AccessibilityRole,
20+
type AccessibilityState,
1921
type StyleProp,
2022
type ViewStyle,
2123
} from 'react-native';
@@ -141,13 +143,19 @@ type ScalePressableProps = {
141143
style?: StyleProp<ViewStyle>;
142144
children: ReactNode;
143145
hitSlop?: number;
146+
accessibilityRole?: AccessibilityRole;
147+
accessibilityState?: AccessibilityState;
148+
accessibilityLabel?: string;
144149
};
145150

146151
const ScalePressable = memo(function ScalePressable({
147152
onPress,
148153
style,
149154
children,
150155
hitSlop,
156+
accessibilityRole,
157+
accessibilityState,
158+
accessibilityLabel,
151159
}: ScalePressableProps) {
152160
const scale = useSharedValue(1);
153161

@@ -158,6 +166,9 @@ const ScalePressable = memo(function ScalePressable({
158166
return (
159167
<AnimatedPressable
160168
hitSlop={hitSlop}
169+
accessibilityRole={accessibilityRole}
170+
accessibilityState={accessibilityState}
171+
accessibilityLabel={accessibilityLabel}
161172
style={[style, animatedStyle]}
162173
onPress={onPress}
163174
onPressIn={() => {
@@ -341,6 +352,11 @@ const ScenarioDock = memo(function ScenarioDock({
341352
<ScalePressable
342353
key={option.id}
343354
onPress={() => onSelectAnimation(option.id)}
355+
accessibilityRole="button"
356+
accessibilityState={{
357+
selected: option.id === animationOptionId,
358+
}}
359+
accessibilityLabel={`${option.label} entering animation`}
344360
style={[
345361
styles.optionChip,
346362
option.id === animationOptionId && styles.optionChipActive,
@@ -581,7 +597,7 @@ export default function App() {
581597
const [scenarioIndex, setScenarioIndex] = useState(0);
582598
const [mapTypeIndex, setMapTypeIndex] = useState(0);
583599
const [providerIndex, setProviderIndex] = useState(0);
584-
const [animationOptionIndex, setAnimationOptionIndex] = useState(2);
600+
const [animationOptionIndex, setAnimationOptionIndex] = useState(0);
585601
const [status, setStatus] = useState('Waiting for map...');
586602
const [mapReady, setMapReady] = useState(false);
587603
const [dockExpanded, setDockExpanded] = useState(false);

package/ios/GoogleMapOverlayController.swift

Lines changed: 64 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,20 @@ import UIKit
44

55
final class GoogleMapOverlayController {
66
private static let clusterCellPoints: Double = 96
7+
// GMSMarker entering animations are main-thread work; cap them per diff so
8+
// bulk viewport refreshes do not block active Google Maps gestures.
9+
private static let maximumAnimatedMarkersPerDiff = 96
710

811
private enum MarkerPayload {
912
case marker(String)
1013
case cluster(memberIds: [String], region: MKCoordinateRegion)
1114
}
1215

16+
private struct MarkerAnimationBatch {
17+
let animation: ResolvedOverlayEnteringAnimation
18+
var markers: [GMSMarker]
19+
}
20+
1321
private weak var mapView: GMSMapView?
1422
private var markers: [String: GMSMarker] = [:]
1523
private var markerVersions: [String: Int] = [:]
@@ -58,7 +66,10 @@ final class GoogleMapOverlayController {
5866
reapplyMarkers()
5967
}
6068

61-
func refreshViewportMarkers() {
69+
func refreshViewportMarkers(
70+
animateEntering: Bool = true,
71+
animationBudget: Int = maximumAnimatedMarkersPerDiff
72+
) {
6273
guard let mapView, usesViewportPipeline else {
6374
return
6475
}
@@ -68,12 +79,20 @@ final class GoogleMapOverlayController {
6879
region: mapView.currentNitroRegion().toMKCoordinateRegion(),
6980
viewSize: mapView.bounds.size,
7081
apply: { [weak self] diff in
71-
self?.applyDiff(diff)
82+
self?.applyDiff(
83+
diff,
84+
animateEntering: animateEntering,
85+
animationBudget: animationBudget
86+
)
7287
}
7388
)
7489
}
7590

76-
func scheduleViewportRefresh(immediate: Bool = false) {
91+
func scheduleViewportRefresh(
92+
immediate: Bool = false,
93+
animateEntering: Bool = true,
94+
animationBudget: Int = maximumAnimatedMarkersPerDiff
95+
) {
7796
guard let mapView, usesViewportPipeline else {
7897
return
7998
}
@@ -84,7 +103,11 @@ final class GoogleMapOverlayController {
84103
viewSize: mapView.bounds.size,
85104
immediate: immediate,
86105
apply: { [weak self] diff in
87-
self?.applyDiff(diff)
106+
self?.applyDiff(
107+
diff,
108+
animateEntering: animateEntering,
109+
animationBudget: animationBudget
110+
)
88111
}
89112
)
90113
}
@@ -173,7 +196,11 @@ final class GoogleMapOverlayController {
173196
)
174197
}
175198

176-
private func applyDiff(_ diff: MarkerRenderDiff) {
199+
private func applyDiff(
200+
_ diff: MarkerRenderDiff,
201+
animateEntering: Bool = true,
202+
animationBudget: Int = maximumAnimatedMarkersPerDiff
203+
) {
177204
guard let mapView else {
178205
return
179206
}
@@ -183,16 +210,34 @@ final class GoogleMapOverlayController {
183210
markerVersions.removeValue(forKey: key)
184211
}
185212

213+
var animationBatches: [MarkerAnimationBatch] = []
214+
var remainingAnimationBudget = animateEntering ? max(0, animationBudget) : 0
215+
186216
for entry in diff.added {
187217
let marker = makeMarker(for: entry.element)
188218
let animation = enteringAnimation(for: entry.element)
189-
OverlayEnteringAnimationResolver.prepareGoogleMarker(marker, animation: animation)
219+
let shouldAnimate = animateEntering
220+
&& remainingAnimationBudget > 0
221+
&& OverlayEnteringAnimationResolver.canAnimateGoogleMarker(animation)
222+
223+
if shouldAnimate {
224+
OverlayEnteringAnimationResolver.prepareGoogleMarker(marker, animation: animation)
225+
if OverlayEnteringAnimationResolver.usesBatchedGoogleMarkerAnimation(animation) {
226+
append(marker, animation: animation, to: &animationBatches)
227+
}
228+
remainingAnimationBudget -= 1
229+
} else {
230+
OverlayEnteringAnimationResolver.showGoogleMarkerWithoutAnimation(marker)
231+
}
190232
marker.map = mapView
191-
OverlayEnteringAnimationResolver.animateGoogleMarker(marker, animation: animation)
192233
markers[entry.key] = marker
193234
markerVersions[entry.key] = entry.version
194235
}
195236

237+
for batch in animationBatches {
238+
OverlayEnteringAnimationResolver.animateGoogleMarkers(batch.markers, animation: batch.animation)
239+
}
240+
196241
for entry in diff.retained {
197242
guard let marker = markers[entry.key] else {
198243
continue
@@ -208,6 +253,18 @@ final class GoogleMapOverlayController {
208253
return marker
209254
}
210255

256+
private func append(
257+
_ marker: GMSMarker,
258+
animation: ResolvedOverlayEnteringAnimation,
259+
to batches: inout [MarkerAnimationBatch]
260+
) {
261+
for index in batches.indices where batches[index].animation == animation {
262+
batches[index].markers.append(marker)
263+
return
264+
}
265+
batches.append(MarkerAnimationBatch(animation: animation, markers: [marker]))
266+
}
267+
211268
private func enteringAnimation(
212269
for element: MarkerClusterEngine.Element
213270
) -> ResolvedOverlayEnteringAnimation {

package/ios/GoogleMapProviderAdapter.swift

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@ import QuartzCore
66
import UIKit
77

88
final class GoogleMapProviderAdapter: NSObject, MapProviderAdapter {
9+
private static let liveGestureRefreshInterval: CFTimeInterval = 0.18
10+
private static let liveGestureAnimationBudget = 24
11+
912
private var isProgrammaticUpdate = false
1013
private var pendingProgrammaticUpdateIDs: [Int] = []
1114
private var nextProgrammaticUpdateID = 0
@@ -30,7 +33,6 @@ final class GoogleMapProviderAdapter: NSObject, MapProviderAdapter {
3033
}
3134

3235
lazy var view: GMSMapView = {
33-
GoogleMapsAPIKey.configureIfNeeded()
3436
let camera = self.camera?.toGMSCameraPosition()
3537
?? GMSCameraPosition(latitude: 0, longitude: 0, zoom: 10)
3638
let mapView: GMSMapView
@@ -55,7 +57,8 @@ final class GoogleMapProviderAdapter: NSObject, MapProviderAdapter {
5557
return mapView
5658
}()
5759

58-
init(googleMapId: String?) {
60+
init(googleMapId: String?) throws {
61+
try GoogleMapsAPIKey.configureIfNeeded()
5962
_googleMapId = googleMapId
6063
super.init()
6164
}
@@ -377,6 +380,7 @@ final class GoogleMapProviderAdapter: NSObject, MapProviderAdapter {
377380
private func startGestureMarkerRefresh() {
378381
isUserGestureMoving = true
379382
lastLiveMarkerRefreshTime = 0
383+
refreshGestureMarkersIfNeeded()
380384
}
381385

382386
private func refreshGestureMarkersIfNeeded() {
@@ -385,18 +389,20 @@ final class GoogleMapProviderAdapter: NSObject, MapProviderAdapter {
385389
}
386390

387391
let now = CACurrentMediaTime()
388-
guard now - lastLiveMarkerRefreshTime >= MarkerRenderPipeline.liveRefreshInterval else {
392+
guard now - lastLiveMarkerRefreshTime >= Self.liveGestureRefreshInterval else {
389393
return
390394
}
391395

392396
lastLiveMarkerRefreshTime = now
393-
overlayController.refreshViewportMarkers()
397+
overlayController.refreshViewportMarkers(
398+
animateEntering: true,
399+
animationBudget: Self.liveGestureAnimationBudget
400+
)
394401
}
395402

396403
private func stopGestureMarkerRefresh() {
397404
isUserGestureMoving = false
398405
lastLiveMarkerRefreshTime = 0
399-
refreshVisibleMarkers()
400406
}
401407

402408
private func animateToClusterRegion(_ region: MKCoordinateRegion) {
@@ -528,6 +534,7 @@ extension GoogleMapProviderAdapter: GMSMapViewDelegate {
528534

529535
func mapView(_ mapView: GMSMapView, idleAt position: GMSCameraPosition) {
530536
stopGestureMarkerRefresh()
537+
refreshVisibleMarkers()
531538
notifyRegionChange(complete: true)
532539
endProgrammaticUpdate()
533540
notifyMapReadyIfNeeded()

package/ios/GoogleMapsAPIKey.swift

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,12 @@ import GoogleMaps
44
enum GoogleMapsAPIKey {
55
private static var configuredKey: String?
66

7-
static func configureIfNeeded() {
7+
static func configureIfNeeded() throws {
88
let key = Bundle.main.object(forInfoDictionaryKey: "GoogleMapsIosApiKey") as? String
99
guard let key = key?.trimmingCharacters(in: .whitespacesAndNewlines),
10-
!key.isEmpty else {
11-
preconditionFailure(
12-
"react-native-nitro-maps: provider=\"google\" on iOS requires GoogleMapsIosApiKey in the host app Info.plist."
13-
)
10+
!key.isEmpty,
11+
!key.hasPrefix("$(") else {
12+
throw MapProviderConfigurationError.missingGoogleMapsIosApiKey
1413
}
1514

1615
guard configuredKey != key else {
@@ -21,3 +20,17 @@ enum GoogleMapsAPIKey {
2120
configuredKey = key
2221
}
2322
}
23+
24+
enum MapProviderConfigurationError: LocalizedError {
25+
case missingGoogleMapsIosApiKey
26+
case unsupportedIOSProvider(MapProvider)
27+
28+
var errorDescription: String? {
29+
switch self {
30+
case .missingGoogleMapsIosApiKey:
31+
return "react-native-nitro-maps: provider=\"google\" on iOS requires GoogleMapsIosApiKey in the host app Info.plist."
32+
case let .unsupportedIOSProvider(provider):
33+
return "Map provider \"\(provider)\" is not supported on iOS."
34+
}
35+
}
36+
}

package/ios/HybridMapView.swift

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -334,9 +334,15 @@ final class HybridMapView: HybridMapViewSpec {
334334
case .apple:
335335
return AppleMapProviderAdapter()
336336
case .google:
337-
return GoogleMapProviderAdapter(googleMapId: _googleMapId)
337+
do {
338+
return try GoogleMapProviderAdapter(googleMapId: _googleMapId)
339+
} catch {
340+
return UnavailableMapProviderAdapter(error: error)
341+
}
338342
case .openstreetmap, .mapbox:
339-
preconditionFailure("Map provider \"\(provider)\" is not supported on iOS.")
343+
return UnavailableMapProviderAdapter(
344+
error: MapProviderConfigurationError.unsupportedIOSProvider(provider)
345+
)
340346
}
341347
}
342348

package/ios/HybridMapViewDelegate.swift

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -126,21 +126,11 @@ final class HybridMapViewDelegate: NSObject, MKMapViewDelegate, UIGestureRecogni
126126
animation: marker.enteringAnimation,
127127
supportsScale: true
128128
)
129-
} else if let cluster = view.annotation as? MapClusterAnnotation {
130-
let animation: ResolvedOverlayEnteringAnimation
131-
if cluster.enteringAnimation.kind == .system {
132-
animation = ResolvedOverlayEnteringAnimation(
133-
kind: .fade,
134-
duration: 0.16,
135-
delay: 0,
136-
reduceMotion: .system
137-
)
138-
} else {
139-
animation = cluster.enteringAnimation
140-
}
129+
} else if let cluster = view.annotation as? MapClusterAnnotation,
130+
cluster.enteringAnimation.kind != .system {
141131
OverlayEnteringAnimationResolver.animateAnnotationView(
142132
view,
143-
animation: animation,
133+
animation: cluster.enteringAnimation,
144134
supportsScale: true
145135
)
146136
}

0 commit comments

Comments
 (0)