Updating user pins on location update

vector/src/main/java/im/vector/app/features/location/LocationSharingFragment.kt

@@ -181,7 +181,7 @@ class LocationSharingFragment @Inject constructor(
     }
 
     private fun handleZoomToUserLocationEvent(event: LocationSharingViewEvents.ZoomToUserLocation) {
-        views.mapView.zoomToLocation(event.userLocation.latitude, event.userLocation.longitude)
+        views.mapView.zoomToLocation(event.userLocation)
     }
 
     private fun handleStartLiveLocationService(event: LocationSharingViewEvents.StartLiveLocationService) {

vector/src/main/java/im/vector/app/features/location/MapBoxMapExt.kt

@@ -0,0 +1,39 @@
+/*
+ * Copyright (c) 2022 New Vector Ltd
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package im.vector.app.features.location
+
+import com.mapbox.mapboxsdk.camera.CameraPosition
+import com.mapbox.mapboxsdk.constants.MapboxConstants
+import com.mapbox.mapboxsdk.geometry.LatLng
+import com.mapbox.mapboxsdk.geometry.LatLngBounds
+import com.mapbox.mapboxsdk.maps.MapboxMap
+
+fun MapboxMap?.zoomToLocation(locationData: LocationData) {
+    this?.cameraPosition = CameraPosition.Builder()
+            .target(LatLng(locationData.latitude, locationData.longitude))
+            .zoom(INITIAL_MAP_ZOOM_IN_PREVIEW)
+            .build()
+}
+
+fun MapboxMap?.zoomToBounds(latLngBounds: LatLngBounds) {
+    this?.getCameraForLatLngBounds(latLngBounds)?.let { camPosition ->
+        // unZoom a little to avoid having pins exactly at the edges of the map
+        cameraPosition = CameraPosition.Builder(camPosition)
+                .zoom((camPosition.zoom - 1).coerceAtLeast(MapboxConstants.MINIMUM_ZOOM.toDouble()))
+                .build()
+    }
+}

vector/src/main/java/im/vector/app/features/location/MapTilerMapView.kt

@@ -25,7 +25,6 @@ import androidx.core.content.ContextCompat
 import androidx.core.view.marginBottom
 import androidx.core.view.marginTop
 import androidx.core.view.updateLayoutParams
-import com.mapbox.mapboxsdk.camera.CameraPosition
 import com.mapbox.mapboxsdk.geometry.LatLng
 import com.mapbox.mapboxsdk.maps.MapView
 import com.mapbox.mapboxsdk.maps.MapboxMap
@@ -164,7 +163,7 @@ class MapTilerMapView @JvmOverloads constructor(
 
         state.userLocationData?.let { locationData ->
             if (!initZoomDone || !state.zoomOnlyOnce) {
-                zoomToLocation(locationData.latitude, locationData.longitude)
+                zoomToLocation(locationData)
                 initZoomDone = true
             }
 
@@ -180,12 +179,9 @@ class MapTilerMapView @JvmOverloads constructor(
         }
     }
 
-    fun zoomToLocation(latitude: Double, longitude: Double) {
+    fun zoomToLocation(locationData: LocationData) {
         Timber.d("## Location: zoomToLocation")
-        mapRefs?.map?.cameraPosition = CameraPosition.Builder()
-                .target(LatLng(latitude, longitude))
-                .zoom(INITIAL_MAP_ZOOM_IN_PREVIEW)
-                .build()
+        mapRefs?.map?.zoomToLocation(locationData)
     }
 
     fun getLocationOfMapCenter(): LocationData? =

vector/src/main/java/im/vector/app/features/location/live/map/LocationLiveMapViewFragment.kt

@@ -25,8 +25,6 @@ import androidx.lifecycle.lifecycleScope
 import com.airbnb.mvrx.args
 import com.airbnb.mvrx.fragmentViewModel
 import com.airbnb.mvrx.withState
-import com.mapbox.mapboxsdk.camera.CameraPosition
-import com.mapbox.mapboxsdk.constants.MapboxConstants
 import com.mapbox.mapboxsdk.geometry.LatLng
 import com.mapbox.mapboxsdk.geometry.LatLngBounds
 import com.mapbox.mapboxsdk.maps.MapView
@@ -43,11 +41,13 @@ import im.vector.app.core.extensions.addChildFragment
 import im.vector.app.core.platform.VectorBaseFragment
 import im.vector.app.databinding.FragmentSimpleContainerBinding
 import im.vector.app.features.location.UrlMapProvider
+import im.vector.app.features.location.zoomToBounds
+import im.vector.app.features.location.zoomToLocation
 import java.lang.ref.WeakReference
 import javax.inject.Inject
 
 /**
- * Screen showing a map with all the current users sharing their live location in room.
+ * Screen showing a map with all the current users sharing their live location in a room.
  */
 @AndroidEntryPoint
 class LocationLiveMapViewFragment : VectorBaseFragment<FragmentSimpleContainerBinding>() {
@@ -91,35 +91,92 @@ class LocationLiveMapViewFragment : VectorBaseFragment<FragmentSimpleContainerBi
         }
     }
 
