Code review fixes.

vector/src/main/java/im/vector/app/core/di/FragmentModule.kt

@@ -64,6 +64,7 @@ import im.vector.app.features.home.room.list.RoomListFragment
 import im.vector.app.features.home.room.threads.list.views.ThreadListFragment
 import im.vector.app.features.location.LocationPreviewFragment
 import im.vector.app.features.location.LocationSharingFragment
+import im.vector.app.features.location.live.map.LocationLiveMapViewFragment
 import im.vector.app.features.login.LoginCaptchaFragment
 import im.vector.app.features.login.LoginFragment
 import im.vector.app.features.login.LoginGenericTextInputFormFragment
@@ -1005,4 +1006,9 @@ interface FragmentModule {
     @IntoMap
     @FragmentKey(LocationPreviewFragment::class)
     fun bindLocationPreviewFragment(fragment: LocationPreviewFragment): Fragment
+
+    @Binds
+    @IntoMap
+    @FragmentKey(LocationLiveMapViewFragment::class)
+    fun bindLocationLiveMapViewFragment(fragment: LocationLiveMapViewFragment): Fragment
 }

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

@@ -18,4 +18,7 @@ package im.vector.app.features.location.live.map
 
 import im.vector.app.core.platform.VectorViewModelAction
 
-sealed interface LocationLiveMapAction : VectorViewModelAction
+sealed class LocationLiveMapAction : VectorViewModelAction {
+    data class AddMapSymbol(val key: String, val value: Long) : LocationLiveMapAction()
+    data class RemoveMapSymbol(val key: String) : LocationLiveMapAction()
+}

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

@@ -17,13 +17,10 @@
 package im.vector.app.features.location.live.map
 
 import android.graphics.drawable.Drawable
-import android.os.Bundle
 import android.view.LayoutInflater
-import android.view.View
 import android.view.ViewGroup
 import androidx.core.graphics.drawable.toBitmap
 import androidx.lifecycle.lifecycleScope
-import com.airbnb.mvrx.args
 import com.airbnb.mvrx.fragmentViewModel
 import com.airbnb.mvrx.withState
 import com.mapbox.mapboxsdk.geometry.LatLng
@@ -36,7 +33,6 @@ import com.mapbox.mapboxsdk.maps.SupportMapFragment
 import com.mapbox.mapboxsdk.plugins.annotation.SymbolManager
 import com.mapbox.mapboxsdk.plugins.annotation.SymbolOptions
 import com.mapbox.mapboxsdk.style.layers.Property
-import dagger.hilt.android.AndroidEntryPoint
 import im.vector.app.R
 import im.vector.app.core.extensions.addChildFragment
 import im.vector.app.core.platform.VectorBaseFragment
@@ -44,6 +40,7 @@ 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 kotlinx.coroutines.launch
 import timber.log.Timber
 import java.lang.ref.WeakReference
 import javax.inject.Inject
@@ -51,13 +48,9 @@ import javax.inject.Inject
 /**
  * Screen showing a map with all the current users sharing their live location in a room.
  */
