From 5a45e83d3473304823ffd60684d3dd65f08da8b5 Mon Sep 17 00:00:00 2001 From: Anthony Trasatti Date: Tue, 3 Nov 2026 18:07:35 -0500 Subject: [PATCH 1/4] fix(android): stop unnecessary hidden location puck rendering --- .../components/location/LocationComponentManager.kt | 4 ++-- .../components/location/RNMBXNativeUserLocation.kt | 13 +++++++++---- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/android/src/main/java/com/rnmapbox/rnmbx/components/location/LocationComponentManager.kt b/android/src/main/java/com/rnmapbox/rnmbx/components/location/LocationComponentManager.kt index 1711ed328..8e3e850f8 100644 --- a/android/src/main/java/com/rnmapbox/rnmbx/components/location/LocationComponentManager.kt +++ b/android/src/main/java/com/rnmapbox/rnmbx/components/location/LocationComponentManager.kt @@ -207,9 +207,9 @@ class LocationComponentManager(mapView: RNMBXMapView, context: Context) { } - fun showNativeUserLocation(showUserLocation: Boolean) { + fun showNativeUserLocation(showUserLocation: Boolean, visible: Boolean = true) { update { - it.copy(showUserLocation = showUserLocation, nativeUserLocation= showUserLocation) + it.copy(showUserLocation = showUserLocation && visible, nativeUserLocation = showUserLocation) } } diff --git a/android/src/main/java/com/rnmapbox/rnmbx/components/location/RNMBXNativeUserLocation.kt b/android/src/main/java/com/rnmapbox/rnmbx/components/location/RNMBXNativeUserLocation.kt index 38f932d71..4f71d36ef 100644 --- a/android/src/main/java/com/rnmapbox/rnmbx/components/location/RNMBXNativeUserLocation.kt +++ b/android/src/main/java/com/rnmapbox/rnmbx/components/location/RNMBXNativeUserLocation.kt @@ -134,6 +134,8 @@ class RNMBXNativeUserLocation(context: Context) : AbstractMapFeature(context), O } private fun _apply(mapView: MapView) { + // Keep location interpolation active for camera following, even with no visible puck. + mMapView?.locationComponentManager?.showNativeUserLocation(mEnabled, visible) val location2 = mapView.location2; val locationPuck3D = model?.let { makeLocationPuck3D(it) } @@ -167,10 +169,13 @@ class RNMBXNativeUserLocation(context: Context) : AbstractMapFeature(context), O location2.puckBearingEnabled = it } + if (!visible) { + location2.pulsingEnabled = false + } pulsing?.let { pulsing -> pulsing.getAndLogIfNotString("kind")?.also { kind -> if (kind == "default") { - location2.pulsingEnabled = true + location2.pulsingEnabled = visible } } if (pulsing.hasKey("color")) { @@ -184,7 +189,7 @@ class RNMBXNativeUserLocation(context: Context) : AbstractMapFeature(context), O } } pulsing.getAndLogIfNotBoolean("isEnabled")?.let { enabled -> - location2.pulsingEnabled = enabled + location2.pulsingEnabled = visible && enabled } if (pulsing.hasKey("radius")) { when (pulsing.getType("radius")) { @@ -226,7 +231,7 @@ class RNMBXNativeUserLocation(context: Context) : AbstractMapFeature(context), O mEnabled = true mapView.getMapboxMap() mapView.getMapAsync(this) - mMapView?.locationComponentManager?.showNativeUserLocation(true) + mMapView?.locationComponentManager?.showNativeUserLocation(true, visible) mMBXMapView = mapView _fetchImages(mapView) _apply() @@ -255,7 +260,7 @@ class RNMBXNativeUserLocation(context: Context) : AbstractMapFeature(context), O } mMapView?.locationComponentManager?.update() - mMapView?.locationComponentManager?.showNativeUserLocation(mEnabled) + mMapView?.locationComponentManager?.showNativeUserLocation(mEnabled, visible) } // region fetch images and subscribe on updates From ab8f1e4ca6ab576eb167c999e09b5e8847d968f3 Mon Sep 17 00:00:00 2001 From: Anthony Trasatti Date: Tue, 3 Nov 2026 18:08:52 -0500 Subject: [PATCH 2/4] test(android): cover hidden puck transitions in existing JVM suite --- .github/workflows/android-actions.yml | 2 +- .../location/LocationComponentManager.kt | 9 +- .../location/RNMBXNativeUserLocation.kt | 85 ++++++------ .../location/LocationComponentManagerTest.kt | 124 ++++++++++++++++++ .../location/RNMBXNativeUserLocationTest.kt | 91 +++++++++++++ 5 files changed, 269 insertions(+), 42 deletions(-) create mode 100644 android/src/test/kotlin/com/rnmapbox/rnmbx/components/location/LocationComponentManagerTest.kt create mode 100644 android/src/test/kotlin/com/rnmapbox/rnmbx/components/location/RNMBXNativeUserLocationTest.kt diff --git a/.github/workflows/android-actions.yml b/.github/workflows/android-actions.yml index 17f24c261..0dbf317b3 100644 --- a/.github/workflows/android-actions.yml +++ b/.github/workflows/android-actions.yml @@ -78,7 +78,7 @@ jobs: - run: yarn postinstall working-directory: example - - run: ./gradlew assemble + - run: ./gradlew assemble :rnmapbox_maps:testDebugUnitTest working-directory: example/android env: CI_MAP_IMPL: ${{ inputs.MAP_IMPL }} diff --git a/android/src/main/java/com/rnmapbox/rnmbx/components/location/LocationComponentManager.kt b/android/src/main/java/com/rnmapbox/rnmbx/components/location/LocationComponentManager.kt index 8e3e850f8..d03fb0634 100644 --- a/android/src/main/java/com/rnmapbox/rnmbx/components/location/LocationComponentManager.kt +++ b/android/src/main/java/com/rnmapbox/rnmbx/components/location/LocationComponentManager.kt @@ -31,7 +31,11 @@ import com.rnmapbox.rnmbx.v11compat.location.PuckBearing * LocationComponentManager attempts to separate that, so that Camera can ask for location tracking independent of display of user current location. * And NativeUserLocation can ask for display of user's current location - independent of Camera's user tracking. */ -class LocationComponentManager(mapView: RNMBXMapView, context: Context) { +class LocationComponentManager @JvmOverloads constructor( + mapView: RNMBXMapView, + context: Context, + private val mLocationManager: LocationManager = LocationManager.getInstance(context), +) { private val MAPBOX_BLUE_COLOR = Color.parseColor("#4A90E2") var mMapView = mapView @@ -47,8 +51,6 @@ class LocationComponentManager(mapView: RNMBXMapView, context: Context) { scale = 1.0, ) - private var mLocationManager: LocationManager = LocationManager.getInstance(context) - private var mNeedsFullUpdate = true private var mLocationManagerStarted = false @@ -207,6 +209,7 @@ class LocationComponentManager(mapView: RNMBXMapView, context: Context) { } + @JvmOverloads fun showNativeUserLocation(showUserLocation: Boolean, visible: Boolean = true) { update { it.copy(showUserLocation = showUserLocation && visible, nativeUserLocation = showUserLocation) diff --git a/android/src/main/java/com/rnmapbox/rnmbx/components/location/RNMBXNativeUserLocation.kt b/android/src/main/java/com/rnmapbox/rnmbx/components/location/RNMBXNativeUserLocation.kt index 4f71d36ef..d91ac1f3f 100644 --- a/android/src/main/java/com/rnmapbox/rnmbx/components/location/RNMBXNativeUserLocation.kt +++ b/android/src/main/java/com/rnmapbox/rnmbx/components/location/RNMBXNativeUserLocation.kt @@ -20,6 +20,7 @@ import com.mapbox.maps.MapboxMap import com.mapbox.maps.Style import com.mapbox.maps.plugin.LocationPuck2D import com.mapbox.maps.plugin.LocationPuck3D +import com.mapbox.maps.plugin.locationcomponent.LocationComponentPlugin import com.mapbox.maps.plugin.locationcomponent.LocationComponentConstants import com.mapbox.maps.plugin.locationcomponent.location import com.mapbox.maps.plugin.locationcomponent.R as LR @@ -169,43 +170,7 @@ class RNMBXNativeUserLocation(context: Context) : AbstractMapFeature(context), O location2.puckBearingEnabled = it } - if (!visible) { - location2.pulsingEnabled = false - } - pulsing?.let { pulsing -> - pulsing.getAndLogIfNotString("kind")?.also { kind -> - if (kind == "default") { - location2.pulsingEnabled = visible - } - } - if (pulsing.hasKey("color")) { - when (pulsing.getType("color")) { - ReadableType.Map -> - location2.pulsingColor = ColorPropConverter.getColor(pulsing.getMap("color"), mContext) ?: 0 - ReadableType.Number -> - location2.pulsingColor = pulsing.getInt("color") - else -> - Logger.e(LOG_TAG, "pusling.color should be either a map or a number, but was ${pulsing.getDynamic("color")}") - } - } - pulsing.getAndLogIfNotBoolean("isEnabled")?.let { enabled -> - location2.pulsingEnabled = visible && enabled - } - if (pulsing.hasKey("radius")) { - when (pulsing.getType("radius")) { - ReadableType.Number -> - location2.pulsingMaxRadius = pulsing.getDouble("radius").toFloat() - ReadableType.String -> - if (pulsing.getString("radius") == "accuracy") { - location2.pulsingMaxRadius = LocationComponentConstants.PULSING_MAX_RADIUS_FOLLOW_ACCURACY - } else { - Logger.e(LOG_TAG, "Expected pulsing/radius to be a number or accuracy but was ${pulsing.getString("radius")}") - } - else -> - Logger.e(LOG_TAG, "Expected pulsing/radius to be a number or accuracy but was ${pulsing.getString("radius")}") - } - } - } + applyLocationPuckPulsing(location2, visible, pulsing, mContext) } private fun makeLocationPuck3D(model: ReadableMap): LocationPuck3D? { @@ -342,4 +307,48 @@ fun makeDefaultLocationPuck2D(context: Context, renderMode: RenderMode): Locatio LR.drawable.mapbox_user_icon_shadow ) ); -} \ No newline at end of file +} +internal fun applyLocationPuckPulsing( + location: LocationComponentPlugin, + visible: Boolean, + pulsing: ReadableMap?, + context: Context, +) { + if (!visible) { + location.pulsingEnabled = false + } + pulsing?.let { pulsing -> + pulsing.getAndLogIfNotString("kind")?.also { kind -> + if (kind == "default") { + location.pulsingEnabled = visible + } + } + if (pulsing.hasKey("color")) { + when (pulsing.getType("color")) { + ReadableType.Map -> + location.pulsingColor = ColorPropConverter.getColor(pulsing.getMap("color"), context) ?: 0 + ReadableType.Number -> + location.pulsingColor = pulsing.getInt("color") + else -> + Logger.e("RNMBXNativeUserLocation", "pusling.color should be either a map or a number, but was ${pulsing.getDynamic("color")}") + } + } + pulsing.getAndLogIfNotBoolean("isEnabled")?.let { enabled -> + location.pulsingEnabled = visible && enabled + } + if (pulsing.hasKey("radius")) { + when (pulsing.getType("radius")) { + ReadableType.Number -> + location.pulsingMaxRadius = pulsing.getDouble("radius").toFloat() + ReadableType.String -> + if (pulsing.getString("radius") == "accuracy") { + location.pulsingMaxRadius = LocationComponentConstants.PULSING_MAX_RADIUS_FOLLOW_ACCURACY + } else { + Logger.e("RNMBXNativeUserLocation", "Expected pulsing/radius to be a number or accuracy but was ${pulsing.getString("radius")}") + } + else -> + Logger.e("RNMBXNativeUserLocation", "Expected pulsing/radius to be a number or accuracy but was ${pulsing.getString("radius")}") + } + } + } +} diff --git a/android/src/test/kotlin/com/rnmapbox/rnmbx/components/location/LocationComponentManagerTest.kt b/android/src/test/kotlin/com/rnmapbox/rnmbx/components/location/LocationComponentManagerTest.kt new file mode 100644 index 000000000..f64c65ca2 --- /dev/null +++ b/android/src/test/kotlin/com/rnmapbox/rnmbx/components/location/LocationComponentManagerTest.kt @@ -0,0 +1,124 @@ +package com.rnmapbox.rnmbx.components.location + +import android.content.Context +import android.graphics.Color +import android.os.Handler +import android.os.Looper +import androidx.lifecycle.Lifecycle +import com.mapbox.maps.plugin.LocationPuck2D +import com.mapbox.maps.plugin.LocationPuck3D +import com.mapbox.maps.plugin.Plugin +import com.mapbox.maps.plugin.delegates.MapPluginProviderDelegate +import com.mapbox.maps.plugin.locationcomponent.LocationComponentPlugin +import com.mapbox.maps.plugin.locationcomponent.generated.LocationComponentSettings +import com.rnmapbox.rnmbx.components.mapview.RNMBXMapView +import com.rnmapbox.rnmbx.location.LocationManager +import org.junit.jupiter.api.Assertions.* +import org.junit.jupiter.api.Test +import org.mockito.Mockito +import org.mockito.kotlin.* + +class LocationComponentManagerTest { + private class Fixture { + val location: LocationComponentPlugin = mock() + val provider: MapPluginProviderDelegate = mock() + val map: RNMBXMapView = mock() + val locationManager: LocationManager = mock() + var lifecycle = Lifecycle.State.STARTED + var settings = LocationComponentSettings.Builder(LocationPuck2D()).build() + var settingsWrites = 0 + val manager: LocationComponentManager + + init { + whenever(provider.getPlugin(Plugin.MAPBOX_LOCATION_COMPONENT_PLUGIN_ID)) + .thenReturn(location) + whenever(map.mapPlugins).thenReturn(provider) + whenever(map.getLifecycleState()).thenAnswer { lifecycle } + doAnswer { invocation -> + val update = invocation.getArgument Unit>(0) + settings = settings.toBuilder().apply(update).build() + settingsWrites++ + null + }.whenever(location).updateSettings(any()) + manager = LocationComponentManager(map, mock(), locationManager) + } + } + + private fun withManager(test: Fixture.() -> Unit) { + Mockito.mockStatic(Color::class.java).use { + Mockito.mockStatic(Looper::class.java).use { + Mockito.mockConstruction(Handler::class.java).use { + Fixture().test() + } + } + } + } + + @Test + fun `hidden puck stays disabled until following starts and stops again`() = withManager { + manager.showNativeUserLocation(true, visible = false) + assertFalse(settings.enabled) + verify(locationManager, never()).startCounted() + + manager.setFollowLocation(true) + assertTrue(settings.enabled) + verify(locationManager).startCounted() + + manager.setFollowLocation(false) + assertFalse(settings.enabled) + verify(locationManager).stopCounted() + } + + @Test + fun `hiding and removing a puck preserves active following`() = withManager { + manager.showNativeUserLocation(true) + manager.setFollowLocation(true) + manager.showNativeUserLocation(true, visible = false) + assertTrue(settings.enabled) + manager.showNativeUserLocation(false) + assertTrue(settings.enabled) + verify(locationManager, never()).stopCounted() + manager.setFollowLocation(false) + assertFalse(settings.enabled) + verify(locationManager).stopCounted() + } + + @Test + fun `repeated hide show cycles balance location subscriptions`() = withManager { + repeat(20) { + manager.showNativeUserLocation(true) + assertTrue(settings.enabled) + manager.showNativeUserLocation(true, visible = false) + assertFalse(settings.enabled) + } + verify(locationManager, times(20)).startCounted() + verify(locationManager, times(20)).stopCounted() + manager.onDestroy() + verify(locationManager, times(20)).stopCounted() + } + + @Test + fun `manager leaves native owned puck configuration untouched during follow transitions`() = withManager { + for (puck in listOf(LocationPuck2D(scaleExpression = "2"), LocationPuck3D(modelUri = "asset://car.glb"))) { + settings = settings.toBuilder().apply { locationPuck = puck }.build() + manager.showNativeUserLocation(true) + manager.showNativeUserLocation(true, visible = false) + manager.setFollowLocation(true) + manager.showNativeUserLocation(true) + assertSame(puck, settings.locationPuck) + manager.setFollowLocation(false) + } + } + + @Test + fun `shutdown does not restart the plugin or stop the location manager twice`() = withManager { + manager.showNativeUserLocation(true) + val writesBeforeStop = settingsWrites + lifecycle = Lifecycle.State.CREATED + manager.showNativeUserLocation(true, visible = false) + assertEquals(writesBeforeStop, settingsWrites) + verify(locationManager).stopCounted() + manager.onDestroy() + verify(locationManager).stopCounted() + } +} diff --git a/android/src/test/kotlin/com/rnmapbox/rnmbx/components/location/RNMBXNativeUserLocationTest.kt b/android/src/test/kotlin/com/rnmapbox/rnmbx/components/location/RNMBXNativeUserLocationTest.kt new file mode 100644 index 000000000..592d6ab3b --- /dev/null +++ b/android/src/test/kotlin/com/rnmapbox/rnmbx/components/location/RNMBXNativeUserLocationTest.kt @@ -0,0 +1,91 @@ +package com.rnmapbox.rnmbx.components.location + +import android.content.Context +import com.facebook.react.bridge.ReadableMap +import com.facebook.react.bridge.ReadableType +import com.mapbox.maps.plugin.locationcomponent.LocationComponentPlugin +import org.junit.jupiter.api.Assertions.* +import org.junit.jupiter.api.Test +import org.mockito.kotlin.* + +class RNMBXNativeUserLocationTest { + private val location: LocationComponentPlugin = mock() + private val context: Context = mock() + private var enabled = false + + init { + doAnswer { enabled = it.getArgument(0); null } + .whenever(location).pulsingEnabled = any() + } + + private fun request(enabled: Boolean): ReadableMap = mock { + on { hasKey("isEnabled") } doReturn true + on { getType("isEnabled") } doReturn ReadableType.Boolean + on { getBoolean("isEnabled") } doReturn enabled + } + + private fun apply(visible: Boolean, request: ReadableMap?) { + applyLocationPuckPulsing(location, visible, request, context) + } + + @Test + fun `explicit pulse resumes after hide show without losing configuration`() { + val requested = request(true) + whenever(requested.hasKey("radius")).thenReturn(true) + whenever(requested.getType("radius")).thenReturn(ReadableType.Number) + whenever(requested.getDouble("radius")).thenReturn(25.0) + whenever(requested.hasKey("color")).thenReturn(true) + whenever(requested.getType("color")).thenReturn(ReadableType.Number) + whenever(requested.getInt("color")).thenReturn(0xFF123456.toInt()) + apply(true, requested) + assertTrue(enabled) + apply(false, requested) + assertFalse(enabled) + apply(true, requested) + assertTrue(enabled) + verify(location, times(3)).pulsingMaxRadius = 25f + verify(location, times(3)).pulsingColor = 0xFF123456.toInt() + } + + @Test + fun `default pulse is suppressed while hidden and restored when visible`() { + val requested: ReadableMap = mock { + on { hasKey("kind") } doReturn true + on { getType("kind") } doReturn ReadableType.String + on { getString("kind") } doReturn "default" + } + apply(true, requested) + assertTrue(enabled) + apply(false, requested) + assertFalse(enabled) + apply(true, requested) + assertTrue(enabled) + } + + @Test + fun `pulse changes while hidden are deferred until visible`() { + apply(false, request(false)) + apply(false, request(true)) + assertFalse(enabled) + apply(true, request(true)) + assertTrue(enabled) + } + + @Test + fun `explicitly disabled pulse stays disabled after hide show`() { + val requested = request(false) + repeat(3) { + apply(true, requested) + assertFalse(enabled) + apply(false, requested) + assertFalse(enabled) + } + } + + @Test + fun `hiding clears a previously enabled pulse even without a pulse prop`() { + enabled = true + apply(false, null) + assertFalse(enabled) + } +} From 77c16c816c5fb1170c61ae205d7779675ac34dcd Mon Sep 17 00:00:00 2001 From: Anthony Trasatti Date: Wed, 4 Nov 2026 09:57:26 -0500 Subject: [PATCH 3/4] chore(android): keep CI unchanged for hidden puck fix --- .github/workflows/android-actions.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/android-actions.yml b/.github/workflows/android-actions.yml index 0dbf317b3..17f24c261 100644 --- a/.github/workflows/android-actions.yml +++ b/.github/workflows/android-actions.yml @@ -78,7 +78,7 @@ jobs: - run: yarn postinstall working-directory: example - - run: ./gradlew assemble :rnmapbox_maps:testDebugUnitTest + - run: ./gradlew assemble working-directory: example/android env: CI_MAP_IMPL: ${{ inputs.MAP_IMPL }} From 0c118954f5dd59d2902dc1e65e78aa83b9e01007 Mon Sep 17 00:00:00 2001 From: Anthony Trasatti Date: Wed, 4 Nov 2026 10:34:27 -0500 Subject: [PATCH 4/4] fix(android): simplify hidden puck handling after review --- .github/workflows/android-actions.yml | 2 +- .../location/LocationComponentManager.kt | 13 ++- .../location/RNMBXNativeUserLocation.kt | 87 ++++++++---------- .../location/LocationComponentManagerTest.kt | 43 +-------- .../location/RNMBXNativeUserLocationTest.kt | 91 ------------------- 5 files changed, 49 insertions(+), 187 deletions(-) delete mode 100644 android/src/test/kotlin/com/rnmapbox/rnmbx/components/location/RNMBXNativeUserLocationTest.kt diff --git a/.github/workflows/android-actions.yml b/.github/workflows/android-actions.yml index 17f24c261..0dbf317b3 100644 --- a/.github/workflows/android-actions.yml +++ b/.github/workflows/android-actions.yml @@ -78,7 +78,7 @@ jobs: - run: yarn postinstall working-directory: example - - run: ./gradlew assemble + - run: ./gradlew assemble :rnmapbox_maps:testDebugUnitTest working-directory: example/android env: CI_MAP_IMPL: ${{ inputs.MAP_IMPL }} diff --git a/android/src/main/java/com/rnmapbox/rnmbx/components/location/LocationComponentManager.kt b/android/src/main/java/com/rnmapbox/rnmbx/components/location/LocationComponentManager.kt index d03fb0634..9a70bd612 100644 --- a/android/src/main/java/com/rnmapbox/rnmbx/components/location/LocationComponentManager.kt +++ b/android/src/main/java/com/rnmapbox/rnmbx/components/location/LocationComponentManager.kt @@ -31,10 +31,10 @@ import com.rnmapbox.rnmbx.v11compat.location.PuckBearing * LocationComponentManager attempts to separate that, so that Camera can ask for location tracking independent of display of user current location. * And NativeUserLocation can ask for display of user's current location - independent of Camera's user tracking. */ -class LocationComponentManager @JvmOverloads constructor( +class LocationComponentManager( mapView: RNMBXMapView, context: Context, - private val mLocationManager: LocationManager = LocationManager.getInstance(context), + private val locationManager: LocationManager = LocationManager.getInstance(context), ) { private val MAPBOX_BLUE_COLOR = Color.parseColor("#4A90E2") @@ -189,14 +189,14 @@ class LocationComponentManager @JvmOverloads constructor( private fun startLocationManager() { if (!mLocationManagerStarted) { - mLocationManager.startCounted() + locationManager.startCounted() mLocationManagerStarted = true } } private fun stopLocationManager() { if (mLocationManagerStarted) { - mLocationManager.stopCounted() + locationManager.stopCounted() mLocationManagerStarted = false } } @@ -204,13 +204,12 @@ class LocationComponentManager @JvmOverloads constructor( private fun useMapLocationProvider(mapView: RNMBXMapView) { val provider = mapView.mapPlugins.location.getLocationProvider() if (provider != null) { - mLocationManager.provider = provider + locationManager.provider = provider } } - @JvmOverloads - fun showNativeUserLocation(showUserLocation: Boolean, visible: Boolean = true) { + fun showNativeUserLocation(showUserLocation: Boolean, visible: Boolean) { update { it.copy(showUserLocation = showUserLocation && visible, nativeUserLocation = showUserLocation) } diff --git a/android/src/main/java/com/rnmapbox/rnmbx/components/location/RNMBXNativeUserLocation.kt b/android/src/main/java/com/rnmapbox/rnmbx/components/location/RNMBXNativeUserLocation.kt index d91ac1f3f..b00118c81 100644 --- a/android/src/main/java/com/rnmapbox/rnmbx/components/location/RNMBXNativeUserLocation.kt +++ b/android/src/main/java/com/rnmapbox/rnmbx/components/location/RNMBXNativeUserLocation.kt @@ -20,7 +20,6 @@ import com.mapbox.maps.MapboxMap import com.mapbox.maps.Style import com.mapbox.maps.plugin.LocationPuck2D import com.mapbox.maps.plugin.LocationPuck3D -import com.mapbox.maps.plugin.locationcomponent.LocationComponentPlugin import com.mapbox.maps.plugin.locationcomponent.LocationComponentConstants import com.mapbox.maps.plugin.locationcomponent.location import com.mapbox.maps.plugin.locationcomponent.R as LR @@ -135,7 +134,6 @@ class RNMBXNativeUserLocation(context: Context) : AbstractMapFeature(context), O } private fun _apply(mapView: MapView) { - // Keep location interpolation active for camera following, even with no visible puck. mMapView?.locationComponentManager?.showNativeUserLocation(mEnabled, visible) val location2 = mapView.location2; @@ -170,7 +168,43 @@ class RNMBXNativeUserLocation(context: Context) : AbstractMapFeature(context), O location2.puckBearingEnabled = it } - applyLocationPuckPulsing(location2, visible, pulsing, mContext) + pulsing?.let { pulsing -> + pulsing.getAndLogIfNotString("kind")?.also { kind -> + if (kind == "default") { + location2.pulsingEnabled = true + } + } + if (pulsing.hasKey("color")) { + when (pulsing.getType("color")) { + ReadableType.Map -> + location2.pulsingColor = ColorPropConverter.getColor(pulsing.getMap("color"), mContext) ?: 0 + ReadableType.Number -> + location2.pulsingColor = pulsing.getInt("color") + else -> + Logger.e(LOG_TAG, "pusling.color should be either a map or a number, but was ${pulsing.getDynamic("color")}") + } + } + pulsing.getAndLogIfNotBoolean("isEnabled")?.let { enabled -> + location2.pulsingEnabled = enabled + } + if (pulsing.hasKey("radius")) { + when (pulsing.getType("radius")) { + ReadableType.Number -> + location2.pulsingMaxRadius = pulsing.getDouble("radius").toFloat() + ReadableType.String -> + if (pulsing.getString("radius") == "accuracy") { + location2.pulsingMaxRadius = LocationComponentConstants.PULSING_MAX_RADIUS_FOLLOW_ACCURACY + } else { + Logger.e(LOG_TAG, "Expected pulsing/radius to be a number or accuracy but was ${pulsing.getString("radius")}") + } + else -> + Logger.e(LOG_TAG, "Expected pulsing/radius to be a number or accuracy but was ${pulsing.getString("radius")}") + } + } + } + if (!visible) { + location2.pulsingEnabled = false + } } private fun makeLocationPuck3D(model: ReadableMap): LocationPuck3D? { @@ -196,7 +230,6 @@ class RNMBXNativeUserLocation(context: Context) : AbstractMapFeature(context), O mEnabled = true mapView.getMapboxMap() mapView.getMapAsync(this) - mMapView?.locationComponentManager?.showNativeUserLocation(true, visible) mMBXMapView = mapView _fetchImages(mapView) _apply() @@ -204,7 +237,7 @@ class RNMBXNativeUserLocation(context: Context) : AbstractMapFeature(context), O override fun removeFromMap(mapView: RNMBXMapView, reason: RemovalReason): Boolean { mEnabled = false - mMapView?.locationComponentManager?.showNativeUserLocation(false) + mMapView?.locationComponentManager?.showNativeUserLocation(false, visible) mMap?.getStyle(this) mMBXMapView = null return super.removeFromMap(mapView, reason) @@ -308,47 +341,3 @@ fun makeDefaultLocationPuck2D(context: Context, renderMode: RenderMode): Locatio ) ); } -internal fun applyLocationPuckPulsing( - location: LocationComponentPlugin, - visible: Boolean, - pulsing: ReadableMap?, - context: Context, -) { - if (!visible) { - location.pulsingEnabled = false - } - pulsing?.let { pulsing -> - pulsing.getAndLogIfNotString("kind")?.also { kind -> - if (kind == "default") { - location.pulsingEnabled = visible - } - } - if (pulsing.hasKey("color")) { - when (pulsing.getType("color")) { - ReadableType.Map -> - location.pulsingColor = ColorPropConverter.getColor(pulsing.getMap("color"), context) ?: 0 - ReadableType.Number -> - location.pulsingColor = pulsing.getInt("color") - else -> - Logger.e("RNMBXNativeUserLocation", "pusling.color should be either a map or a number, but was ${pulsing.getDynamic("color")}") - } - } - pulsing.getAndLogIfNotBoolean("isEnabled")?.let { enabled -> - location.pulsingEnabled = visible && enabled - } - if (pulsing.hasKey("radius")) { - when (pulsing.getType("radius")) { - ReadableType.Number -> - location.pulsingMaxRadius = pulsing.getDouble("radius").toFloat() - ReadableType.String -> - if (pulsing.getString("radius") == "accuracy") { - location.pulsingMaxRadius = LocationComponentConstants.PULSING_MAX_RADIUS_FOLLOW_ACCURACY - } else { - Logger.e("RNMBXNativeUserLocation", "Expected pulsing/radius to be a number or accuracy but was ${pulsing.getString("radius")}") - } - else -> - Logger.e("RNMBXNativeUserLocation", "Expected pulsing/radius to be a number or accuracy but was ${pulsing.getString("radius")}") - } - } - } -} diff --git a/android/src/test/kotlin/com/rnmapbox/rnmbx/components/location/LocationComponentManagerTest.kt b/android/src/test/kotlin/com/rnmapbox/rnmbx/components/location/LocationComponentManagerTest.kt index f64c65ca2..d014f0607 100644 --- a/android/src/test/kotlin/com/rnmapbox/rnmbx/components/location/LocationComponentManagerTest.kt +++ b/android/src/test/kotlin/com/rnmapbox/rnmbx/components/location/LocationComponentManagerTest.kt @@ -6,7 +6,6 @@ import android.os.Handler import android.os.Looper import androidx.lifecycle.Lifecycle import com.mapbox.maps.plugin.LocationPuck2D -import com.mapbox.maps.plugin.LocationPuck3D import com.mapbox.maps.plugin.Plugin import com.mapbox.maps.plugin.delegates.MapPluginProviderDelegate import com.mapbox.maps.plugin.locationcomponent.LocationComponentPlugin @@ -24,20 +23,17 @@ class LocationComponentManagerTest { val provider: MapPluginProviderDelegate = mock() val map: RNMBXMapView = mock() val locationManager: LocationManager = mock() - var lifecycle = Lifecycle.State.STARTED var settings = LocationComponentSettings.Builder(LocationPuck2D()).build() - var settingsWrites = 0 val manager: LocationComponentManager init { whenever(provider.getPlugin(Plugin.MAPBOX_LOCATION_COMPONENT_PLUGIN_ID)) .thenReturn(location) whenever(map.mapPlugins).thenReturn(provider) - whenever(map.getLifecycleState()).thenAnswer { lifecycle } + whenever(map.getLifecycleState()).thenReturn(Lifecycle.State.STARTED) doAnswer { invocation -> val update = invocation.getArgument Unit>(0) settings = settings.toBuilder().apply(update).build() - settingsWrites++ null }.whenever(location).updateSettings(any()) manager = LocationComponentManager(map, mock(), locationManager) @@ -70,55 +66,24 @@ class LocationComponentManagerTest { } @Test - fun `hiding and removing a puck preserves active following`() = withManager { - manager.showNativeUserLocation(true) + fun `hiding a puck preserves active following`() = withManager { + manager.showNativeUserLocation(true, visible = true) manager.setFollowLocation(true) manager.showNativeUserLocation(true, visible = false) assertTrue(settings.enabled) - manager.showNativeUserLocation(false) - assertTrue(settings.enabled) verify(locationManager, never()).stopCounted() - manager.setFollowLocation(false) - assertFalse(settings.enabled) - verify(locationManager).stopCounted() } @Test fun `repeated hide show cycles balance location subscriptions`() = withManager { repeat(20) { - manager.showNativeUserLocation(true) + manager.showNativeUserLocation(true, visible = true) assertTrue(settings.enabled) manager.showNativeUserLocation(true, visible = false) assertFalse(settings.enabled) } verify(locationManager, times(20)).startCounted() verify(locationManager, times(20)).stopCounted() - manager.onDestroy() - verify(locationManager, times(20)).stopCounted() } - @Test - fun `manager leaves native owned puck configuration untouched during follow transitions`() = withManager { - for (puck in listOf(LocationPuck2D(scaleExpression = "2"), LocationPuck3D(modelUri = "asset://car.glb"))) { - settings = settings.toBuilder().apply { locationPuck = puck }.build() - manager.showNativeUserLocation(true) - manager.showNativeUserLocation(true, visible = false) - manager.setFollowLocation(true) - manager.showNativeUserLocation(true) - assertSame(puck, settings.locationPuck) - manager.setFollowLocation(false) - } - } - - @Test - fun `shutdown does not restart the plugin or stop the location manager twice`() = withManager { - manager.showNativeUserLocation(true) - val writesBeforeStop = settingsWrites - lifecycle = Lifecycle.State.CREATED - manager.showNativeUserLocation(true, visible = false) - assertEquals(writesBeforeStop, settingsWrites) - verify(locationManager).stopCounted() - manager.onDestroy() - verify(locationManager).stopCounted() - } } diff --git a/android/src/test/kotlin/com/rnmapbox/rnmbx/components/location/RNMBXNativeUserLocationTest.kt b/android/src/test/kotlin/com/rnmapbox/rnmbx/components/location/RNMBXNativeUserLocationTest.kt deleted file mode 100644 index 592d6ab3b..000000000 --- a/android/src/test/kotlin/com/rnmapbox/rnmbx/components/location/RNMBXNativeUserLocationTest.kt +++ /dev/null @@ -1,91 +0,0 @@ -package com.rnmapbox.rnmbx.components.location - -import android.content.Context -import com.facebook.react.bridge.ReadableMap -import com.facebook.react.bridge.ReadableType -import com.mapbox.maps.plugin.locationcomponent.LocationComponentPlugin -import org.junit.jupiter.api.Assertions.* -import org.junit.jupiter.api.Test -import org.mockito.kotlin.* - -class RNMBXNativeUserLocationTest { - private val location: LocationComponentPlugin = mock() - private val context: Context = mock() - private var enabled = false - - init { - doAnswer { enabled = it.getArgument(0); null } - .whenever(location).pulsingEnabled = any() - } - - private fun request(enabled: Boolean): ReadableMap = mock { - on { hasKey("isEnabled") } doReturn true - on { getType("isEnabled") } doReturn ReadableType.Boolean - on { getBoolean("isEnabled") } doReturn enabled - } - - private fun apply(visible: Boolean, request: ReadableMap?) { - applyLocationPuckPulsing(location, visible, request, context) - } - - @Test - fun `explicit pulse resumes after hide show without losing configuration`() { - val requested = request(true) - whenever(requested.hasKey("radius")).thenReturn(true) - whenever(requested.getType("radius")).thenReturn(ReadableType.Number) - whenever(requested.getDouble("radius")).thenReturn(25.0) - whenever(requested.hasKey("color")).thenReturn(true) - whenever(requested.getType("color")).thenReturn(ReadableType.Number) - whenever(requested.getInt("color")).thenReturn(0xFF123456.toInt()) - apply(true, requested) - assertTrue(enabled) - apply(false, requested) - assertFalse(enabled) - apply(true, requested) - assertTrue(enabled) - verify(location, times(3)).pulsingMaxRadius = 25f - verify(location, times(3)).pulsingColor = 0xFF123456.toInt() - } - - @Test - fun `default pulse is suppressed while hidden and restored when visible`() { - val requested: ReadableMap = mock { - on { hasKey("kind") } doReturn true - on { getType("kind") } doReturn ReadableType.String - on { getString("kind") } doReturn "default" - } - apply(true, requested) - assertTrue(enabled) - apply(false, requested) - assertFalse(enabled) - apply(true, requested) - assertTrue(enabled) - } - - @Test - fun `pulse changes while hidden are deferred until visible`() { - apply(false, request(false)) - apply(false, request(true)) - assertFalse(enabled) - apply(true, request(true)) - assertTrue(enabled) - } - - @Test - fun `explicitly disabled pulse stays disabled after hide show`() { - val requested = request(false) - repeat(3) { - apply(true, requested) - assertFalse(enabled) - apply(false, requested) - assertFalse(enabled) - } - } - - @Test - fun `hiding clears a previously enabled pulse even without a pulse prop`() { - enabled = true - apply(false, null) - assertFalse(enabled) - } -}