From 8c4e8a3d37507b9fcbce27f6fdedc76a0383b32b Mon Sep 17 00:00:00 2001 From: Anthony Trasatti Date: Sat, 31 Oct 2026 08:59:12 -0400 Subject: [PATCH 1/2] [location] Stop pulsing while the puck is hidden or stopped --- CHANGELOG.md | 3 ++ .../LocationIndicatorLayerRenderer.kt | 3 ++ .../locationcomponent/LocationPuckManager.kt | 3 ++ .../animators/PuckAnimatorManager.kt | 32 ++++++++---- .../animators/PuckPulsingAnimator.kt | 12 ++--- .../LocationIndicatorLayerRendererTest.kt | 14 +++++ .../LocationPuckManagerTest.kt | 21 ++++++++ .../animators/PuckAnimatorManagerTest.kt | 51 +++++++++++++++++++ .../animators/PuckPulsingAnimatorTest.kt | 8 +++ 9 files changed, 130 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e1171e1d7f..7f1a3b62e9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,9 @@ Mapbox welcomes participation and contributions from everyone. # main +## Bug fixes 🐞 +* Stop location puck pulse animations while the puck is hidden or the location component is stopped. + ## Features ✨ and improvements 🏁 * Accessing a getter for a plugin that was intentionally disabled (compass, logo, attribution, scalebar, lifecycle, indoorSelector) because the Compose extension replaces with its own APIs, now throws a descriptive `PluginDisabledException`, instead of an unhelpful `NullPointerException`. diff --git a/plugin-locationcomponent/src/main/java/com/mapbox/maps/plugin/locationcomponent/LocationIndicatorLayerRenderer.kt b/plugin-locationcomponent/src/main/java/com/mapbox/maps/plugin/locationcomponent/LocationIndicatorLayerRenderer.kt index 27b696b426..c6922c4105 100644 --- a/plugin-locationcomponent/src/main/java/com/mapbox/maps/plugin/locationcomponent/LocationIndicatorLayerRenderer.kt +++ b/plugin-locationcomponent/src/main/java/com/mapbox/maps/plugin/locationcomponent/LocationIndicatorLayerRenderer.kt @@ -27,6 +27,7 @@ internal class LocationIndicatorLayerRenderer( ) : LocationLayerRenderer { private var style: MapboxStyleManager? = null + private var isVisible = false override fun initializeComponents(style: MapboxStyleManager) { this.style = style @@ -126,6 +127,7 @@ internal class LocationIndicatorLayerRenderer( } private fun setLayerVisibility(visible: Boolean) { + isVisible = visible layer.visibility(visible) } @@ -156,6 +158,7 @@ internal class LocationIndicatorLayerRenderer( radius: Float, opacity: Float? ) { + if (!isVisible) return val rgbaArray = colorToRgbaArray(pulsingColorInt) rgbaArray[3] = opacity ?: 1f layer.emphasisCircleRadius(radius.toDouble()) diff --git a/plugin-locationcomponent/src/main/java/com/mapbox/maps/plugin/locationcomponent/LocationPuckManager.kt b/plugin-locationcomponent/src/main/java/com/mapbox/maps/plugin/locationcomponent/LocationPuckManager.kt index b7db0514e1..575deacc36 100644 --- a/plugin-locationcomponent/src/main/java/com/mapbox/maps/plugin/locationcomponent/LocationPuckManager.kt +++ b/plugin-locationcomponent/src/main/java/com/mapbox/maps/plugin/locationcomponent/LocationPuckManager.kt @@ -83,6 +83,7 @@ internal class LocationPuckManager( onBearingUpdated, onAccuracyRadiusUpdated ) + animationManager.onPuckVisibilityChanged(false) animationManager.setLocationLayerRenderer(locationLayerRenderer) animationManager.applySettings(settings) lastLocation?.let { @@ -236,11 +237,13 @@ internal class LocationPuckManager( if (forceUpdate || isHidden) { isHidden = false locationLayerRenderer.show() + animationManager.onPuckVisibilityChanged(true) } } fun hide() { isHidden = true + animationManager.onPuckVisibilityChanged(false) locationLayerRenderer.hide() } diff --git a/plugin-locationcomponent/src/main/java/com/mapbox/maps/plugin/locationcomponent/animators/PuckAnimatorManager.kt b/plugin-locationcomponent/src/main/java/com/mapbox/maps/plugin/locationcomponent/animators/PuckAnimatorManager.kt index 318e0a747e..7c073dc2bd 100644 --- a/plugin-locationcomponent/src/main/java/com/mapbox/maps/plugin/locationcomponent/animators/PuckAnimatorManager.kt +++ b/plugin-locationcomponent/src/main/java/com/mapbox/maps/plugin/locationcomponent/animators/PuckAnimatorManager.kt @@ -23,6 +23,9 @@ internal class PuckAnimatorManager( PuckAccuracyRadiusAnimator(indicatorAccuracyRadiusChangedListener) private var pulsingAnimator = PuckPulsingAnimator(pixelRatio) + private var isStarted = false + private var isPuckVisible = false + internal var puckAnimationEnabled: Boolean get() = bearingAnimator.enabled set(value) { @@ -69,18 +72,31 @@ internal class PuckAnimatorManager( } fun onStart() { - if (pulsingAnimator.enabled) { - pulsingAnimator.animateInfinite() - } + isStarted = true + updatePulsingState() } fun onStop() { + isStarted = false bearingAnimator.cancelRunning() positionAnimator.cancelRunning() pulsingAnimator.cancelRunning() accuracyRadiusAnimator.cancelRunning() } + fun onPuckVisibilityChanged(visible: Boolean) { + isPuckVisible = visible + updatePulsingState() + } + + private fun updatePulsingState() { + if (isStarted && isPuckVisible && pulsingAnimator.enabled) { + pulsingAnimator.animateInfinite() + } else { + pulsingAnimator.cancelRunning() + } + } + fun animateBearing( vararg targets: Double, options: (ValueAnimator.() -> Unit)? @@ -110,11 +126,9 @@ internal class PuckAnimatorManager( enabled = settings.pulsingEnabled if (settings.pulsingEnabled) { maxRadius = target - animateInfinite() - } else { - cancelRunning() } } + updatePulsingState() } fun applySettings(settings: LocationComponentSettings) { @@ -122,12 +136,8 @@ internal class PuckAnimatorManager( enabled = settings.pulsingEnabled maxRadius = settings.pulsingMaxRadius.toDouble() pulsingColor = settings.pulsingColor - if (settings.pulsingEnabled) { - animateInfinite() - } else { - cancelRunning() - } } + updatePulsingState() accuracyRadiusAnimator.apply { enabled = settings.showAccuracyRing accuracyCircleColor = settings.accuracyRingColor diff --git a/plugin-locationcomponent/src/main/java/com/mapbox/maps/plugin/locationcomponent/animators/PuckPulsingAnimator.kt b/plugin-locationcomponent/src/main/java/com/mapbox/maps/plugin/locationcomponent/animators/PuckPulsingAnimator.kt index 998bdf61c2..5e89574275 100644 --- a/plugin-locationcomponent/src/main/java/com/mapbox/maps/plugin/locationcomponent/animators/PuckPulsingAnimator.kt +++ b/plugin-locationcomponent/src/main/java/com/mapbox/maps/plugin/locationcomponent/animators/PuckPulsingAnimator.kt @@ -21,6 +21,12 @@ internal class PuckPulsingAnimator(private val pixelRatio: Float = 1.0f) : repeatMode = RESTART repeatCount = INFINITE interpolator = PULSING_DEFAULT_INTERPOLATOR + addListener(object : AnimatorListenerAdapter() { + override fun onAnimationRepeat(animation: Animator) { + super.onAnimationRepeat(animation) + setObjectValues(0.0, maxRadius) + } + }) } fun animateInfinite() { @@ -30,12 +36,6 @@ internal class PuckPulsingAnimator(private val pixelRatio: Float = 1.0f) : if (!isRunning) { animate(0.0, maxRadius) } - addListener(object : AnimatorListenerAdapter() { - override fun onAnimationRepeat(animation: Animator) { - super.onAnimationRepeat(animation) - setObjectValues(0.0, maxRadius) - } - }) } override fun updateLayer(fraction: Float, value: Double) { diff --git a/plugin-locationcomponent/src/test/java/com/mapbox/maps/plugin/locationcomponent/LocationIndicatorLayerRendererTest.kt b/plugin-locationcomponent/src/test/java/com/mapbox/maps/plugin/locationcomponent/LocationIndicatorLayerRendererTest.kt index f26958c5b2..8da54fcfaa 100644 --- a/plugin-locationcomponent/src/test/java/com/mapbox/maps/plugin/locationcomponent/LocationIndicatorLayerRendererTest.kt +++ b/plugin-locationcomponent/src/test/java/com/mapbox/maps/plugin/locationcomponent/LocationIndicatorLayerRendererTest.kt @@ -70,6 +70,20 @@ class LocationIndicatorLayerRendererTest { locationLayerRenderer.initializeComponents(style) } + @Test + fun hiddenRendererIgnoresPendingPulseUpdates() { + locationLayerRenderer.updatePulsingUi(Color.BLUE, 10f, 0.5f) + verify(exactly = 0) { layerWrapper.emphasisCircleRadius(any()) } + locationLayerRenderer.show() + locationLayerRenderer.updatePulsingUi(Color.BLUE, 10f, 0.5f) + verify(exactly = 1) { layerWrapper.emphasisCircleRadius(10.0) } + clearMocks(layerWrapper, answers = false) + locationLayerRenderer.hide() + locationLayerRenderer.updatePulsingUi(Color.BLUE, 15f, 0.4f) + verify(exactly = 0) { layerWrapper.emphasisCircleRadius(any()) } + verify(exactly = 0) { layerWrapper.emphasisCircleColor(any()) } + } + @Test fun sanity() { Assert.assertNotNull(locationLayerRenderer) diff --git a/plugin-locationcomponent/src/test/java/com/mapbox/maps/plugin/locationcomponent/LocationPuckManagerTest.kt b/plugin-locationcomponent/src/test/java/com/mapbox/maps/plugin/locationcomponent/LocationPuckManagerTest.kt index 322b639534..6acffd5de9 100644 --- a/plugin-locationcomponent/src/test/java/com/mapbox/maps/plugin/locationcomponent/LocationPuckManagerTest.kt +++ b/plugin-locationcomponent/src/test/java/com/mapbox/maps/plugin/locationcomponent/LocationPuckManagerTest.kt @@ -97,6 +97,27 @@ class LocationPuckManagerTest { unmockkAll() } + @Test + fun visibilityGatesPulsingAcrossLocationAndStyleReload() { + locationPuckManager.initialize(style) + verify { animationManager.onPuckVisibilityChanged(false) } + verify(exactly = 0) { animationManager.onPuckVisibilityChanged(true) } + + locationPuckManager.updateCurrentPosition(Point.fromLngLat(10.0, 20.0)) + verify { animationManager.onPuckVisibilityChanged(true) } + locationPuckManager.hide() + assertTrue(locationPuckManager.isHidden) + + io.mockk.clearMocks(animationManager, answers = false) + locationPuckManager.lastLocation = Point.fromLngLat(10.0, 20.0) + locationPuckManager.updateSettings(settings) + verifyOrder { + animationManager.onPuckVisibilityChanged(false) + animationManager.applySettings(settings) + animationManager.onPuckVisibilityChanged(true) + } + } + @Test fun testInitialise() { locationPuckManager.initialize(style) diff --git a/plugin-locationcomponent/src/test/java/com/mapbox/maps/plugin/locationcomponent/animators/PuckAnimatorManagerTest.kt b/plugin-locationcomponent/src/test/java/com/mapbox/maps/plugin/locationcomponent/animators/PuckAnimatorManagerTest.kt index e8ea83fdab..a8fb0b6e98 100644 --- a/plugin-locationcomponent/src/test/java/com/mapbox/maps/plugin/locationcomponent/animators/PuckAnimatorManagerTest.kt +++ b/plugin-locationcomponent/src/test/java/com/mapbox/maps/plugin/locationcomponent/animators/PuckAnimatorManagerTest.kt @@ -6,6 +6,7 @@ import android.os.Looper import com.mapbox.geojson.Point import com.mapbox.maps.plugin.locationcomponent.LocationLayerRenderer import com.mapbox.maps.plugin.locationcomponent.generated.LocationComponentSettings +import io.mockk.clearMocks import io.mockk.every import io.mockk.mockk import io.mockk.verify @@ -36,6 +37,9 @@ class PuckAnimatorManagerTest { @Before fun setUp() { + var pulsingEnabled = false + every { pulsingAnimator.enabled } answers { pulsingEnabled } + every { pulsingAnimator.enabled = any() } answers { pulsingEnabled = firstArg() } puckAnimatorManager = PuckAnimatorManager( mockk(), mockk(), @@ -63,6 +67,7 @@ class PuckAnimatorManagerTest { @Test fun onStartOne() { every { pulsingAnimator.enabled } returns true + puckAnimatorManager.onPuckVisibilityChanged(true) puckAnimatorManager.onStart() verify(exactly = 1) { pulsingAnimator.animateInfinite() } } @@ -162,6 +167,8 @@ class PuckAnimatorManagerTest { @Test fun applyAnimationSettingsOne() { + puckAnimatorManager.onPuckVisibilityChanged(true) + puckAnimatorManager.onStart() val settings = mockk(relaxed = true) every { settings.pulsingEnabled } returns true every { settings.pulsingMaxRadius } returns 20.0f @@ -222,6 +229,8 @@ class PuckAnimatorManagerTest { @Test fun testUpdatePulsingPuckRadius() { + puckAnimatorManager.onPuckVisibilityChanged(true) + puckAnimatorManager.onStart() val updatedRadius = 50.0 val settings = mockk(relaxed = true) every { settings.pulsingEnabled } returns true @@ -250,6 +259,48 @@ class PuckAnimatorManagerTest { verify(exactly = 0) { pulsingAnimator.maxRadius = updatedRadius } } + @Test + fun pulsingWaitsForVisibilityAndResumesAfterStop() { + val settings = mockk(relaxed = true) + every { settings.pulsingEnabled } returns true + puckAnimatorManager.applySettings(settings) + puckAnimatorManager.onStart() + verify(exactly = 0) { pulsingAnimator.animateInfinite() } + + puckAnimatorManager.onPuckVisibilityChanged(true) + verify(exactly = 1) { pulsingAnimator.animateInfinite() } + clearMocks(pulsingAnimator, answers = false) + + puckAnimatorManager.onPuckVisibilityChanged(false) + puckAnimatorManager.updatePulsingRadius(50.0, settings) + puckAnimatorManager.applySettings(settings) + verify(exactly = 0) { pulsingAnimator.animateInfinite() } + verify { pulsingAnimator.cancelRunning() } + + puckAnimatorManager.onPuckVisibilityChanged(true) + verify(exactly = 1) { pulsingAnimator.animateInfinite() } + puckAnimatorManager.onStop() + clearMocks(pulsingAnimator, answers = false) + puckAnimatorManager.updatePulsingRadius(60.0, settings) + puckAnimatorManager.applySettings(settings) + verify(exactly = 0) { pulsingAnimator.animateInfinite() } + puckAnimatorManager.onStart() + verify(exactly = 1) { pulsingAnimator.animateInfinite() } + } + + @Test + fun visibilityDoesNotOverrideDisabledPulsing() { + val settings = mockk(relaxed = true) + every { settings.pulsingEnabled } returns false + puckAnimatorManager.applySettings(settings) + puckAnimatorManager.onStart() + repeat(3) { + puckAnimatorManager.onPuckVisibilityChanged(true) + puckAnimatorManager.onPuckVisibilityChanged(false) + } + verify(exactly = 0) { pulsingAnimator.animateInfinite() } + } + companion object { private val START_POINT = Point.fromLngLat(-122.4194, 37.7749) private val END_POINT = Point.fromLngLat(-77.0369, 38.9072) diff --git a/plugin-locationcomponent/src/test/java/com/mapbox/maps/plugin/locationcomponent/animators/PuckPulsingAnimatorTest.kt b/plugin-locationcomponent/src/test/java/com/mapbox/maps/plugin/locationcomponent/animators/PuckPulsingAnimatorTest.kt index 9ec82d608a..3d54c93c3a 100644 --- a/plugin-locationcomponent/src/test/java/com/mapbox/maps/plugin/locationcomponent/animators/PuckPulsingAnimatorTest.kt +++ b/plugin-locationcomponent/src/test/java/com/mapbox/maps/plugin/locationcomponent/animators/PuckPulsingAnimatorTest.kt @@ -23,6 +23,14 @@ class PuckPulsingAnimatorTest { pulsingAnimator = spyk(animator) } + @Test + fun repeatedStartsDoNotAccumulateRepeatListeners() { + every { pulsingAnimator.animate(any(), any()) } just Runs + val listenerCount = pulsingAnimator.listeners?.size ?: 0 + repeat(5) { pulsingAnimator.animateInfinite() } + Assert.assertEquals(listenerCount, pulsingAnimator.listeners?.size ?: 0) + } + @Test fun creation() { Assert.assertEquals(PuckPulsingAnimator.PULSING_DEFAULT_DURATION, pulsingAnimator.duration) From f0f5c6bdbb60198a7116d9205962de3c19820af5 Mon Sep 17 00:00:00 2001 From: Anthony Trasatti Date: Sat, 31 Oct 2026 09:16:45 -0400 Subject: [PATCH 2/2] [location] Prevent settings from reactivating a stopped component --- .../LocationComponentPluginImpl.kt | 6 +++++- .../LocationComponentPluginImplTest.kt | 16 ++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/plugin-locationcomponent/src/main/java/com/mapbox/maps/plugin/locationcomponent/LocationComponentPluginImpl.kt b/plugin-locationcomponent/src/main/java/com/mapbox/maps/plugin/locationcomponent/LocationComponentPluginImpl.kt index cb60748bba..3dcba35bf2 100644 --- a/plugin-locationcomponent/src/main/java/com/mapbox/maps/plugin/locationcomponent/LocationComponentPluginImpl.kt +++ b/plugin-locationcomponent/src/main/java/com/mapbox/maps/plugin/locationcomponent/LocationComponentPluginImpl.kt @@ -35,6 +35,8 @@ internal class LocationComponentPluginImpl : LocationComponentPlugin, LocationCo private lateinit var weakContext: WeakReference + private var isStarted = false + @VisibleForTesting(otherwise = PRIVATE) internal var locationPuckManager: LocationPuckManager? = null @@ -200,11 +202,12 @@ internal class LocationComponentPluginImpl : LocationComponentPlugin, LocationCo * Called whenever activity's/fragment's lifecycle is entering a "started" state. */ override fun onStart() { + isStarted = true activateLocationComponent() } private fun activateLocationComponent() { - if (internalSettings.enabled) { + if (isStarted && internalSettings.enabled) { val style = delegateProvider.mapStyleManagerDelegate if (locationPuckManager?.isLayerInitialised() == true && isLocationComponentActivated) { return @@ -238,6 +241,7 @@ internal class LocationComponentPluginImpl : LocationComponentPlugin, LocationCo * Called whenever activity's/fragment's lifecycle is entering a "stopped" state. */ override fun onStop() { + isStarted = false isLocationComponentActivated = false locationPuckManager?.onStop() locationProvider?.unRegisterLocationConsumer(this) diff --git a/plugin-locationcomponent/src/test/java/com/mapbox/maps/plugin/locationcomponent/LocationComponentPluginImplTest.kt b/plugin-locationcomponent/src/test/java/com/mapbox/maps/plugin/locationcomponent/LocationComponentPluginImplTest.kt index f647206201..2045d0817a 100644 --- a/plugin-locationcomponent/src/test/java/com/mapbox/maps/plugin/locationcomponent/LocationComponentPluginImplTest.kt +++ b/plugin-locationcomponent/src/test/java/com/mapbox/maps/plugin/locationcomponent/LocationComponentPluginImplTest.kt @@ -94,6 +94,22 @@ class LocationComponentPluginImplTest { locationComponentPlugin = LocationComponentPluginImpl() } + @Test + fun settingsWhileStoppedDoNotReactivateLocationComponent() { + preparePluginInitialisationWithEnabled() + locationComponentPlugin.onStop() + io.mockk.clearMocks(locationPuckManager, locationProvider, answers = false) + + locationComponentPlugin.pulsingEnabled = true + locationComponentPlugin.pulsingColor = Color.GREEN + verify(exactly = 0) { locationPuckManager.onStart() } + verify(exactly = 0) { locationProvider.registerLocationConsumer(any()) } + + locationComponentPlugin.onStart() + verify(exactly = 1) { locationPuckManager.onStart() } + verify(exactly = 1) { locationProvider.registerLocationConsumer(any()) } + } + @After fun cleanUp() { unmockkAll()