-@AndroidEntryPoint
-class LocationLiveMapViewFragment : VectorBaseFragment<FragmentSimpleContainerBinding>() {
-
-    @Inject
-    lateinit var urlMapProvider: UrlMapProvider
-
-    private val args: LocationLiveMapViewArgs by args()
+class LocationLiveMapViewFragment @Inject constructor(
+        private var urlMapProvider: UrlMapProvider,
+) : VectorBaseFragment<FragmentSimpleContainerBinding>() {
 
     private val viewModel: LocationLiveMapViewModel by fragmentViewModel()
 
@@ -71,16 +64,15 @@ class LocationLiveMapViewFragment : VectorBaseFragment<FragmentSimpleContainerBi
         return FragmentSimpleContainerBinding.inflate(layoutInflater, container, false)
     }
 
-    override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
-        super.onViewCreated(view, savedInstanceState)
+    override fun onResume() {
+        super.onResume()
         setupMap()
     }
 
     private fun setupMap() {
         val mapFragment = getOrCreateSupportMapFragment()
-
         mapFragment.getMapAsync { mapboxMap ->
-            lifecycleScope.launchWhenCreated {
+            lifecycleScope.launch {
                 mapboxMap.setStyle(urlMapProvider.getMapUrl()) { style ->
                     mapStyle = style
                     this@LocationLiveMapViewFragment.mapboxMap = WeakReference(mapboxMap)
@@ -108,10 +100,8 @@ class LocationLiveMapViewFragment : VectorBaseFragment<FragmentSimpleContainerBi
     private fun updateMap(userLiveLocations: List<UserLiveLocationViewState>) {
         symbolManager?.let { sManager ->
             val latLngBoundsBuilder = LatLngBounds.Builder()
-
             userLiveLocations.forEach { userLocation ->
                 createOrUpdateSymbol(userLocation, sManager)
-
                 if (isMapFirstUpdate) {
                     val latLng = LatLng(userLocation.locationData.latitude, userLocation.locationData.longitude)
                     latLngBoundsBuilder.include(latLng)
@@ -123,10 +113,10 @@ class LocationLiveMapViewFragment : VectorBaseFragment<FragmentSimpleContainerBi
         } ?: postponeUpdateOfMap(userLiveLocations)
     }
 
-    private fun createOrUpdateSymbol(userLocation: UserLiveLocationViewState, symbolManager: SymbolManager) {
-        val symbolId = viewModel.mapSymbolIds[userLocation.userId]
+    private fun createOrUpdateSymbol(userLocation: UserLiveLocationViewState, symbolManager: SymbolManager) = withState(viewModel) { state ->
+        val symbolId = state.mapSymbolIds[userLocation.userId]
 
-        if (symbolId == null) {
+        if (symbolId == null || symbolManager.annotations.get(symbolId) == null) {
             createSymbol(userLocation, symbolManager)
         } else {
             updateSymbol(symbolId, userLocation, symbolManager)
@@ -137,7 +127,7 @@ class LocationLiveMapViewFragment : VectorBaseFragment<FragmentSimpleContainerBi
         addUserPinToMapStyle(userLocation.userId, userLocation.pinDrawable)
         val symbolOptions = buildSymbolOptions(userLocation)
         val symbol = symbolManager.create(symbolOptions)
-        viewModel.mapSymbolIds[userLocation.userId] = symbol.id
+        viewModel.handle(LocationLiveMapAction.AddMapSymbol(userLocation.userId, symbol.id))
     }
 
     private fun updateSymbol(symbolId: Long, userLocation: UserLiveLocationViewState, symbolManager: SymbolManager) {
@@ -149,21 +139,20 @@ class LocationLiveMapViewFragment : VectorBaseFragment<FragmentSimpleContainerBi
         }
     }
 
-    private fun removeOutdatedSymbols(userLiveLocations: List<UserLiveLocationViewState>, symbolManager: SymbolManager) {
-        val userIdsToRemove = viewModel.mapSymbolIds.keys.subtract(userLiveLocations.map { it.userId }.toSet())
-        userIdsToRemove
-                .mapNotNull { userId ->
+    private fun removeOutdatedSymbols(userLiveLocations: List<UserLiveLocationViewState>, symbolManager: SymbolManager) = withState(viewModel) { state ->
+        val userIdsToRemove = state.mapSymbolIds.keys.subtract(userLiveLocations.map { it.userId }.toSet())
+        userIdsToRemove.forEach { userId ->
             removeUserPinFromMapStyle(userId)
-                    viewModel.mapSymbolIds[userId]
-                    viewModel.mapSymbolIds.remove(userId)
-                }
-                .forEach { symbolId ->
+            viewModel.handle(LocationLiveMapAction.RemoveMapSymbol(userId))
+
+            state.mapSymbolIds[userId]?.let { symbolId ->
                 Timber.d("trying to delete symbol with id: $symbolId")
                 symbolManager.annotations.get(symbolId)?.let {
                     symbolManager.delete(it)
                 }
             }
         }
+    }
 
     private fun updateMapZoomWhenNeeded(userLiveLocations: List<UserLiveLocationViewState>, latLngBoundsBuilder: LatLngBounds.Builder) {
         if (userLiveLocations.isNotEmpty() && isMapFirstUpdate) {

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

@@ -38,11 +38,6 @@ 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) } }
@@ -50,6 +45,23 @@ class LocationLiveMapViewModel @AssistedInject constructor(
     }
 
     override fun handle(action: LocationLiveMapAction) {
-        // do nothing, no action for now
+        when (action) {
+            is LocationLiveMapAction.AddMapSymbol    -> handleAddMapSymbol(action)
+            is LocationLiveMapAction.RemoveMapSymbol -> handleRemoveMapSymbol(action)
+        }
+    }
+
+    private fun handleAddMapSymbol(action: LocationLiveMapAction.AddMapSymbol) = withState { state ->
+        val newMapSymbolIds = state.mapSymbolIds.toMutableMap().apply { set(action.key, action.value) }
+        setState {
+            copy(mapSymbolIds = newMapSymbolIds)
+        }
+    }
+
+    private fun handleRemoveMapSymbol(action: LocationLiveMapAction.RemoveMapSymbol) = withState { state ->
+        val newMapSymbolIds = state.mapSymbolIds.toMutableMap().apply { remove(action.key) }
+        setState {
+            copy(mapSymbolIds = newMapSymbolIds)
+        }
     }
 }

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

@@ -22,7 +22,11 @@ import im.vector.app.features.location.LocationData
 
 data class LocationLiveMapViewState(
         val roomId: String,
-        val userLocations: List<UserLiveLocationViewState> = emptyList()
+        val userLocations: List<UserLiveLocationViewState> = emptyList(),
+        /**
+         * Map to keep track of symbol ids associated to each user Id.
+         */
+        val mapSymbolIds: Map<String, Long> = emptyMap()
 ) : MavericksState {
     constructor(locationLiveMapViewArgs: LocationLiveMapViewArgs) : this(
             roomId = locationLiveMapViewArgs.roomId