diff --git a/README.md b/README.md index 1856e8d..3d8dcb6 100644 --- a/README.md +++ b/README.md @@ -30,6 +30,7 @@ Built with [Nitro Modules](https://nitro.margelo.com/) for high-performance nati - [Installation](#installation) - [Quick start](#quick-start) - [Map providers](#map-providers) +- [Region change events](#region-change-events) - [Native POI press events](#native-poi-press-events) - [Custom marker images](#custom-marker-images) - [GeoJSON overlays](#geojson-overlays) @@ -210,7 +211,9 @@ function MyMap() { console.log(region)} + onRegionChangeComplete={(region, details) => + console.log(region, details.isGesture) + } > (null); - const flyToWarsaw = () => { - mapRef.current?.animateCamera({ - center: { latitude: 52.2297, longitude: 21.0122 }, - zoom: 12, - }); + const flyToWarsaw = async () => { + const map = mapRef.current; + if (map == null) { + return; + } + + try { + await map.animateCamera( + { center: { latitude: 52.2297, longitude: 21.0122 }, zoom: 12 }, + 1000, + ); + + // The camera is there now, so this reads where it actually arrived. + const camera = await map.getCamera(); + console.log(camera.center); + } catch (error) { + // The map view unmounted, so there is no camera left to move or read. + console.warn(error); + } }; return ; @@ -336,6 +353,26 @@ builds wrapped in React's `` see this on every mount: React tears the effect down and sets it up again, the first call is rejected by that teardown, and the second one does the work. +#### When the camera promises settle + +`animateCamera`, `animateToRegion` and `fitToCoordinates` resolve when the camera +has arrived, not when the animation is handed to the map. `setCamera` moves +without animating, so it resolves right away, as do a `0` duration and +`fitToCoordinates` with `animated: false`. + +An animation that is cut short - by a gesture, by a later camera command, or by +the map view unmounting mid-animation - resolves too rather than hanging. It does +not reject, and it does not report whether the requested position was reached: +read `getVisibleRegion()` or `getCamera()` after the `await` when that matters. +(On Apple Maps a gesture cannot cut `animateCamera` or `animateToRegion` short: +the map ignores touches until the animation ends.) + +Pass `fitToCoordinates`' `animated` argument explicitly: left out, iOS animates +the fit and Android jumps to it. + +> **Behavior change after 1.2.1:** these promises used to resolve as soon as the +> animation started, so `await` returned with the camera still at its old position. + #### Screen points `pointForCoordinate` and `coordinateForPoint` convert between a coordinate and a @@ -392,7 +429,7 @@ function LabelledMap() { ``` A point describes the camera at the time of the call, so convert again once the -camera has moved; `onRegionChangeComplete` reports the moves the user makes. A +camera has moved; `onRegionChangeComplete` reports every move, whoever made it. A coordinate that is off screen converts to a point outside the map view's bounds. Both calls reject straight away for input that is not on the map: a coordinate outside ±90 / ±180, or a point whose `x` or `y` is not finite. @@ -423,6 +460,68 @@ Changing `provider` remounts the native map view. Controlled props such as `regi Provider-specific TypeScript props are exposed through `MapViewPropsForProvider

`. For example, `showsScale` is accepted for `apple` but rejected for `google` because Google Maps SDK has no native scale control. +## Region change events + +`onRegionChange` fires when the camera starts moving and `onRegionChangeComplete` when it +stops. Both fire for every move, whoever started it: a pinch or pan, `setCamera`, +`animateCamera`, `fitToCoordinates`, or an updated `region` / `camera` prop. The second +argument says which it was. + +```tsx + { + if (details.isGesture) { + cancelAutoFollow(); + } + }} + onRegionChangeComplete={(region, details) => { + console.log( + details.isGesture ? 'user moved the map' : 'the app moved the map', + ); + }} +/> +``` + +One move emits exactly one `onRegionChange` and one `onRegionChangeComplete`, with the +same `details` on both, and nothing fires while the camera is on its way. An update that +leaves the camera where it is - a `region` or `camera` prop set to what the map already +shows, or a repeated `fitToCoordinates` - emits nothing, and neither does the map settling +into its first position as it appears. Read `getVisibleRegion()` from `onMapReady` for +that one. + +A gesture that interrupts the app's own animation ends that move and starts the user's: +`onRegionChangeComplete` with `isGesture: false` where the finger caught the camera, then +`onRegionChange` with `isGesture: true`. The opposite does not split: a camera command +issued while the map is still moving from a gesture stays part of the gesture's move. On +Apple Maps an `animateCamera` animation cannot be interrupted this way, because the map +ignores touches until it ends. + +`isGesture` means a touch gesture on the map itself. Android's own controls (the zoom +buttons and the my-location button) report as `isGesture: false`, because the Google Maps +SDK classifies them as an API animation rather than a gesture. + +### react-native-maps migration (region events) + +| react-native-maps | react-native-better-maps | +| ------------------------------------------------- | ---------------------------------------------------- | +| `onRegionChangeStart(region, details)` | `onRegionChange(region, details)` | +| `onRegionChange(region, details)`, on every frame | No equivalent - nothing fires while the camera moves | +| `onRegionChangeComplete(region, details)` | Same | +| `details.isGesture`, on Google Maps only | `details.isGesture`, on Apple and Google Maps alike | + +Note the first two rows: `onRegionChange` here fires once, when a move begins, which is +what `react-native-maps` calls `onRegionChangeStart`. + +> **Behavior change after 1.2.1:** both callbacks used to fire only for user gestures, and +> took the region alone. Code that treated every event as user input should now check +> `details.isGesture`. + +Feeding `onRegionChangeComplete` back into a controlled `region` prop is safe: the map is +already showing that region, so the update moves nothing and emits nothing. Feeding back +`onRegionChange` is not. It hands the map the region the camera is leaving, and for a move +the app started, going back there cancels it. + ## Native POI press events Provider-owned points of interest are base-map features supplied by Apple Maps or Google Maps, such as restaurants, parks, schools, hotels, and stores. They are separate from app-owned `` elements and bulk `markers`; marker presses still use `Marker.onPress` and `MapView.onMarkerPress`. @@ -806,6 +905,7 @@ An optional overlay field set to `null` - the way JSON data usually says "no val | Visible region | Supported | Supported | Supported | | Fit to coordinates | Supported | Supported | Supported | | Screen point conversion | Supported | Supported | Supported | +| Region change events | Supported, with `isGesture` | Supported, with `isGesture` | Supported, with `isGesture` | | Map types | Standard, satellite, hybrid; terrain falls back to standard | Standard, satellite, hybrid, terrain | Standard, satellite, hybrid, terrain | | Gestures | Supported | Supported | Supported | | User location | Supported; host app owns permission prompt | Supported; host app owns permission prompt | Supported; host app owns permission prompt | @@ -845,6 +945,7 @@ An optional overlay field set to `null` - the way JSON data usually says "no val | `Coordinate` | `{ latitude, longitude }` | | `Point` | `{ x, y }` in dp from the map view's top-left corner | | `Region` | Center + span | +| `RegionChangeDetails` | `{ isGesture }` context for a region change | | `Camera` | Position, zoom, heading, pitch | | `MapType` | `'standard' \| 'satellite' \| 'hybrid' \| 'terrain'` | | `MapProvider` | `'apple' \| 'google' \| 'openstreetmap' \| 'mapbox'` | diff --git a/docs/architecture.md b/docs/architecture.md index 79f364f..f827633 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -63,17 +63,17 @@ Provider adapters own SDK-specific view creation, destruction, lifecycle, camera ### Events -Map and overlay callbacks are wired through Nitro listeners on the HybridView. Callbacks receive payloads directly (e.g. `onPress(coordinate)`, `onRegionChange(region)`). - -| Callback | Payload | Notes | -| ------------------------------------------- | ------------------------ | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `onRegionChange` / `onRegionChangeComplete` | `Region` | iOS uses `MKCoordinateRegion` (center + span); Android derives center + deltas from visible `LatLngBounds`. Values agree without rotation/pitch but may diverge when the map is tilted or rotated. | -| `onPress` / `onLongPress` | `Coordinate` | Map background only; marker taps do not also fire map `onPress`. | -| `onPoiPress` | `PoiPressEvent` | Provider-owned base-map POIs only. Apple Maps emits category data; Google Maps emits place ID. POI taps do not also fire map `onPress`. | -| `onMapReady` | none | Fires once after the map finishes loading tiles. | -| `Marker.onPress` / `onDragEnd` | none / `Coordinate` | Dispatched by overlay `id` from native to JS registry. | -| Overlay `onPress` | none | Polyline/polygon/circle children with `onPress` are sent as `tappable`; any other shape, bulk descriptors included, is untappable on every provider unless `tappable: true` is set. | -| `onClusterPress` | `string[]`, `Coordinate` | Fires when a marker cluster is tapped; IDs are member marker overlay ids. | +Map and overlay callbacks are wired through Nitro listeners on the HybridView. Callbacks receive payloads directly (e.g. `onPress(coordinate)`, `onRegionChange(region, details)`). + +| Callback | Payload | Notes | +| ------------------------------------------- | ------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `onRegionChange` / `onRegionChangeComplete` | `Region`, `RegionChangeDetails` | iOS uses `MKCoordinateRegion` (center + span); Android derives center + deltas from visible `LatLngBounds`. Values agree without rotation/pitch but may diverge when the map is tilted or rotated. Both fire for gestures and for programmatic moves; `details.isGesture` tells them apart. | +| `onPress` / `onLongPress` | `Coordinate` | Map background only; marker taps do not also fire map `onPress`. | +| `onPoiPress` | `PoiPressEvent` | Provider-owned base-map POIs only. Apple Maps emits category data; Google Maps emits place ID. POI taps do not also fire map `onPress`. | +| `onMapReady` | none | Fires once after the map finishes loading tiles. | +| `Marker.onPress` / `onDragEnd` | none / `Coordinate` | Dispatched by overlay `id` from native to JS registry. | +| Overlay `onPress` | none | Polyline/polygon/circle children with `onPress` are sent as `tappable`; any other shape, bulk descriptors included, is untappable on every provider unless `tappable: true` is set. | +| `onClusterPress` | `string[]`, `Coordinate` | Fires when a marker cluster is tapped; IDs are member marker overlay ids. | ### Advanced MapView props diff --git a/example/App.tsx b/example/App.tsx index 16c4158..6220b39 100644 --- a/example/App.tsx +++ b/example/App.tsx @@ -48,6 +48,7 @@ import { type OverlayEnteringAnimation, type PoiPressEvent, Region, + type RegionChangeDetails, } from 'react-native-better-maps'; import { APPLE_POI_DETAILS_DEFAULT_PRESENTATION, @@ -127,6 +128,11 @@ const springSoft = { damping: 20, stiffness: 240 }; const AnimatedPressable = Animated.createAnimatedComponent(Pressable); +function regionLabel(region: Region, details: RegionChangeDetails): string { + const source = details.isGesture ? 'gesture' : 'app'; + return `${region.latitude.toFixed(4)}, ${region.longitude.toFixed(4)} (${source})`; +} + function mergeMapPadding( padding: EdgePadding | undefined, showsScale: boolean, @@ -599,8 +605,11 @@ type MapSceneProps = { onPress: (coordinate: Coordinate) => void; onPoiPress: (event: PoiPressEvent) => void; onLongPress: (coordinate: Coordinate) => void; - onRegionChange: (region: Region) => void; - onRegionChangeComplete: (region: Region) => void; + onRegionChange: (region: Region, details: RegionChangeDetails) => void; + onRegionChangeComplete: ( + region: Region, + details: RegionChangeDetails, + ) => void; }; const MapScene = memo(function MapScene({ @@ -959,17 +968,19 @@ export default function App() { ); }, []); - const handleRegionChange = useCallback((region: Region) => { - setStatus( - `Region start · ${region.latitude.toFixed(4)}, ${region.longitude.toFixed(4)}`, - ); - }, []); + const handleRegionChange = useCallback( + (region: Region, details: RegionChangeDetails) => { + setStatus(`Region start · ${regionLabel(region, details)}`); + }, + [], + ); - const handleRegionChangeComplete = useCallback((region: Region) => { - setStatus( - `Region complete · ${region.latitude.toFixed(4)}, ${region.longitude.toFixed(4)}`, - ); - }, []); + const handleRegionChangeComplete = useCallback( + (region: Region, details: RegionChangeDetails) => { + setStatus(`Region complete · ${regionLabel(region, details)}`); + }, + [], + ); return ( diff --git a/package/android/src/main/java/com/margelo/nitro/nitromaps/CameraAnimations.kt b/package/android/src/main/java/com/margelo/nitro/nitromaps/CameraAnimations.kt new file mode 100644 index 0000000..ed271a7 --- /dev/null +++ b/package/android/src/main/java/com/margelo/nitro/nitromaps/CameraAnimations.kt @@ -0,0 +1,54 @@ +package com.margelo.nitro.nitromaps + +import com.google.android.gms.maps.GoogleMap + +/** + * Hands out the [GoogleMap.CancelableCallback] for each camera animation, and ends + * whichever are still running when the map goes away. + * + * The SDK reports how an animation ended - `onFinish` once the camera arrives, + * `onCancel` when a gesture or a later camera update cuts it short - but never that + * the map it ran on was destroyed, and a promise waiting on that report would hang + * for good. + * + * Main thread only, like the map it serves. + */ +internal class CameraAnimations { + private val running = mutableListOf() + + /** A callback that runs [onEnd] exactly once, however the animation ends. */ + fun callback(onEnd: () -> Unit): GoogleMap.CancelableCallback { + val animation = Animation(onEnd) + running += animation + return animation + } + + /** Ends every animation still running. */ + fun release() { + val ended = running.toList() + running.clear() + for (animation in ended) { + animation.end() + } + } + + private inner class Animation( + private val onEnd: () -> Unit, + ) : GoogleMap.CancelableCallback { + private var hasEnded = false + + override fun onFinish() = end() + + override fun onCancel() = end() + + fun end() { + if (hasEnded) { + return + } + + hasEnded = true + running -= this + onEnd() + } + } +} diff --git a/package/android/src/main/java/com/margelo/nitro/nitromaps/GoogleMapProviderAdapter.kt b/package/android/src/main/java/com/margelo/nitro/nitromaps/GoogleMapProviderAdapter.kt index dcbf73b..f106b08 100644 --- a/package/android/src/main/java/com/margelo/nitro/nitromaps/GoogleMapProviderAdapter.kt +++ b/package/android/src/main/java/com/margelo/nitro/nitromaps/GoogleMapProviderAdapter.kt @@ -36,7 +36,13 @@ class GoogleMapProviderAdapter( ) : MapProviderAdapter, LifecycleEventListener { private var googleMap: GoogleMap? = null - private var isUserGesture = false + private val regionChanges = + RegionChangeTracker( + position = { googleMap?.cameraPosition }, + region = { currentRegion() }, + onBegin = { region, details -> onRegionChange?.invoke(region, details) }, + onComplete = { region, details -> onRegionChangeComplete?.invoke(region, details) }, + ) private var hasFiredMapReady = false private val overlayController = MapOverlayController(context) private val locationSource = FusedLocationSource(context) @@ -47,6 +53,7 @@ class GoogleMapProviderAdapter( private var pendingCircles: Array? = null private val density: Float = context.resources.displayMetrics.density private val deferredMap = DeferredGoogleMap() + private val cameraAnimations = CameraAnimations() private var lastAppliedRegion: Region? = null private var lastAppliedRegionCamera: CameraPosition? = null @@ -118,7 +125,7 @@ class GoogleMapProviderAdapter( get() = _region set(value) { _region = value - if (value != null && !isUserGesture && _camera == null) { + if (value != null && !regionChanges.isGesture && _camera == null) { applyRegion(value) } } @@ -128,7 +135,7 @@ class GoogleMapProviderAdapter( get() = _camera set(value) { _camera = value - if (value != null && !isUserGesture) { + if (value != null && !regionChanges.isGesture) { applyCameraProp(value) } } @@ -267,8 +274,8 @@ class GoogleMapProviderAdapter( overlayController.clusterEnteringAnimation = value } - override var onRegionChange: ((region: Region) -> Unit)? = null - override var onRegionChangeComplete: ((region: Region) -> Unit)? = null + override var onRegionChange: ((region: Region, details: RegionChangeDetails) -> Unit)? = null + override var onRegionChangeComplete: ((region: Region, details: RegionChangeDetails) -> Unit)? = null override var onMapReady: (() -> Unit)? = null override var onPress: ((coordinate: Coordinate) -> Unit)? = null override var onPoiPress: ((event: NativePoiPressEvent) -> Unit)? = null @@ -352,8 +359,10 @@ class GoogleMapProviderAdapter( duration: Double?, ): Promise { val durationMs = cameraAnimationDurationMs(duration) - return deferredMap.promise { map -> - updateMapCamera(map, camera, animated = true, durationMs = durationMs) + return deferredMap.promiseCompletion { map, complete -> + updateMapCamera(map, camera, animated = true, durationMs = durationMs) { + complete(Result.success(Unit)) + } } } @@ -368,16 +377,8 @@ class GoogleMapProviderAdapter( } val durationMs = cameraAnimationDurationMs(duration) - return deferredMap.promiseCompletion { map, complete -> - // Waits for the first layout pass, as `fitToCoordinates` does, and so does the promise. - runWhenMapViewLaidOut( - onCancel = { - complete(Result.failure(IllegalStateException(MAP_RELEASED_BEFORE_LAYOUT_MESSAGE))) - }, - ) { - complete(runCatching { fitCamera(map, region, durationMs) }) - } - } + // Waits for the first layout pass, as `fitToCoordinates` does, and so does the promise. + return promiseMoveWhenLaidOut { map, onEnd -> fitCamera(map, region, durationMs, onEnd) } } override fun getVisibleRegion(): Promise = deferredMap.promise { map -> map.projection.toNitroVisibleRegion() } @@ -400,7 +401,7 @@ class GoogleMapProviderAdapter( // `newLatLngBounds` throws on a map that has no size yet, so the camera update // waits for the first layout pass -- and so does the promise. - return promiseWhenLaidOut { map -> + return promiseMoveWhenLaidOut { map, onEnd -> val builder = LatLngBounds.Builder() for (coordinate in validCoordinates) { builder.include(LatLng(coordinate.latitude, coordinate.longitude)) @@ -417,9 +418,10 @@ class GoogleMapProviderAdapter( ) ?: bounds val update = CameraUpdateFactory.newLatLngBounds(target, 0) if (animated == true) { - map.animateCamera(update) + map.animateCamera(update, cameraAnimations.callback(onEnd)) } else { map.moveCamera(update) + onEnd() } } } @@ -491,16 +493,17 @@ class GoogleMapProviderAdapter( syncMarkerPressHandlers() map.setOnCameraMoveStartedListener { reason -> - handleRegionWillChange( - userInteracting = reason == GoogleMap.OnCameraMoveStartedListener.REASON_GESTURE, + regionChanges.moveStarted( + isGesture = reason == GoogleMap.OnCameraMoveStartedListener.REASON_GESTURE, ) } map.setOnCameraMoveListener { overlayController.onCameraMove() + regionChanges.cameraMoved() } map.setOnCameraIdleListener { overlayController.onCameraIdle() - handleRegionDidChange() + regionChanges.cameraStopped() } map.setOnMapClickListener { latLng -> onPress?.invoke(latLng.toCoordinate()) @@ -667,13 +670,15 @@ class GoogleMapProviderAdapter( /** * Frames [region], animating over [durationMs] when it is positive and jumping there - * otherwise: `animateCamera` throws for a duration that is not. Needs a laid-out map + * otherwise: `animateCamera` throws for a duration that is not. Calls [onEnd] once the + * camera has stopped - straight away when nothing had to move. Needs a laid-out map * view, since `newLatLngBounds` throws on one without a size. */ private fun fitCamera( map: GoogleMap, region: Region, durationMs: Int, + onEnd: () -> Unit = {}, ) { val lastRegion = lastAppliedRegion val lastCamera = lastAppliedRegionCamera @@ -685,6 +690,15 @@ class GoogleMapProviderAdapter( ) { // Same region as last time and the camera has not moved since, so the // fit would land on the camera the map already shows. + onEnd() + return + } + + // What the map already shows is exactly what an `onRegionChangeComplete` consumer + // hands back as the next `region` prop. Fitting it again must not move the camera, + // or the echo would answer every move with another one. + if (currentRegion().approximatelyEquals(region)) { + onEnd() return } @@ -692,21 +706,28 @@ class GoogleMapProviderAdapter( // leaves over, so passing `mapPadding` here as well would inset the region twice. val update = CameraUpdateFactory.newLatLngBounds(region.toLatLngBounds(), 0) if (durationMs > 0) { - map.animateCamera(update, durationMs, null) + map.animateCamera(update, durationMs, cameraAnimations.callback(onEnd)) // The camera settles later; there is nothing reliable to remember yet. lastAppliedRegionCamera = null } else { map.moveCamera(update) lastAppliedRegionCamera = map.cameraPosition + onEnd() } lastAppliedRegion = region } + /** + * Moves the camera to [camera], and calls [onEnd] once it has stopped: straight away + * after a jump, or when nothing had to move, and otherwise when the animation finishes + * or is cut short. + */ private fun updateMapCamera( map: GoogleMap, camera: Camera, animated: Boolean, durationMs: Int = 0, + onEnd: () -> Unit = {}, ) { // Every camera path ends here - the `camera` prop, its replay in `configureMap`, and // `applyCamera`/`animateCamera` - so this one check covers them all. An invalid camera is @@ -716,22 +737,26 @@ class GoogleMapProviderAdapter( // before the call is queued. if (!camera.isValid()) { Log.w(NITRO_MAPS_LOG_TAG, "Ignored an invalid camera: $camera.") + onEnd() return } val target = camera.toCameraPosition(map.cameraPosition) if (map.cameraPosition.approximatelyEquals(target)) { + onEnd() return } val update = CameraUpdateFactory.newCameraPosition(target) // A duration under a millisecond jumps, as it does on iOS: the timed `animateCamera` throws // for it, and the untimed one would animate for the SDK's own default duration. - if (animated && durationMs > 0) { - map.animateCamera(update, durationMs, null) - } else { + if (!animated || durationMs <= 0) { map.moveCamera(update) + onEnd() + return } + + map.animateCamera(update, durationMs, cameraAnimations.callback(onEnd)) } private fun installViewportSizeListener(mapView: MapView) { @@ -766,6 +791,22 @@ class GoogleMapProviderAdapter( } } + /** + * Like [promiseWhenLaidOut], for a camera move that ends in a later SDK callback: the + * promise settles when [block] calls the `onEnd` it is handed, not when it returns. + */ + private fun promiseMoveWhenLaidOut(block: (GoogleMap, onEnd: () -> Unit) -> Unit): Promise = + deferredMap.promiseCompletion { map, complete -> + runWhenMapViewLaidOut( + onCancel = { + complete(Result.failure(IllegalStateException(MAP_RELEASED_BEFORE_LAYOUT_MESSAGE))) + }, + ) { + runCatching { block(map) { complete(Result.success(Unit)) } } + .onFailure { error -> complete(Result.failure(error)) } + } + } + /** * Runs [block] once the map view has a size - see [DeferredLayout]. [onCancel] runs * instead if the map is destroyed before that. @@ -784,29 +825,6 @@ class GoogleMapProviderAdapter( overlayController.setViewportSize(view.width, view.height) } - private fun handleRegionWillChange(userInteracting: Boolean) { - if (userInteracting && !isUserGesture) { - isUserGesture = true - emitRegionChange(complete = false) - } - } - - private fun handleRegionDidChange() { - if (isUserGesture) { - emitRegionChange(complete = true) - isUserGesture = false - } - } - - private fun emitRegionChange(complete: Boolean) { - val region = currentRegion() - if (complete) { - onRegionChangeComplete?.invoke(region) - } else { - onRegionChange?.invoke(region) - } - } - private fun currentRegion(): Region { val bounds = googleMap?.projection?.visibleRegion?.latLngBounds if (bounds != null) { @@ -857,6 +875,9 @@ class GoogleMapProviderAdapter( private fun destroyMapView() { deferredMap.release() deferredLayout.release() + // A call that never reached the SDK rejects above. An animation already under way + // ends here instead, like any other move cut short, and resolves. + cameraAnimations.release() if (lifecycle.isDestroyed) { return diff --git a/package/android/src/main/java/com/margelo/nitro/nitromaps/HybridMapView.kt b/package/android/src/main/java/com/margelo/nitro/nitromaps/HybridMapView.kt index cf7daef..97a3eaa 100644 --- a/package/android/src/main/java/com/margelo/nitro/nitromaps/HybridMapView.kt +++ b/package/android/src/main/java/com/margelo/nitro/nitromaps/HybridMapView.kt @@ -189,13 +189,13 @@ class HybridMapView( adapter?.clusterEnteringAnimation = value } - override var onRegionChange: ((region: Region) -> Unit)? = null + override var onRegionChange: ((region: Region, details: RegionChangeDetails) -> Unit)? = null set(value) { field = value adapter?.onRegionChange = value } - override var onRegionChangeComplete: ((region: Region) -> Unit)? = null + override var onRegionChangeComplete: ((region: Region, details: RegionChangeDetails) -> Unit)? = null set(value) { field = value adapter?.onRegionChangeComplete = value diff --git a/package/android/src/main/java/com/margelo/nitro/nitromaps/MapProviderAdapter.kt b/package/android/src/main/java/com/margelo/nitro/nitromaps/MapProviderAdapter.kt index cc149d6..fe1cd74 100644 --- a/package/android/src/main/java/com/margelo/nitro/nitromaps/MapProviderAdapter.kt +++ b/package/android/src/main/java/com/margelo/nitro/nitromaps/MapProviderAdapter.kt @@ -24,8 +24,8 @@ interface MapProviderAdapter { var markerEnteringAnimation: OverlayEnteringAnimationDescriptor? var clusterEnteringAnimation: OverlayEnteringAnimationDescriptor? - var onRegionChange: ((region: Region) -> Unit)? - var onRegionChangeComplete: ((region: Region) -> Unit)? + var onRegionChange: ((region: Region, details: RegionChangeDetails) -> Unit)? + var onRegionChangeComplete: ((region: Region, details: RegionChangeDetails) -> Unit)? var onMapReady: (() -> Unit)? var onPress: ((coordinate: Coordinate) -> Unit)? var onPoiPress: ((event: NativePoiPressEvent) -> Unit)? @@ -45,14 +45,24 @@ interface MapProviderAdapter { fun fetchCamera(): Promise + /** + * Resolves once the camera has arrived: a non-animated move right away, an animated + * one when it finishes or when a gesture, a later command or [release] cuts it short. + * A call made before the map exists waits for it, and rejects if [release] comes first. + */ fun applyCamera(camera: Camera): Promise - /** [duration] is in milliseconds, here and in [animateToRegion], as JS passes it. */ + /** + * [duration] is in milliseconds, here and in [animateToRegion], as JS passes it. + * + * @see applyCamera for when the promise settles. + */ fun animateCamera( camera: Camera, duration: Double?, ): Promise + /** @see applyCamera for when the promise settles. */ fun animateToRegion( region: Region, duration: Double?, @@ -60,6 +70,7 @@ interface MapProviderAdapter { fun getVisibleRegion(): Promise + /** @see applyCamera for when the promise settles. */ fun fitToCoordinates( coordinates: Array, padding: EdgePadding?, diff --git a/package/android/src/main/java/com/margelo/nitro/nitromaps/RegionChangeTracker.kt b/package/android/src/main/java/com/margelo/nitro/nitromaps/RegionChangeTracker.kt new file mode 100644 index 0000000..f753d86 --- /dev/null +++ b/package/android/src/main/java/com/margelo/nitro/nitromaps/RegionChangeTracker.kt @@ -0,0 +1,94 @@ +package com.margelo.nitro.nitromaps + +/** + * Turns the Google Maps camera callbacks into one `onRegionChange` / + * `onRegionChangeComplete` pair per move of the camera. + * + * The SDK is noisier than that. It reports a move starting again whenever the + * reason for it changes, and it reports a start and an idle for a camera update + * that leaves the camera exactly where it was - a repeated fit, or a `region` + * prop sent again with the same values, which an inline object literal does on + * every render. So a move is announced only once the camera has left the place + * it last came to rest, and a move that never did announces nothing. Otherwise a + * handler that re-renders the map would be answered by a move that re-renders it + * again, for ever. Until the camera has come to rest once there is nothing to + * measure against - the map is still settling into its first position - so that + * is not announced either. + * + * `package/ios/Camera/RegionChangeTracker.swift` is the same class for the iOS + * adapters. Main thread only, like the map callbacks that drive it. + */ +internal class RegionChangeTracker( + private val position: () -> Position, + private val region: () -> Region, + private val onBegin: (Region, RegionChangeDetails) -> Unit, + private val onComplete: (Region, RegionChangeDetails) -> Unit, +) { + private class Move( + val details: RegionChangeDetails, + /** `null` while the map is still settling into its first position. */ + val start: Resting?, + ) { + var hasBegun = false + } + + private class Resting( + val position: Position, + val region: Region, + ) + + private var move: Move? = null + private var rest: Resting? = null + + /** True while the finger, rather than the app, is driving the camera. */ + val isGesture: Boolean + get() = move?.details?.isGesture == true + + /** The SDK says the camera is about to move: `onCameraMoveStarted`. */ + fun moveStarted(isGesture: Boolean) { + val current = move + if (current == null) { + move = Move(RegionChangeDetails(isGesture = isGesture), rest) + return + } + + // Still the same move - unless a gesture takes over from the app's own, which + // ends the app's move where the finger caught the camera and starts the user's there. + if (!isGesture || current.details.isGesture) { + return + } + + finish(current) + move = Move(RegionChangeDetails(isGesture = true), Resting(position(), region())) + } + + /** The SDK says the camera has moved on a step: `onCameraMove`. */ + fun cameraMoved() { + move?.let(::beginIfMoved) + } + + /** The SDK says the camera has come to rest: `onCameraIdle`. */ + fun cameraStopped() { + move?.let(::finish) + rest = Resting(position(), region()) + } + + private fun finish(current: Move) { + move = null + // A jump can come to rest without a single step reported in between. + beginIfMoved(current) + if (current.hasBegun) { + onComplete(region(), current.details) + } + } + + private fun beginIfMoved(current: Move) { + val start = current.start ?: return + if (current.hasBegun || position() == start.position) { + return + } + + current.hasBegun = true + onBegin(start.region, current.details) + } +} diff --git a/package/android/src/test/java/com/margelo/nitro/nitromaps/RegionApproximateEqualityTest.kt b/package/android/src/test/java/com/margelo/nitro/nitromaps/RegionApproximateEqualityTest.kt new file mode 100644 index 0000000..62454f6 --- /dev/null +++ b/package/android/src/test/java/com/margelo/nitro/nitromaps/RegionApproximateEqualityTest.kt @@ -0,0 +1,48 @@ +package com.margelo.nitro.nitromaps + +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * `applyRegion` compares the requested region against the one the map already shows, so + * a consumer that echoes `onRegionChangeComplete` back into the `region` prop does not + * start a re-fit loop. These pin down what "already shows it" means. + */ +class RegionApproximateEqualityTest { + @Test + fun acceptsTheSameRegion() { + assertTrue(region().approximatelyEquals(region())) + } + + @Test + fun acceptsADifferenceBelowTheEpsilon() { + assertTrue(region().approximatelyEquals(region(latitude = 52.23 + 1e-9))) + assertTrue(region().approximatelyEquals(region(longitudeDelta = 0.1 + 1e-9))) + } + + @Test + fun rejectsAMovedCenter() { + assertFalse(region().approximatelyEquals(region(latitude = 52.24))) + assertFalse(region().approximatelyEquals(region(longitude = 21.02))) + } + + @Test + fun rejectsAResizedSpan() { + assertFalse(region().approximatelyEquals(region(latitudeDelta = 0.2))) + assertFalse(region().approximatelyEquals(region(longitudeDelta = 0.2))) + } + + private fun region( + latitude: Double = 52.23, + longitude: Double = 21.01, + latitudeDelta: Double = 0.1, + longitudeDelta: Double = 0.1, + ): Region = + Region( + latitude = latitude, + longitude = longitude, + latitudeDelta = latitudeDelta, + longitudeDelta = longitudeDelta, + ) +} diff --git a/package/android/src/test/java/com/margelo/nitro/nitromaps/RegionChangeTrackerTest.kt b/package/android/src/test/java/com/margelo/nitro/nitromaps/RegionChangeTrackerTest.kt new file mode 100644 index 0000000..97bacd0 --- /dev/null +++ b/package/android/src/test/java/com/margelo/nitro/nitromaps/RegionChangeTrackerTest.kt @@ -0,0 +1,188 @@ +package com.margelo.nitro.nitromaps + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * Drives [RegionChangeTracker] with the callback sequences the Google Maps SDK was + * seen to produce on an emulator, with an integer standing in for the camera position. + * `package/ios/Tests/Camera/RegionChangeTrackerTests.swift` runs the same cases. + */ +class RegionChangeTrackerTest { + private var position = 0 + private val events = mutableListOf() + + private val tracker = + RegionChangeTracker( + position = { position }, + region = { regionAt(position) }, + onBegin = { region, details -> events += event("begin", region, details) }, + onComplete = { region, details -> events += event("complete", region, details) }, + ) + + /** A map that has settled into its first position, as every case but one assumes. */ + private fun settle() { + tracker.cameraStopped() + } + + @Test + fun anAnimationEmitsOneBeginAndOneComplete() { + settle() + tracker.moveStarted(isGesture = false) + position = 1 + tracker.cameraMoved() + position = 2 + tracker.cameraMoved() + tracker.cameraStopped() + + assertEquals(listOf("begin app @0", "complete app @2"), events) + } + + @Test + fun aJumpWithNoMoveCallbackStillEmitsBothEvents() { + settle() + tracker.moveStarted(isGesture = false) + position = 5 + tracker.cameraStopped() + + assertEquals(listOf("begin app @0", "complete app @5"), events) + } + + @Test + fun measuresAMoveFromWhereTheCameraLastCameToRest() { + // MapKit already reports the destination when the app sets the camera. + settle() + position = 4 + tracker.moveStarted(isGesture = false) + tracker.cameraStopped() + + assertEquals(listOf("begin app @0", "complete app @4"), events) + } + + @Test + fun anUpdateThatLeavesTheCameraInPlaceEmitsNothing() { + // A repeated fit, or a `region` prop sent again with the same values. + settle() + tracker.moveStarted(isGesture = false) + tracker.cameraMoved() + tracker.cameraStopped() + + assertEquals(emptyList(), events) + } + + @Test + fun theMapSettlingIntoItsFirstPositionEmitsNothing() { + tracker.moveStarted(isGesture = false) + position = 7 + tracker.cameraMoved() + tracker.cameraStopped() + assertEquals(emptyList(), events) + + // From there on, moves are measured against it. + tracker.moveStarted(isGesture = false) + position = 8 + tracker.cameraStopped() + assertEquals(listOf("begin app @7", "complete app @8"), events) + } + + @Test + fun aSecondStartForTheSameMoveIsIgnored() { + // One animation interrupting another: the SDK does not go idle in between. + settle() + tracker.moveStarted(isGesture = false) + position = 1 + tracker.cameraMoved() + tracker.moveStarted(isGesture = false) + position = 2 + tracker.cameraMoved() + tracker.cameraStopped() + + assertEquals(listOf("begin app @0", "complete app @2"), events) + } + + @Test + fun aGestureTakingOverEndsTheAppsMoveAndStartsItsOwn() { + settle() + tracker.moveStarted(isGesture = false) + position = 1 + tracker.cameraMoved() + tracker.moveStarted(isGesture = true) + position = 2 + tracker.cameraMoved() + tracker.cameraStopped() + + assertEquals( + listOf("begin app @0", "complete app @1", "begin gesture @1", "complete gesture @2"), + events, + ) + } + + @Test + fun aGestureTakingOverBeforeTheCameraMovedLeavesOnlyTheGesture() { + settle() + tracker.moveStarted(isGesture = false) + tracker.moveStarted(isGesture = true) + position = 3 + tracker.cameraMoved() + tracker.cameraStopped() + + assertEquals(listOf("begin gesture @0", "complete gesture @3"), events) + } + + @Test + fun theAppTakingOverFromAGestureStaysOneGestureMove() { + settle() + tracker.moveStarted(isGesture = true) + position = 1 + tracker.cameraMoved() + tracker.moveStarted(isGesture = false) + position = 2 + tracker.cameraMoved() + tracker.cameraStopped() + + assertEquals(listOf("begin gesture @0", "complete gesture @2"), events) + } + + @Test + fun reportsAGestureOnlyWhileOneIsUnderWay() { + settle() + assertFalse(tracker.isGesture) + + tracker.moveStarted(isGesture = true) + assertTrue(tracker.isGesture) + + tracker.cameraStopped() + assertFalse(tracker.isGesture) + + tracker.moveStarted(isGesture = false) + assertFalse(tracker.isGesture) + } + + @Test + fun ignoresStepsOutsideAMove() { + settle() + position = 1 + tracker.cameraMoved() + + assertEquals(emptyList(), events) + } + + private fun event( + kind: String, + region: Region, + details: RegionChangeDetails, + ): String { + val source = if (details.isGesture) "gesture" else "app" + return "$kind $source @${region.latitude.toInt()}" + } + + private fun regionAt(position: Int): Region = + Region( + latitude = position.toDouble(), + longitude = 0.0, + latitudeDelta = 1.0, + longitudeDelta = 1.0, + ) +} diff --git a/package/ios/AppleMapProviderAdapter.swift b/package/ios/AppleMapProviderAdapter.swift index d542d09..ccc31cd 100644 --- a/package/ios/AppleMapProviderAdapter.swift +++ b/package/ios/AppleMapProviderAdapter.swift @@ -3,10 +3,27 @@ import NitroModules import UIKit final class AppleMapProviderAdapter: MapProviderAdapter { + /// MapKit animates `setVisibleMapRect` over a duration it does not publish. This is + /// only the watchdog's guess at it, not a duration handed to MapKit. + private static let fitAnimationDuration: TimeInterval = 0.3 private static let defaultAnimationDuration: TimeInterval = 0.25 private let mapViewDelegate = HybridMapViewDelegate() - private var isUserRegionChange = false + private let cameraMoves = CameraMoveTracker() + private lazy var regionChanges = RegionChangeTracker( + position: { [unowned self] in view.camera.placement }, + region: { [unowned self] in currentRegion() }, + onBegin: { [unowned self] region, isGesture in + onRegionChange?(region, RegionChangeDetails(isGesture: isGesture)) + }, + onComplete: { [unowned self] region, isGesture in + onRegionChangeComplete?(region, RegionChangeDetails(isGesture: isGesture)) + } + ) + /// Whether MapKit is between a `regionWillChange` and its `regionDidChange`. + private var isRegionChanging = false + /// The end of a move, held back for a turn of the run loop - see `handleRegionDidChange()`. + private var pendingRegionChangeEnd: DispatchWorkItem? private var isMapReady = false private var hasDeliveredMapReady = false private var liveClusterTimer: Timer? @@ -165,8 +182,8 @@ final class AppleMapProviderAdapter: MapProviderAdapter { } } - var onRegionChange: ((Region) -> Void)? - var onRegionChangeComplete: ((Region) -> Void)? + var onRegionChange: ((Region, RegionChangeDetails) -> Void)? + var onRegionChangeComplete: ((Region, RegionChangeDetails) -> Void)? var onMapReady: (() -> Void)? { didSet { deliverMapReadyIfPossible() @@ -224,13 +241,18 @@ final class AppleMapProviderAdapter: MapProviderAdapter { Promise.resolved(withResult: view.camera.toCamera()) } - func applyCamera(camera: Camera) throws { + func applyCamera(camera: Camera) throws -> Promise { updateMapCamera(camera, animated: false) + return Promise.resolved() } - func animateCamera(camera: Camera, duration: TimeInterval?) throws { + func animateCamera(camera: Camera, duration: TimeInterval?) throws -> Promise { let animationDuration = duration ?? Self.defaultAnimationDuration - updateMapCamera(camera, animated: true, duration: animationDuration) + let promise = Promise() + trackAnimation(settling: promise, duration: animationDuration) { completion in + updateMapCamera(camera, animated: true, duration: animationDuration, completion: completion) + } + return promise } func animateToRegion(region: Region, duration: TimeInterval?) throws -> Promise { @@ -245,16 +267,22 @@ final class AppleMapProviderAdapter: MapProviderAdapter { // honours the animation's duration at any distance, so the region goes in // as the camera `setRegion` would pick for it - which takes a map with a // size. - return whenLaidOut { [weak self] in + let animationDuration = duration ?? Self.defaultAnimationDuration + let promise = Promise() + whenLaidOut(rejecting: promise) { [weak self] in guard let self else { return } - self.moveMapCamera( - to: self.view.camera(framing: region.toMKCoordinateRegion()), - animated: true, - duration: duration ?? Self.defaultAnimationDuration - ) + self.trackAnimation(settling: promise, duration: animationDuration) { completion in + self.moveMapCamera( + to: self.view.camera(framing: region.toMKCoordinateRegion()), + animated: true, + duration: animationDuration, + completion: completion + ) + } } + return promise } func getVisibleRegion() throws -> Promise { @@ -265,10 +293,10 @@ final class AppleMapProviderAdapter: MapProviderAdapter { coordinates: [Coordinate], padding: EdgePadding?, animated: Bool? - ) throws { + ) throws -> Promise { let validCoordinates = coordinates.filter { $0.isValid } guard !validCoordinates.isEmpty else { - return + return Promise.resolved() } var mapRect = MKMapRect.null @@ -284,12 +312,22 @@ final class AppleMapProviderAdapter: MapProviderAdapter { } let edgePadding = padding?.toUIEdgeInsets() ?? .zero - let shouldAnimate = animated ?? true - view.setVisibleMapRect( - mapRect, - edgePadding: edgePadding, - animated: shouldAnimate - ) + view.setVisibleMapRect(mapRect, edgePadding: edgePadding, animated: animated ?? true) + + // MapKit reports from inside that call: the end of any move this one cut short, the + // start of this one, and its end too when it jumps rather than animates - which it + // does for a rect far from the one on screen, `animated` or not. A rect it already + // shows it ignores without a word. Only a fit still under way has anything left to + // wait for. + guard isRegionChanging else { + return Promise.resolved() + } + + // `setVisibleMapRect` takes no completion handler: the `regionDidChange` that ends + // the move settles it. + let promise = Promise() + cameraMoves.track(promise, duration: Self.fitAnimationDuration) + return promise } func pointForCoordinate(coordinate: Coordinate) throws -> Promise { @@ -322,22 +360,41 @@ final class AppleMapProviderAdapter: MapProviderAdapter { view.setRegion(targetRegion, animated: animated) } - func updateMapCamera(_ camera: Camera, animated: Bool, duration: Double = 0) { + /// Moves the camera, and reports whether anything was handed to MapKit: nothing is for + /// an invalid camera, or for the one the map is already at. + @discardableResult + func updateMapCamera( + _ camera: Camera, + animated: Bool, + duration: Double = 0, + completion: (() -> Void)? = nil + ) -> Bool { // `setCamera` raises an Objective-C NSException - `Invalid camera // centerCoordinate` - from `-[MKMapCamera _validate]` for a center MapKit // cannot place, and Swift cannot catch that. The framing values do not // raise, but a non-finite one collapses the altitude or leaves // `view.region` reading back as `NaN`. guard camera.isValid else { - return + return false } - moveMapCamera(to: camera.toMKMapCamera(), animated: animated, duration: duration) + return moveMapCamera( + to: camera.toMKMapCamera(), + animated: animated, + duration: duration, + completion: completion + ) } - private func moveMapCamera(to mapCamera: MKMapCamera, animated: Bool, duration: Double) { + @discardableResult + private func moveMapCamera( + to mapCamera: MKMapCamera, + animated: Bool, + duration: Double, + completion: (() -> Void)? = nil + ) -> Bool { guard !view.camera.approximatelyEquals(mapCamera) else { - return + return false } if animated { @@ -345,33 +402,63 @@ final class AppleMapProviderAdapter: MapProviderAdapter { withDuration: duration, animations: { self.view.camera = mapCamera + }, + completion: { _ in + completion?() } ) } else { view.camera = mapCamera } + + return true } - /// Runs `work` once the map view has a size - now, or in the first layout - /// pass that gives it one - and resolves then, as Android does for the fits - /// that need a size. Rejects if the adapter is released before that pass. - private func whenLaidOut(_ work: @escaping () -> Void) -> Promise { + /// Runs `work` once the map view has a size - now, or in the first layout pass that + /// gives it one, as Android does for the fits that need a size. `promise` rejects + /// instead if the adapter is released before that pass. + private func whenLaidOut(rejecting promise: Promise, _ work: @escaping () -> Void) { guard view.bounds.isEmpty else { work() - return Promise.resolved() + return } - let promise = Promise() workAwaitingLayout.append { result in switch result { case .success: work() - promise.resolve() case .failure(let error): promise.reject(withError: error) } } - return promise + } + + /// Settles `promise` once the camera animation `handOver` starts has stopped. + /// `handOver` passes the completion it is given on to `UIView.animate`, and reports + /// whether it handed anything to MapKit. + private func trackAnimation( + settling promise: Promise, + duration: TimeInterval, + handOver: (_ completion: @escaping () -> Void) -> Bool + ) { + let move = CameraMoveCompletion(promise: promise) + + // UIKit calls the completion when the animation runs out, and straight away when a + // later camera animation replaces it. + let didMove = handOver { move.settle() } + + // Nothing handed over - an invalid camera, or the one the map is already at - or a + // camera MapKit applied without animating, as it does for a map that is not on + // screen yet: either way there is nothing left to wait for. + guard didMove, isRegionChanging else { + move.settle() + return + } + + // Also settled when MapKit reports the camera at rest: a fit or a `region` update + // that cuts this animation short does not end it for UIKit, whose completion then + // waits out the full duration. + cameraMoves.track(move, duration: duration) } private func runWorkAwaitingLayout() { @@ -410,35 +497,37 @@ final class AppleMapProviderAdapter: MapProviderAdapter { func handleRegionWillChange(userInteracting: Bool) { startLiveClustering() - guard userInteracting, !isUserRegionChange else { - return - } - isUserRegionChange = true - emitRegionChange(complete: false) + isRegionChanging = true + pendingRegionChangeEnd?.cancel() + pendingRegionChangeEnd = nil + regionChanges.moveStarted(isGesture: userInteracting) + } + + func handleVisibleRegionChange() { + regionChanges.cameraMoved() } func handleRegionDidChange() { stopLiveClustering() + isRegionChanging = false + cameraMoves.settleAll() - guard isUserRegionChange else { - return - } - + // MapKit reports the end of each leg of a gesture, including the ones the + // finger is still driving. The move is only over once it lets go. guard !view.isUserInteracting else { return } - emitRegionChange(complete: true) - isUserRegionChange = false - } - - private func emitRegionChange(complete: Bool) { - let region = currentRegion() - if complete { - onRegionChangeComplete?(region) - } else { - onRegionChange?(region) + // A camera command that cuts an animation short makes MapKit end that move and + // start the next back to back, from inside the command. Google Maps reports the + // same thing as one move, so the end waits a turn of the run loop, and a move that + // starts in the meantime carries on the one before. + let end = DispatchWorkItem { [weak self] in + self?.pendingRegionChangeEnd = nil + self?.regionChanges.cameraStopped() } + pendingRegionChangeEnd = end + DispatchQueue.main.async(execute: end) } func startLiveClustering() { @@ -527,7 +616,11 @@ final class AppleMapProviderAdapter: MapProviderAdapter { settleWorkAwaitingLayout(with: .failure(Self.releasedBeforeLayoutError())) liveClusterTimer?.invalidate() liveClusterTimer = nil - isUserRegionChange = false + cameraMoves.settleAll() + pendingRegionChangeEnd?.cancel() + pendingRegionChangeEnd = nil + isRegionChanging = false + regionChanges.reset() isMapReady = false hasDeliveredMapReady = false onRegionChange = nil diff --git a/package/ios/Camera+GMSCameraPosition.swift b/package/ios/Camera+GMSCameraPosition.swift index 06f2529..f2a9d5d 100644 --- a/package/ios/Camera+GMSCameraPosition.swift +++ b/package/ios/Camera+GMSCameraPosition.swift @@ -28,6 +28,16 @@ extension GMSCameraPosition { && abs(viewingAngle - other.viewingAngle) < angleEpsilon } + var placement: CameraPlacement { + CameraPlacement( + latitude: target.latitude, + longitude: target.longitude, + scale: Double(zoom), + heading: bearing, + pitch: viewingAngle + ) + } + func toCamera() -> Camera { Camera( center: Coordinate(latitude: target.latitude, longitude: target.longitude), diff --git a/package/ios/Camera+MKMapCamera.swift b/package/ios/Camera+MKMapCamera.swift index 78ef4fb..013b418 100644 --- a/package/ios/Camera+MKMapCamera.swift +++ b/package/ios/Camera+MKMapCamera.swift @@ -39,6 +39,16 @@ extension MKMapCamera { && abs(pitch - other.pitch) < angleEpsilon } + var placement: CameraPlacement { + CameraPlacement( + latitude: centerCoordinate.latitude, + longitude: centerCoordinate.longitude, + scale: centerCoordinateDistance, + heading: heading, + pitch: Double(pitch) + ) + } + func toCamera() -> Camera { let centerCoordinate = Coordinate( latitude: centerCoordinate.latitude, diff --git a/package/ios/Camera/CameraPlacement.swift b/package/ios/Camera/CameraPlacement.swift new file mode 100644 index 0000000..e02b403 --- /dev/null +++ b/package/ios/Camera/CameraPlacement.swift @@ -0,0 +1,17 @@ +import Foundation + +/// Where a map camera is, as a plain value that both SDKs' cameras reduce to. +/// +/// `MKMapCamera` and `GMSCameraPosition` are classes, and `MKMapView.camera` hands out a +/// fresh copy on every read, so two reads of a camera that has not moved are never `==` +/// as objects. Compared exactly on purpose: the question is whether the camera moved at +/// all, not whether it moved far. +struct CameraPlacement: Equatable { + let latitude: Double + let longitude: Double + /// The distance to the ground for MapKit, the zoom level for Google Maps. Only ever + /// compared with a placement from the same SDK. + let scale: Double + let heading: Double + let pitch: Double +} diff --git a/package/ios/Camera/RegionChangeTracker.swift b/package/ios/Camera/RegionChangeTracker.swift new file mode 100644 index 0000000..225694e --- /dev/null +++ b/package/ios/Camera/RegionChangeTracker.swift @@ -0,0 +1,108 @@ +import Foundation + +/// Turns a map SDK's camera callbacks into one `onRegionChange` / +/// `onRegionChangeComplete` pair per move of the camera. +/// +/// The SDKs are noisier than that. Google Maps reports a start and an idle for a +/// camera update that leaves the camera exactly where it was - a repeated fit, or a +/// `region` prop sent again with the same values, which an inline object literal does +/// on every render. So a move is announced only once the camera has left the place it +/// last came to rest, and a move that never did announces nothing. Otherwise a handler +/// that re-renders the map would be answered by a move that re-renders it again, for +/// ever. +/// +/// The place it last came to rest, rather than wherever the SDK says the camera is when +/// a move starts: MapKit already reports the destination by then when the app sets the +/// camera. Until the camera has come to rest once there is nothing to measure against - +/// the map is still settling into its first position - so that is not announced either. +/// +/// Generic over the position and the region so it builds without MapKit, Google Maps +/// or the Nitro-generated types. The Android adapter has the same class in Kotlin. +/// Main thread only, like the delegate callbacks that drive it. +final class RegionChangeTracker { + private struct Move { + let isGesture: Bool + /// `nil` while the map is still settling into its first position. + let start: (position: Position, region: Region)? + var hasBegun = false + } + + private let position: () -> Position + private let region: () -> Region + private let onBegin: (Region, _ isGesture: Bool) -> Void + private let onComplete: (Region, _ isGesture: Bool) -> Void + private var move: Move? + private var rest: (position: Position, region: Region)? + + init( + position: @escaping () -> Position, + region: @escaping () -> Region, + onBegin: @escaping (Region, _ isGesture: Bool) -> Void, + onComplete: @escaping (Region, _ isGesture: Bool) -> Void + ) { + self.position = position + self.region = region + self.onBegin = onBegin + self.onComplete = onComplete + } + + /// True while the finger, rather than the app, is driving the camera. + var isGesture: Bool { + move?.isGesture == true + } + + /// The SDK says the camera is about to move. + func moveStarted(isGesture: Bool) { + guard let current = move else { + move = Move(isGesture: isGesture, start: rest) + return + } + + // Still the same move - unless a gesture takes over from the app's own, which ends + // the app's move where the finger caught the camera and starts the user's there. + guard isGesture, !current.isGesture else { + return + } + + finish() + move = Move(isGesture: true, start: (position(), region())) + } + + /// The SDK says the camera has moved on a step. + func cameraMoved() { + guard let current = move, !current.hasBegun, let start = current.start, + position() != start.position + else { + return + } + + move?.hasBegun = true + onBegin(start.region, current.isGesture) + } + + /// The SDK says the camera has come to rest. + func cameraStopped() { + finish() + rest = (position(), region()) + } + + /// Forgets the move under way without reporting it, and where the camera last came to + /// rest: the map view is about to show a different map. + func reset() { + move = nil + rest = nil + } + + private func finish() { + // A jump can come to rest without a single step reported in between. + cameraMoved() + guard let current = move else { + return + } + + move = nil + if current.hasBegun { + onComplete(region(), current.isGesture) + } + } +} diff --git a/package/ios/CameraMoveTracker.swift b/package/ios/CameraMoveTracker.swift new file mode 100644 index 0000000..2bcd941 --- /dev/null +++ b/package/ios/CameraMoveTracker.swift @@ -0,0 +1,101 @@ +import Foundation +import NitroModules + +/// The Promise of one camera move, settled exactly once. +/// +/// A move ends in more ways than it begins: its animation runs out, the map reports the +/// camera has come to rest, a gesture or a later command cuts it short, or the view goes +/// away first. Whichever signal arrives first wins, and the rest are ignored - resolving +/// a Nitro Promise twice traps. +final class CameraMoveCompletion { + private let promise: Promise + fileprivate var onSettled: ((CameraMoveCompletion) -> Void)? + private var isSettled = false + + init(promise: Promise) { + self.promise = promise + } + + func settle() { + guard !isSettled else { + return + } + + isSettled = true + onSettled?(self) + onSettled = nil + promise.resolve() + } +} + +/// Holds the camera moves still under way, so the one callback that says the camera has +/// come to rest settles all of them, and none outlives the adapter. +/// +/// Register a move only after handing it to the map. Both SDKs report the end of the move +/// it cuts short from inside that hand-over, and a move registered before it would be +/// settled by that report - before its own animation had even started. +/// +/// Main thread only, like the map views it serves. +final class CameraMoveTracker { + /// Grace period on top of the requested duration before a move is assumed over. The + /// map is expected to report the end of every move, and an unsettled Promise would + /// hang its `await` for the lifetime of the view if it ever did not. + private static let watchdogSlack: TimeInterval = 2 + + private var pending: [CameraMoveCompletion] = [] + private var watchdog: Timer? + + deinit { + settleAll() + } + + /// Tracks `promise` until the camera comes to rest. + func track(_ promise: Promise, duration: TimeInterval) { + track(CameraMoveCompletion(promise: promise), duration: duration) + } + + /// Tracks `move` until the camera comes to rest, or until the move settles itself. + func track(_ move: CameraMoveCompletion, duration: TimeInterval) { + move.onSettled = { [weak self] settled in + self?.remove(settled) + } + pending.append(move) + scheduleWatchdog(after: duration + Self.watchdogSlack) + } + + /// Settles every move still under way. The camera has come to rest, so each of them + /// is over - finished, superseded, or cut short - or the view is going away. + func settleAll() { + guard !pending.isEmpty else { + return + } + + let moves = pending + pending.removeAll() + stopWatchdog() + for move in moves { + move.settle() + } + } + + private func remove(_ move: CameraMoveCompletion) { + pending.removeAll { $0 === move } + if pending.isEmpty { + stopWatchdog() + } + } + + private func scheduleWatchdog(after interval: TimeInterval) { + watchdog?.invalidate() + let timer = Timer(timeInterval: interval, repeats: false) { [weak self] _ in + self?.settleAll() + } + RunLoop.main.add(timer, forMode: .common) + watchdog = timer + } + + private func stopWatchdog() { + watchdog?.invalidate() + watchdog = nil + } +} diff --git a/package/ios/GoogleMapProviderAdapter.swift b/package/ios/GoogleMapProviderAdapter.swift index 6dfa4d1..2fd8b18 100644 --- a/package/ios/GoogleMapProviderAdapter.swift +++ b/package/ios/GoogleMapProviderAdapter.swift @@ -11,9 +11,20 @@ final class GoogleMapProviderAdapter: NSObject, MapProviderAdapter { private static let liveGestureAnimationBudget = 24 private static let defaultAnimationDuration: TimeInterval = 0.25 + private let cameraMoves = CameraMoveTracker() + private lazy var regionChanges = RegionChangeTracker( + position: { [unowned self] in view.camera.placement }, + region: { [unowned self] in view.currentNitroRegion() }, + onBegin: { [unowned self] region, isGesture in + onRegionChange?(region, RegionChangeDetails(isGesture: isGesture)) + }, + onComplete: { [unowned self] region, isGesture in + onRegionChangeComplete?(region, RegionChangeDetails(isGesture: isGesture)) + } + ) + private var isMapReady = false private var hasDeliveredMapReady = false - private var isUserRegionChange = false private var isUserGestureMoving = false private var lastLiveMarkerRefreshTime: CFTimeInterval = 0 private var myLocationObservation: NSKeyValueObservation? @@ -186,8 +197,8 @@ final class GoogleMapProviderAdapter: NSObject, MapProviderAdapter { } } - var onRegionChange: ((Region) -> Void)? - var onRegionChangeComplete: ((Region) -> Void)? + var onRegionChange: ((Region, RegionChangeDetails) -> Void)? + var onRegionChangeComplete: ((Region, RegionChangeDetails) -> Void)? var onMapReady: (() -> Void)? { didSet { deliverMapReadyIfPossible() @@ -244,19 +255,29 @@ final class GoogleMapProviderAdapter: NSObject, MapProviderAdapter { Promise.resolved(withResult: view.camera.toCamera()) } - func applyCamera(camera: Camera) throws { + func applyCamera(camera: Camera) throws -> Promise { updateMapCamera(camera, animated: false) + return Promise.resolved() } - func animateCamera(camera: Camera, duration: TimeInterval?) throws { + func animateCamera(camera: Camera, duration: TimeInterval?) throws -> Promise { let animationDuration = duration ?? Self.defaultAnimationDuration - updateMapCamera(camera, animated: animationDuration > 0, duration: animationDuration) + let animated = animationDuration > 0 + guard updateMapCamera(camera, animated: animated, duration: animationDuration), animated else { + return Promise.resolved() + } + + return trackCameraMove(duration: animationDuration) } func animateToRegion(region: Region, duration: TimeInterval?) throws -> Promise { let animationDuration = duration ?? Self.defaultAnimationDuration - applyRegion(region, animated: animationDuration > 0, duration: animationDuration) - return Promise.resolved() + let animated = animationDuration > 0 + guard applyRegion(region, animated: animated, duration: animationDuration), animated else { + return Promise.resolved() + } + + return trackCameraMove(duration: animationDuration) } func getVisibleRegion() throws -> Promise { @@ -267,10 +288,10 @@ final class GoogleMapProviderAdapter: NSObject, MapProviderAdapter { coordinates: [Coordinate], padding: EdgePadding?, animated: Bool? - ) throws { + ) throws -> Promise { let validCoordinates = coordinates.filter { $0.isValid } guard !validCoordinates.isEmpty else { - return + return Promise.resolved() } var bounds = GMSCoordinateBounds() @@ -279,7 +300,14 @@ final class GoogleMapProviderAdapter: NSObject, MapProviderAdapter { } let edgePadding = padding?.toUIEdgeInsets() ?? .zero let update = GMSCameraUpdate.fit(bounds, with: edgePadding) - applyCameraUpdate(update, animated: animated ?? true, duration: nil) + let shouldAnimate = animated ?? true + applyCameraUpdate(update, animated: shouldAnimate, duration: nil) + + guard shouldAnimate else { + return Promise.resolved() + } + + return trackCameraMove(duration: Self.defaultAnimationDuration) } func pointForCoordinate(coordinate: Coordinate) throws -> Promise { @@ -295,7 +323,8 @@ final class GoogleMapProviderAdapter: NSObject, MapProviderAdapter { } func prepareForRecycle() { - isUserRegionChange = false + cameraMoves.settleAll() + regionChanges.reset() isUserGestureMoving = false lastLiveMarkerRefreshTime = 0 lastAppliedRegion = nil @@ -341,13 +370,16 @@ final class GoogleMapProviderAdapter: NSObject, MapProviderAdapter { clusterEnteringAnimation = nil } + /// Frames `region`, and reports whether anything was handed to the Google SDK: nothing + /// is for an invalid region, or for one the map already shows. + @discardableResult private func applyRegion( _ region: Region, animated: Bool = false, duration: TimeInterval? = nil - ) { + ) -> Bool { guard region.isValid else { - return + return false } if let lastAppliedRegion, @@ -356,7 +388,14 @@ final class GoogleMapProviderAdapter: NSObject, MapProviderAdapter { view.camera.approximatelyEquals(lastAppliedRegionCamera) { // Same region as last time and the camera has not moved since, so the // fit would land on the camera the map already shows. - return + return false + } + + // What the map already shows is exactly what an `onRegionChangeComplete` consumer + // hands back as the next `region` prop. Fitting it again must not move the camera, + // or the echo would answer every move with another one. + guard !view.currentNitroRegion().approximatelyEquals(region) else { + return false } // No insets of its own: Google Maps already fits bounds inside the area @@ -371,20 +410,38 @@ final class GoogleMapProviderAdapter: NSObject, MapProviderAdapter { // `moveCamera` updates `camera` synchronously; an animation does not, so // there is nothing reliable to remember until it settles. lastAppliedRegionCamera = animated ? nil : view.camera + return true } - private func updateMapCamera(_ camera: Camera, animated: Bool, duration: Double? = nil) { + /// Moves the camera, and reports whether anything was handed to the Google SDK: + /// nothing is for an invalid camera, or for the one the map is already at. + @discardableResult + private func updateMapCamera( + _ camera: Camera, + animated: Bool, + duration: Double? = nil + ) -> Bool { guard camera.isValid else { - return + return false } let target = camera.toGMSCameraPosition(current: view.camera) guard !view.camera.approximatelyEquals(target) else { - return + return false } let update = GMSCameraUpdate.setCamera(target) applyCameraUpdate(update, animated: animated, duration: duration) + return true + } + + /// `GMSMapView.animate(with:)` takes no completion handler, so an animated move is + /// settled by `mapView(_:idleAt:)`. Called after the hand-over, as + /// `CameraMoveTracker` requires. + private func trackCameraMove(duration: TimeInterval) -> Promise { + let promise = Promise() + cameraMoves.track(promise, duration: duration) + return promise } private func applyCameraUpdate( @@ -406,31 +463,6 @@ final class GoogleMapProviderAdapter: NSObject, MapProviderAdapter { } } - private func handleRegionWillChange(userInteracting: Bool) { - guard userInteracting, !isUserRegionChange else { - return - } - isUserRegionChange = true - emitRegionChange(complete: false) - } - - private func handleRegionDidChange() { - guard isUserRegionChange else { - return - } - emitRegionChange(complete: true) - isUserRegionChange = false - } - - private func emitRegionChange(complete: Bool) { - let region = view.currentNitroRegion() - if complete { - onRegionChangeComplete?(region) - } else { - onRegionChange?(region) - } - } - private func refreshVisibleMarkers() { overlayController.scheduleViewportRefresh(immediate: true) } @@ -613,7 +645,7 @@ final class GoogleMapProviderAdapter: NSObject, MapProviderAdapter { extension GoogleMapProviderAdapter: GMSMapViewDelegate { func mapView(_ mapView: GMSMapView, willMove gesture: Bool) { - handleRegionWillChange(userInteracting: gesture) + regionChanges.moveStarted(isGesture: gesture) if gesture { startGestureMarkerRefresh() } @@ -621,12 +653,14 @@ extension GoogleMapProviderAdapter: GMSMapViewDelegate { func mapView(_ mapView: GMSMapView, didChange position: GMSCameraPosition) { refreshGestureMarkersIfNeeded() + regionChanges.cameraMoved() } func mapView(_ mapView: GMSMapView, idleAt position: GMSCameraPosition) { refreshVisibleMarkers() stopGestureMarkerRefresh() - handleRegionDidChange() + cameraMoves.settleAll() + regionChanges.cameraStopped() notifyMapReadyIfNeeded() } diff --git a/package/ios/HybridMapView.swift b/package/ios/HybridMapView.swift index 80d97f9..6232bae 100644 --- a/package/ios/HybridMapView.swift +++ b/package/ios/HybridMapView.swift @@ -151,12 +151,12 @@ final class HybridMapView: HybridMapViewSpec { } } - var onRegionChange: ((Region) -> Void)? { + var onRegionChange: ((Region, RegionChangeDetails) -> Void)? { get { getBacked(\.onRegionChange) } set { setBackedOnMain(newValue, store: \.onRegionChange) { $0.onRegionChange = $1 } } } - var onRegionChangeComplete: ((Region) -> Void)? { + var onRegionChangeComplete: ((Region, RegionChangeDetails) -> Void)? { get { getBacked(\.onRegionChangeComplete) } set { setBackedOnMain(newValue, store: \.onRegionChangeComplete) { @@ -242,13 +242,13 @@ final class HybridMapView: HybridMapViewSpec { } func applyCamera(camera: Camera) throws -> Promise { - promiseOnMainVoid { try $0.applyCamera(camera: camera) } + promiseOnMain { try $0.applyCamera(camera: camera) } } func animateCamera(camera: Camera, duration: Double?) throws -> Promise { // Milliseconds from JS, seconds for UIKit and Core Animation. let seconds = duration.map { $0 / 1000 } - return promiseOnMainVoid { + return promiseOnMain { try $0.animateCamera(camera: camera, duration: seconds) } } @@ -269,7 +269,7 @@ final class HybridMapView: HybridMapViewSpec { padding: EdgePadding?, animated: Bool? ) throws -> Promise { - promiseOnMainVoid { + promiseOnMain { try $0.fitToCoordinates( coordinates: coordinates, padding: padding, @@ -451,35 +451,6 @@ final class HybridMapView: HybridMapViewSpec { } } - private func promiseOnMainVoid( - _ work: @escaping (MapProviderAdapter) throws -> Void - ) -> Promise { - let promise = Promise() - let lifecycle = currentLifecycleSnapshot() - let run = { [weak self] in - guard let self else { - promise.reject(withError: Self.mapViewNotMountedError()) - return - } - - do { - let adapter = try self.currentAdapter(matching: lifecycle) - try work(adapter) - promise.resolve(withResult: ()) - } catch { - promise.reject(withError: error) - } - } - - if Thread.isMainThread { - run() - } else { - DispatchQueue.main.async(execute: run) - } - - return promise - } - private func promiseOnMain( _ work: @escaping (MapProviderAdapter) throws -> Promise ) -> Promise { diff --git a/package/ios/HybridMapViewDelegate.swift b/package/ios/HybridMapViewDelegate.swift index bca7bf4..7e68e23 100644 --- a/package/ios/HybridMapViewDelegate.swift +++ b/package/ios/HybridMapViewDelegate.swift @@ -87,6 +87,10 @@ final class HybridMapViewDelegate: NSObject, MKMapViewDelegate, UIGestureRecogni parent?.handleRegionDidChange() } + func mapViewDidChangeVisibleRegion(_ mapView: MKMapView) { + parent?.handleVisibleRegionChange() + } + func mapViewDidFinishLoadingMap(_ mapView: MKMapView) { parent?.notifyMapReadyIfNeeded() } diff --git a/package/ios/MapProviderAdapter.swift b/package/ios/MapProviderAdapter.swift index c534977..d9d85d3 100644 --- a/package/ios/MapProviderAdapter.swift +++ b/package/ios/MapProviderAdapter.swift @@ -23,8 +23,8 @@ protocol MapProviderAdapter: AnyObject { var markerEnteringAnimation: OverlayEnteringAnimationDescriptor? { get set } var clusterEnteringAnimation: OverlayEnteringAnimationDescriptor? { get set } - var onRegionChange: ((Region) -> Void)? { get set } - var onRegionChangeComplete: ((Region) -> Void)? { get set } + var onRegionChange: ((Region, RegionChangeDetails) -> Void)? { get set } + var onRegionChangeComplete: ((Region, RegionChangeDetails) -> Void)? { get set } var onMapReady: (() -> Void)? { get set } var onPress: ((Coordinate) -> Void)? { get set } var onPoiPress: ((NativePoiPressEvent) -> Void)? { get set } @@ -43,17 +43,33 @@ protocol MapProviderAdapter: AnyObject { var onClusterPress: (([String], Coordinate) -> Void)? { get set } func fetchCamera() throws -> Promise - func applyCamera(camera: Camera) throws + + /// Resolves once the camera has arrived: a non-animated move right away, an animated + /// one when it finishes or when a gesture, a later command or ``prepareForRecycle()`` + /// cuts it short. + func applyCamera(camera: Camera) throws -> Promise + /// `duration` is in seconds, here and in `animateToRegion`: `HybridMapView` /// converts the milliseconds JS passes. - func animateCamera(camera: Camera, duration: TimeInterval?) throws - /// Resolves once the animation has been handed over, which for a map without - /// a size yet is in its first layout pass. + /// + /// - SeeAlso: ``applyCamera(camera:)`` for when the Promise settles. + func animateCamera(camera: Camera, duration: TimeInterval?) throws -> Promise + + /// - SeeAlso: ``applyCamera(camera:)`` for when the Promise settles. func animateToRegion(region: Region, duration: TimeInterval?) throws -> Promise + func getVisibleRegion() throws -> Promise - func fitToCoordinates(coordinates: [Coordinate], padding: EdgePadding?, animated: Bool?) throws + + /// - SeeAlso: ``applyCamera(camera:)`` for when the Promise settles. + func fitToCoordinates( + coordinates: [Coordinate], + padding: EdgePadding?, + animated: Bool? + ) throws -> Promise + func pointForCoordinate(coordinate: Coordinate) throws -> Promise func coordinateForPoint(point: Point) throws -> Promise + func prepareForRecycle() } @@ -80,8 +96,8 @@ final class UnavailableMapProviderAdapter: MapProviderAdapter { var markerEnteringAnimation: OverlayEnteringAnimationDescriptor? var clusterEnteringAnimation: OverlayEnteringAnimationDescriptor? - var onRegionChange: ((Region) -> Void)? - var onRegionChangeComplete: ((Region) -> Void)? + var onRegionChange: ((Region, RegionChangeDetails) -> Void)? + var onRegionChangeComplete: ((Region, RegionChangeDetails) -> Void)? var onMapReady: (() -> Void)? var onPress: ((Coordinate) -> Void)? var onPoiPress: ((NativePoiPressEvent) -> Void)? @@ -127,11 +143,11 @@ final class UnavailableMapProviderAdapter: MapProviderAdapter { Promise.rejected(withError: error) } - func applyCamera(camera: Camera) throws { + func applyCamera(camera: Camera) throws -> Promise { throw error } - func animateCamera(camera: Camera, duration: TimeInterval?) throws { + func animateCamera(camera: Camera, duration: TimeInterval?) throws -> Promise { throw error } @@ -147,7 +163,7 @@ final class UnavailableMapProviderAdapter: MapProviderAdapter { coordinates: [Coordinate], padding: EdgePadding?, animated: Bool? - ) throws { + ) throws -> Promise { throw error } diff --git a/package/ios/MapViewState.swift b/package/ios/MapViewState.swift index 5e28f3b..919cfbf 100644 --- a/package/ios/MapViewState.swift +++ b/package/ios/MapViewState.swift @@ -20,8 +20,8 @@ struct MapViewState { var mapPadding: EdgePadding? var markerEnteringAnimation: OverlayEnteringAnimationDescriptor? var clusterEnteringAnimation: OverlayEnteringAnimationDescriptor? - var onRegionChange: ((Region) -> Void)? - var onRegionChangeComplete: ((Region) -> Void)? + var onRegionChange: ((Region, RegionChangeDetails) -> Void)? + var onRegionChangeComplete: ((Region, RegionChangeDetails) -> Void)? var onMapReady: (() -> Void)? var onPress: ((Coordinate) -> Void)? var onPoiPress: ((NativePoiPressEvent) -> Void)? diff --git a/package/ios/Package.swift b/package/ios/Package.swift index b2d9efa..0fba48a 100644 --- a/package/ios/Package.swift +++ b/package/ios/Package.swift @@ -8,6 +8,10 @@ let package = Package( name: "NitroMapsSupport", platforms: [.macOS(.v13)], targets: [ + .target( + name: "NitroMapsCamera", + path: "Camera" + ), .target( name: "NitroMapsColorParser", path: "ColorParser" @@ -28,6 +32,11 @@ let package = Package( name: "NitroMapsShapeDiff", path: "ShapeDiff" ), + .testTarget( + name: "NitroMapsCameraTests", + dependencies: ["NitroMapsCamera"], + path: "Tests/Camera" + ), .testTarget( name: "NitroMapsColorParserTests", dependencies: ["NitroMapsColorParser"], diff --git a/package/ios/Tests/Camera/RegionChangeTrackerTests.swift b/package/ios/Tests/Camera/RegionChangeTrackerTests.swift new file mode 100644 index 0000000..0ff8175 --- /dev/null +++ b/package/ios/Tests/Camera/RegionChangeTrackerTests.swift @@ -0,0 +1,192 @@ +import Testing + +@testable import NitroMapsCamera + +/// Drives `RegionChangeTracker` with the callback sequences MapKit and Google Maps were +/// seen to produce, with an integer standing in for the camera position and for the +/// region derived from it. The Kotlin `RegionChangeTrackerTest` runs the same cases. +@MainActor +private final class Harness { + var position = 0 + var events: [String] = [] + + lazy var tracker = RegionChangeTracker( + position: { [unowned self] in position }, + region: { [unowned self] in position }, + onBegin: { [unowned self] region, isGesture in + events.append("begin \(isGesture ? "gesture" : "app") @\(region)") + }, + onComplete: { [unowned self] region, isGesture in + events.append("complete \(isGesture ? "gesture" : "app") @\(region)") + } + ) + + /// A map that has settled into its first position, as every case but one assumes. + static func settled() -> Harness { + let harness = Harness() + harness.tracker.cameraStopped() + return harness + } +} + +@Test @MainActor +func anAnimationEmitsOneBeginAndOneComplete() { + let harness = Harness.settled() + harness.tracker.moveStarted(isGesture: false) + harness.position = 1 + harness.tracker.cameraMoved() + harness.position = 2 + harness.tracker.cameraMoved() + harness.tracker.cameraStopped() + + #expect(harness.events == ["begin app @0", "complete app @2"]) +} + +@Test @MainActor +func aJumpWithNoStepReportedStillEmitsBothEvents() { + let harness = Harness.settled() + harness.tracker.moveStarted(isGesture: false) + harness.position = 5 + harness.tracker.cameraStopped() + + #expect(harness.events == ["begin app @0", "complete app @5"]) +} + +@Test @MainActor +func measuresAMoveFromWhereTheCameraLastCameToRest() { + // MapKit already reports the destination when the app sets the camera. + let harness = Harness.settled() + harness.position = 4 + harness.tracker.moveStarted(isGesture: false) + harness.tracker.cameraStopped() + + #expect(harness.events == ["begin app @0", "complete app @4"]) +} + +@Test @MainActor +func anUpdateThatLeavesTheCameraInPlaceEmitsNothing() { + // A repeated fit, or a `region` prop sent again with the same values. + let harness = Harness.settled() + harness.tracker.moveStarted(isGesture: false) + harness.tracker.cameraMoved() + harness.tracker.cameraStopped() + + #expect(harness.events.isEmpty) +} + +@Test @MainActor +func theMapSettlingIntoItsFirstPositionEmitsNothing() { + let harness = Harness() + harness.tracker.moveStarted(isGesture: false) + harness.position = 7 + harness.tracker.cameraMoved() + harness.tracker.cameraStopped() + #expect(harness.events.isEmpty) + + // From there on, moves are measured against it. + harness.tracker.moveStarted(isGesture: false) + harness.position = 8 + harness.tracker.cameraStopped() + #expect(harness.events == ["begin app @7", "complete app @8"]) +} + +@Test @MainActor +func aSecondStartForTheSameMoveIsIgnored() { + // One animation interrupting another, reported as one continuous move. + let harness = Harness.settled() + harness.tracker.moveStarted(isGesture: false) + harness.position = 1 + harness.tracker.cameraMoved() + harness.tracker.moveStarted(isGesture: false) + harness.position = 2 + harness.tracker.cameraMoved() + harness.tracker.cameraStopped() + + #expect(harness.events == ["begin app @0", "complete app @2"]) +} + +@Test @MainActor +func aGestureTakingOverEndsTheAppsMoveAndStartsItsOwn() { + let harness = Harness.settled() + harness.tracker.moveStarted(isGesture: false) + harness.position = 1 + harness.tracker.cameraMoved() + harness.tracker.moveStarted(isGesture: true) + harness.position = 2 + harness.tracker.cameraMoved() + harness.tracker.cameraStopped() + + #expect( + harness.events == [ + "begin app @0", "complete app @1", "begin gesture @1", "complete gesture @2", + ] + ) +} + +@Test @MainActor +func aGestureTakingOverBeforeTheCameraMovedLeavesOnlyTheGesture() { + let harness = Harness.settled() + harness.tracker.moveStarted(isGesture: false) + harness.tracker.moveStarted(isGesture: true) + harness.position = 3 + harness.tracker.cameraMoved() + harness.tracker.cameraStopped() + + #expect(harness.events == ["begin gesture @0", "complete gesture @3"]) +} + +@Test @MainActor +func theAppTakingOverFromAGestureStaysOneGestureMove() { + let harness = Harness.settled() + harness.tracker.moveStarted(isGesture: true) + harness.position = 1 + harness.tracker.cameraMoved() + harness.tracker.moveStarted(isGesture: false) + harness.position = 2 + harness.tracker.cameraMoved() + harness.tracker.cameraStopped() + + #expect(harness.events == ["begin gesture @0", "complete gesture @2"]) +} + +@Test @MainActor +func reportsAGestureOnlyWhileOneIsUnderWay() { + let harness = Harness.settled() + #expect(!harness.tracker.isGesture) + + harness.tracker.moveStarted(isGesture: true) + #expect(harness.tracker.isGesture) + + harness.tracker.cameraStopped() + #expect(!harness.tracker.isGesture) + + harness.tracker.moveStarted(isGesture: false) + #expect(!harness.tracker.isGesture) +} + +@Test @MainActor +func ignoresStepsOutsideAMove() { + let harness = Harness.settled() + harness.position = 1 + harness.tracker.cameraMoved() + + #expect(harness.events.isEmpty) +} + +@Test @MainActor +func resetForgetsTheMoveAndWhereTheCameraRested() { + let harness = Harness.settled() + harness.tracker.moveStarted(isGesture: false) + harness.position = 1 + harness.tracker.cameraMoved() + harness.tracker.reset() + harness.tracker.cameraStopped() + #expect(harness.events == ["begin app @0"]) + + // A new map settles into its first position without a word. + harness.tracker.reset() + harness.tracker.moveStarted(isGesture: false) + harness.position = 2 + harness.tracker.cameraStopped() + #expect(harness.events == ["begin app @0"]) +} diff --git a/package/src/index.ts b/package/src/index.ts index a12347d..b274c44 100644 --- a/package/src/index.ts +++ b/package/src/index.ts @@ -12,6 +12,7 @@ export type { Coordinate, Point, Region, + RegionChangeDetails, Camera, EdgePadding, VisibleRegion, diff --git a/package/src/native/specs/MapView.nitro.ts b/package/src/native/specs/MapView.nitro.ts index a340b8b..b774154 100644 --- a/package/src/native/specs/MapView.nitro.ts +++ b/package/src/native/specs/MapView.nitro.ts @@ -7,7 +7,12 @@ import type { Camera } from '../../types/camera'; import type { Coordinate } from '../../types/coordinate'; import type { MapProvider, MapType } from '../../types/map'; import type { Point } from '../../types/point'; -import type { EdgePadding, Region, VisibleRegion } from '../../types/region'; +import type { + EdgePadding, + Region, + RegionChangeDetails, + VisibleRegion, +} from '../../types/region'; import type { CircleDescriptor, MarkerDescriptor, @@ -181,11 +186,14 @@ export interface MapViewProps extends HybridViewProps { /** Entering animation for marker clusters. */ clusterEnteringAnimation?: OverlayEnteringAnimationDescriptor; - /** Called once when a user-initiated region change begins. */ - onRegionChange?: (region: Region) => void; + /** Called once when a region change begins, whoever started it. */ + onRegionChange?: (region: Region, details: RegionChangeDetails) => void; - /** Called once when a user-initiated region change ends. */ - onRegionChangeComplete?: (region: Region) => void; + /** Called once when a region change ends, whoever started it. */ + onRegionChangeComplete?: ( + region: Region, + details: RegionChangeDetails, + ) => void; /** Called when the map is ready to use. */ onMapReady?: () => void; @@ -246,7 +254,8 @@ export interface MapViewMethods extends HybridViewMethods { fetchCamera(): Promise; /** - * Sets the camera position immediately. + * Sets the camera position immediately. Resolves once the move is done, which + * for a non-animated move is right away. * * Named `applyCamera` in the Nitro spec to avoid colliding with the `camera` * prop accessor (`getCamera`/`setCamera`) in generated C++ bindings. @@ -255,20 +264,25 @@ export interface MapViewMethods extends HybridViewMethods { /** * Animates the camera to the given position, over `duration` milliseconds - - * 250 when omitted, a jump for 0. + * 250 when omitted, a jump for 0. Resolves once the camera has arrived, or + * once the animation is cut short. */ animateCamera(camera: Camera, duration?: number): Promise; /** * Animates the camera to frame the given region, over `duration` - * milliseconds - 250 when omitted, a jump for 0. + * milliseconds - 250 when omitted, a jump for 0. Resolves once the camera has + * arrived, or once the animation is cut short. */ animateToRegion(region: Region, duration?: number): Promise; /** Returns the currently visible geographic region. */ getVisibleRegion(): Promise; - /** Fits the camera to show all given coordinates with optional edge padding. */ + /** + * Fits the camera to show all given coordinates with optional edge padding. + * Resolves once the camera has arrived, or once the animation is cut short. + */ fitToCoordinates( coordinates: Coordinate[], padding?: EdgePadding, diff --git a/package/src/types/index.ts b/package/src/types/index.ts index 8a8f1aa..bdc28af 100644 --- a/package/src/types/index.ts +++ b/package/src/types/index.ts @@ -1,7 +1,12 @@ export type { Coordinate } from './coordinate'; export type { Point } from './point'; export type { Camera } from './camera'; -export type { Region, EdgePadding, VisibleRegion } from './region'; +export type { + Region, + EdgePadding, + RegionChangeDetails, + VisibleRegion, +} from './region'; export type { ApplePoiCategory, ApplePoiDetailPresentation, diff --git a/package/src/types/map.ts b/package/src/types/map.ts index 4f57332..7214f9e 100644 --- a/package/src/types/map.ts +++ b/package/src/types/map.ts @@ -12,7 +12,7 @@ import type { ApplePoiDetailPresentation, } from '../native/specs/MapView.nitro'; import type { MarkerDescriptor, OverlayEnteringAnimation } from './overlays'; -import type { EdgePadding, Region } from './region'; +import type { EdgePadding, Region, RegionChangeDetails } from './region'; /** * Available map display styles. @@ -116,11 +116,28 @@ interface BaseMapViewProps { /** Called when any circle is pressed. */ onCirclePress?: (id: string) => void; - /** Called once when a user-initiated region change begins. */ - onRegionChange?: (region: Region) => void; + /** + * Called once when the camera starts to move, with the region it is leaving - + * not on every frame while it moves. + * + * Fires for gestures and for programmatic moves alike - `setCamera`, + * `animateCamera`, `fitToCoordinates` and the `region` / `camera` props. + * Use `details.isGesture` to tell the two apart. An update that leaves the + * camera where it is fires nothing, and neither does the map settling into + * its first position as it appears. + */ + onRegionChange?: (region: Region, details: RegionChangeDetails) => void; - /** Called once when a user-initiated region change ends. */ - onRegionChangeComplete?: (region: Region) => void; + /** + * Called once when the camera comes to rest, with the region it arrived at + * and the same `details` as the `onRegionChange` that began the move. + * + * @see {@linkcode BaseMapViewProps.onRegionChange} + */ + onRegionChangeComplete?: ( + region: Region, + details: RegionChangeDetails, + ) => void; /** Called when the map is ready to use. */ onMapReady?: () => void; diff --git a/package/src/types/ref.ts b/package/src/types/ref.ts index b99a53e..57cf5aa 100644 --- a/package/src/types/ref.ts +++ b/package/src/types/ref.ts @@ -14,6 +14,13 @@ import type { EdgePadding, Region, VisibleRegion } from './region'; * A call that is still waiting when the map view unmounts rejects, as does any * call made afterwards. Nothing silently does nothing. * + * The camera methods resolve when the move is over, never before. An animated + * move resolves when the camera arrives, or as soon as something cuts it short: + * a gesture, a later camera command, or the map view unmounting mid-animation. + * It resolves rather than rejects in those cases, and does not say whether the + * requested position was reached - read {@linkcode MapViewRef.getCamera} or + * {@linkcode MapViewRef.getVisibleRegion} after the `await` when that matters. + * * @example * ```tsx * const mapRef = useRef(null); @@ -30,7 +37,8 @@ export interface MapViewRef { getCamera(): Promise; /** - * Sets the camera position immediately. + * Sets the camera position immediately. Resolves once the camera is there, + * which for this un-animated move is right away. * * Rejects straight away, and leaves the map where it is, for a camera the * map cannot use: a center outside the world, or a zoom, heading, pitch or @@ -40,9 +48,10 @@ export interface MapViewRef { setCamera(camera: Camera): Promise; /** - * Animates the camera to the given position. Resolves once the animation has - * been handed to the native map, not when it finishes, and rejects for a - * camera the map cannot use, as {@linkcode MapViewRef.setCamera} does. + * Animates the camera to the given position. Resolves when the animation + * ends - after roughly `duration`, or earlier if something cuts it short - + * and rejects for a camera the map cannot use, as + * {@linkcode MapViewRef.setCamera} does. * * @param duration Animation duration in milliseconds. Defaults to `250`; * `0` moves the camera without animating. @@ -55,10 +64,10 @@ export interface MapViewRef { * {@linkcode MapViewProps.onRegionChangeComplete} passed back returns the map * to that view. * - * Resolves once the animation has been handed to the native map, not when - * it finishes, and rejects for a region the map cannot use - a center - * outside the world, or a delta that is not a finite number greater than 0 - - * which the `region` prop skips instead. + * Resolves when the animation ends - after roughly `duration`, or earlier if + * something cuts it short. Rejects for a region the map cannot use, which the + * `region` prop skips instead: a center outside the world, or a delta that is + * not a finite number greater than 0. * * @param duration Animation duration in milliseconds. Defaults to `250`; * `0` moves the camera without animating. @@ -70,7 +79,8 @@ export interface MapViewRef { /** * Fits the camera to show all given coordinates with optional edge padding. - * An empty {@linkcode coordinates} list is a no-op. + * Resolves once the camera has arrived, which for an un-animated fit is + * right away. An empty {@linkcode coordinates} list is a no-op. * * {@linkcode padding} is added on top of {@linkcode MapViewProps.mapPadding}. * diff --git a/package/src/types/region.ts b/package/src/types/region.ts index 445965f..88214b9 100644 --- a/package/src/types/region.ts +++ b/package/src/types/region.ts @@ -29,3 +29,11 @@ export interface VisibleRegion { farLeft: Coordinate; farRight: Coordinate; } + +/** + * Context delivered alongside a region change. + */ +export interface RegionChangeDetails { + /** Whether the change was started by a user gesture rather than by the app. */ + isGesture: boolean; +} diff --git a/package/type-tests/provider-props.ts b/package/type-tests/provider-props.ts index 843a4ac..e2443b2 100644 --- a/package/type-tests/provider-props.ts +++ b/package/type-tests/provider-props.ts @@ -2,6 +2,7 @@ import type { ApplePoiCategory, MapViewProps, MapViewPropsForProvider, + Region, } from '../src'; export const appleProps: MapViewPropsForProvider<'apple'> = { @@ -57,6 +58,18 @@ export const defaultProviderGoogleMapIdProps: MapViewProps = { googleMapId: 'google-map-id', }; +export const regionChangeProps: MapViewProps = { + onRegionChange: (region, details) => { + region satisfies Region; + details.isGesture satisfies boolean; + }, + onRegionChangeComplete: (region, details) => { + region satisfies Region; + // @ts-expect-error The details carry the gesture flag, not the region. + details.latitude satisfies number; + }, +}; + export const plannedProviderProps: MapViewProps = { provider: 'mapbox', };