+    private fun getOrCreateSupportMapFragment() =
+            childFragmentManager.findFragmentByTag(MAP_FRAGMENT_TAG) as? SupportMapFragment
+                    ?: run {
+                        val options = MapboxMapOptions.createFromAttributes(requireContext(), null)
+                        SupportMapFragment.newInstance(options)
+                                .also { addChildFragment(R.id.fragmentContainer, it, tag = MAP_FRAGMENT_TAG) }
+                    }
+
     override fun invalidate() = withState(viewModel) { viewState ->
         updateMap(viewState.userLocations)
     }
 
     private fun updateMap(userLiveLocations: List<UserLiveLocationViewState>) {
-        symbolManager?.let {
-            it.deleteAll()
-
+        symbolManager?.let { sManager ->
             val latLngBoundsBuilder = LatLngBounds.Builder()
+
             userLiveLocations.forEach { userLocation ->
-                addUserPinToMapStyle(userLocation.userId, userLocation.pinDrawable)
-                val symbolOptions = buildSymbolOptions(userLocation)
-                it.create(symbolOptions)
+                createOrUpdateSymbol(userLocation, sManager)
 
                 if (isMapFirstUpdate) {
-                    latLngBoundsBuilder.include(LatLng(userLocation.locationData.latitude, userLocation.locationData.longitude))
+                    val latLng = LatLng(userLocation.locationData.latitude, userLocation.locationData.longitude)
+                    latLngBoundsBuilder.include(latLng)
                 }
             }
 
-            if (isMapFirstUpdate) {
-                isMapFirstUpdate = false
-                zoomToViewAllUsers(latLngBoundsBuilder.build())
-            }
-        } ?: run {
-            pendingLiveLocations.clear()
-            pendingLiveLocations.addAll(userLiveLocations)
+            removeOutdatedSymbols(userLiveLocations, sManager)
+            updateMapZoomWhenNeeded(userLiveLocations, latLngBoundsBuilder)
+
+        } ?: postponeUpdateOfMap(userLiveLocations)
+    }
+
+    private fun createOrUpdateSymbol(userLocation: UserLiveLocationViewState, symbolManager: SymbolManager) {
+        val symbolId = viewModel.mapSymbolIds[userLocation.userId]
+
+        if (symbolId == null) {
+            createSymbol(userLocation, symbolManager)
+        } else {
+            updateSymbol(symbolId, userLocation, symbolManager)
         }
     }
 
+    private fun createSymbol(userLocation: UserLiveLocationViewState, symbolManager: SymbolManager) {
+        addUserPinToMapStyle(userLocation.userId, userLocation.pinDrawable)
+        val symbolOptions = buildSymbolOptions(userLocation)
+        val symbol = symbolManager.create(symbolOptions)
+        viewModel.mapSymbolIds[userLocation.userId] = symbol.id
+    }
+
+    private fun updateSymbol(symbolId: Long, userLocation: UserLiveLocationViewState, symbolManager: SymbolManager) {
+        val newLocation = LatLng(userLocation.locationData.latitude, userLocation.locationData.longitude)
+        val symbol = symbolManager.annotations.get(symbolId)
+        symbol?.let {
+            it.latLng = newLocation
+            symbolManager.update(it)
+        }
+    }
+
+    private fun removeOutdatedSymbols(userLiveLocations: List<UserLiveLocationViewState>, symbolManager: SymbolManager) {
+        val userIdsToRemove = viewModel.mapSymbolIds.keys.subtract(userLiveLocations.map { it.userId }.toSet())
+        userIdsToRemove
+                .mapNotNull { userId ->
+                    removeUserPinFromMapStyle(userId)
+                    viewModel.mapSymbolIds[userId]
+                }
+                .forEach { symbolId ->
+                    val symbol = symbolManager.annotations.get(symbolId)
+                    symbolManager.delete(symbol)
+                }
+    }
+
+    private fun updateMapZoomWhenNeeded(userLiveLocations: List<UserLiveLocationViewState>, latLngBoundsBuilder: LatLngBounds.Builder) {
+        if (userLiveLocations.isNotEmpty() && isMapFirstUpdate) {
+            isMapFirstUpdate = false
+            if (userLiveLocations.size > 1) {
+                mapboxMap?.get()?.zoomToBounds(latLngBoundsBuilder.build())
+            } else {
+                mapboxMap?.get()?.zoomToLocation(userLiveLocations.first().locationData)
+            }
+        }
+    }
+
+    private fun postponeUpdateOfMap(userLiveLocations: List<UserLiveLocationViewState>) {
+        pendingLiveLocations.clear()
+        pendingLiveLocations.addAll(userLiveLocations)
+    }
+
     private fun addUserPinToMapStyle(userId: String, userPinDrawable: Drawable) {
         mapStyle?.let { style ->
             if (style.getImage(userId) == null) {
@@ -128,31 +185,16 @@ class LocationLiveMapViewFragment : VectorBaseFragment<FragmentSimpleContainerBi
         }
     }
 
+    private fun removeUserPinFromMapStyle(userId: String) {
+        mapStyle?.removeImage(userId)
+    }
+
     private fun buildSymbolOptions(userLiveLocation: UserLiveLocationViewState) =
             SymbolOptions()
                     .withLatLng(LatLng(userLiveLocation.locationData.latitude, userLiveLocation.locationData.longitude))
                     .withIconImage(userLiveLocation.userId)
                     .withIconAnchor(Property.ICON_ANCHOR_BOTTOM)
 
-    private fun zoomToViewAllUsers(latLngBounds: LatLngBounds) {
-        mapboxMap?.get()?.let { mapboxMap ->
-            mapboxMap.getCameraForLatLngBounds(latLngBounds)?.let { cameraPosition ->
-                // update the zoom a little to avoid having pins exactly at the edges of the map
-                mapboxMap.cameraPosition = CameraPosition.Builder(cameraPosition)
-                        .zoom((cameraPosition.zoom - 1).coerceAtLeast(MapboxConstants.MINIMUM_ZOOM.toDouble()))
-                        .build()
-            }
-        }
-    }
-
-    private fun getOrCreateSupportMapFragment() =
-            childFragmentManager.findFragmentByTag(MAP_FRAGMENT_TAG) as? SupportMapFragment
-                    ?: run {
-                        val options = MapboxMapOptions.createFromAttributes(requireContext(), null)
-                        SupportMapFragment.newInstance(options)
-                                .also { addChildFragment(R.id.fragmentContainer, it, tag = MAP_FRAGMENT_TAG) }
-                    }
-
     companion object {
         private const val MAP_FRAGMENT_TAG = "im.vector.app.features.location.live.map"
     }

vector/src/main/java/im/vector/app/features/location/live/map/LocationLiveMapViewModel.kt

@@ -39,6 +39,11 @@ class LocationLiveMapViewModel @AssistedInject constructor(
 
     companion object : MavericksViewModelFactory<LocationLiveMapViewModel, LocationLiveMapViewState> by hiltMavericksViewModelFactory()
 
+    /**
+     * Map to keep track of symbol ids associated to each user Id.
+     */
+    val mapSymbolIds = mutableMapOf<String, Long>()
+
     init {
         getListOfUserLiveLocationUseCase.execute(initialState.roomId)
                 .onEach { setState { copy(userLocations = it) } }