fix(android): stop unnecessary hidden location puck rendering

.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 }}

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(
+    mapView: RNMBXMapView,
+    context: Context,
+    private val locationManager: 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
@@ -187,29 +189,29 @@ class LocationComponentManager(mapView: RNMBXMapView, context: Context) {
 
     private fun startLocationManager() {
         if (!mLocationManagerStarted) {
-            mLocationManager.startCounted()
+            locationManager.startCounted()
             mLocationManagerStarted = true
         }
     }
 
     private fun stopLocationManager() {
         if (mLocationManagerStarted) {
-            mLocationManager.stopCounted()
+            locationManager.stopCounted()
             mLocationManagerStarted = false
         }
     }
 
     private fun useMapLocationProvider(mapView: RNMBXMapView) {
         val provider = mapView.mapPlugins.location.getLocationProvider()
         if (provider != null) {
-            mLocationManager.provider = provider
+            locationManager.provider = provider
         }
     }
 
 
-    fun showNativeUserLocation(showUserLocation: Boolean) {
+    fun showNativeUserLocation(showUserLocation: Boolean, visible: Boolean) {
         update {
-            it.copy(showUserLocation = showUserLocation, nativeUserLocation= showUserLocation)
+            it.copy(showUserLocation = showUserLocation && visible, nativeUserLocation = showUserLocation)
         }
     }
 

android/src/main/java/com/rnmapbox/rnmbx/components/location/RNMBXNativeUserLocation.kt

@@ -134,6 +134,7 @@ class RNMBXNativeUserLocation(context: Context) : AbstractMapFeature(context), O
     }
 
     private fun _apply(mapView: MapView) {
+        mMapView?.locationComponentManager?.showNativeUserLocation(mEnabled, visible)
         val location2 = mapView.location2;
 
         val locationPuck3D = model?.let { makeLocationPuck3D(it) }
@@ -201,6 +202,9 @@ class RNMBXNativeUserLocation(context: Context) : AbstractMapFeature(context), O
                 }
             }
         }
+        if (!visible) {
+            location2.pulsingEnabled = false
+        }
     }
 
     private fun makeLocationPuck3D(model: ReadableMap): LocationPuck3D? {
@@ -226,15 +230,14 @@ class RNMBXNativeUserLocation(context: Context) : AbstractMapFeature(context), O
         mEnabled = true
         mapView.getMapboxMap()
         mapView.getMapAsync(this)
-        mMapView?.locationComponentManager?.showNativeUserLocation(true)
         mMBXMapView = mapView
         _fetchImages(mapView)
         _apply()
     }
 
     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)
@@ -255,7 +258,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
@@ -337,4 +340,4 @@ fun makeDefaultLocationPuck2D(context: Context, renderMode: RenderMode): Locatio
             LR.drawable.mapbox_user_icon_shadow
         )
     );
-}
+}

android/src/test/kotlin/com/rnmapbox/rnmbx/components/location/LocationComponentManagerTest.kt

@@ -0,0 +1,89 @@
+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.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 settings = LocationComponentSettings.Builder(LocationPuck2D()).build()
+        val manager: LocationComponentManager
+
+        init {
+            whenever(provider.getPlugin<LocationComponentPlugin>(Plugin.MAPBOX_LOCATION_COMPONENT_PLUGIN_ID))
+                .thenReturn(location)
+            whenever(map.mapPlugins).thenReturn(provider)
+            whenever(map.getLifecycleState()).thenReturn(Lifecycle.State.STARTED)
+            doAnswer { invocation ->
+                val update = invocation.getArgument<LocationComponentSettings.Builder.() -> Unit>(0)
+                settings = settings.toBuilder().apply(update).build()
+                null
+            }.whenever(location).updateSettings(any())
+            manager = LocationComponentManager(map, mock<Context>(), 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 a puck preserves active following`() = withManager {
+        manager.showNativeUserLocation(true, visible = true)
+        manager.setFollowLocation(true)
+        manager.showNativeUserLocation(true, visible = false)
+        assertTrue(settings.enabled)
+        verify(locationManager, never()).stopCounted()
+    }
+
+    @Test
+    fun `repeated hide show cycles balance location subscriptions`() = withManager {
+        repeat(20) {
+            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()
+    }
+
+}