[CaptureLocation & DropPin task] Fix crashes on config change (#2765)

* Get view model from parent fragment * Revert variable rename for cleaner diff * Replace uiState with navigationRequests * Reinit view model on config change * Remove debugging log * Move arg key into const * Log error * Fix failing check * Revert changes to manifest * Fix tests * Add TODOs * Revert formatting change * copied same to other map frag * viewmodel check * Remove final CR * Remove final CR * test try * nit fix * updated * nit fix * moved to base --------- Co-authored-by: Gino Miceli <[email protected]> Co-authored-by: Gino Miceli <[email protected]>
google · Oct 10, 2024 · 3005856 · 3005856
1 parent 7d41026
commit 3005856
Show file tree

Hide file tree

Showing 8 changed files with 63 additions and 42 deletions.
diff --git a/ground/src/main/java/com/google/android/ground/ui/common/AbstractMapFragmentWithControls.kt b/ground/src/main/java/com/google/android/ground/ui/common/AbstractMapFragmentWithControls.kt
@@ -20,12 +20,14 @@ import android.view.LayoutInflater
 import android.view.View
 import android.view.ViewGroup
 import androidx.annotation.StringRes
+import androidx.hilt.navigation.fragment.hiltNavGraphViewModels
 import androidx.lifecycle.Lifecycle
 import androidx.lifecycle.lifecycleScope
 import androidx.lifecycle.repeatOnLifecycle
 import com.google.android.ground.R
 import com.google.android.ground.databinding.MapTaskFragBinding
 import com.google.android.ground.model.submission.CaptureLocationTaskData.Companion.toCaptureLocationResult
+import com.google.android.ground.ui.datacollection.DataCollectionViewModel
 import com.google.android.ground.ui.map.CameraPosition
 import com.google.android.ground.ui.map.MapFragment
 import com.google.android.ground.util.toDmsFormat
@@ -42,6 +44,13 @@ abstract class AbstractMapFragmentWithControls : AbstractMapContainerFragment()
 
   protected lateinit var binding: MapTaskFragBinding
 
+  protected val dataCollectionViewModel: DataCollectionViewModel by
+    hiltNavGraphViewModels(R.id.data_collection)
+
+  protected val taskId: String by lazy {
+    arguments?.getString(TASK_ID_FRAGMENT_ARG_KEY) ?: error("null taskId fragment arg")
+  }
+
   override fun onCreateView(
     inflater: LayoutInflater,
     container: ViewGroup?,
@@ -112,4 +121,8 @@ abstract class AbstractMapFragmentWithControls : AbstractMapContainerFragment()
     }
     updateLocationInfoCard(R.string.map_location, position.coordinates.toDmsFormat())
   }
+
+  companion object {
+    const val TASK_ID_FRAGMENT_ARG_KEY = "taskId"
+  }
 }
diff --git a/...c/main/java/com/google/android/ground/ui/datacollection/DataCollectionViewPagerAdapter.kt b/...c/main/java/com/google/android/ground/ui/datacollection/DataCollectionViewPagerAdapter.kt
@@ -38,6 +38,8 @@ class DataCollectionViewPagerAdapter
 @AssistedInject
 constructor(
   private val drawAreaTaskFragmentProvider: Provider<DrawAreaTaskFragment>,
+  private val captureLocationTaskFragmentProvider: Provider<CaptureLocationTaskFragment>,
+  private val dropPinTaskFragmentProvider: Provider<DropPinTaskFragment>,
   @Assisted fragment: Fragment,
   @Assisted val tasks: List<Task>,
 ) : FragmentStateAdapter(fragment) {
@@ -51,12 +53,12 @@ constructor(
         Task.Type.TEXT -> TextTaskFragment()
         Task.Type.MULTIPLE_CHOICE -> MultipleChoiceTaskFragment()
         Task.Type.PHOTO -> PhotoTaskFragment()
-        Task.Type.DROP_PIN -> DropPinTaskFragment()
+        Task.Type.DROP_PIN -> dropPinTaskFragmentProvider.get()
         Task.Type.DRAW_AREA -> drawAreaTaskFragmentProvider.get()
         Task.Type.NUMBER -> NumberTaskFragment()
         Task.Type.DATE -> DateTaskFragment()
         Task.Type.TIME -> TimeTaskFragment()
-        Task.Type.CAPTURE_LOCATION -> CaptureLocationTaskFragment()
+        Task.Type.CAPTURE_LOCATION -> captureLocationTaskFragmentProvider.get()
         Task.Type.UNKNOWN ->
           throw UnsupportedOperationException("Unsupported task type: ${task.type}")
       }

diff --git a/...com/google/android/ground/ui/datacollection/tasks/location/CaptureLocationTaskFragment.kt b/...com/google/android/ground/ui/datacollection/tasks/location/CaptureLocationTaskFragment.kt
@@ -15,24 +15,27 @@
  */
 package com.google.android.ground.ui.datacollection.tasks.location
 
+import android.os.Bundle
 import android.view.LayoutInflater
 import android.view.View
 import android.widget.LinearLayout
 import com.google.android.ground.R
 import com.google.android.ground.model.submission.isNotNullOrEmpty
 import com.google.android.ground.model.submission.isNullOrEmpty
+import com.google.android.ground.ui.common.AbstractMapFragmentWithControls.Companion.TASK_ID_FRAGMENT_ARG_KEY
 import com.google.android.ground.ui.datacollection.components.ButtonAction
 import com.google.android.ground.ui.datacollection.components.TaskView
 import com.google.android.ground.ui.datacollection.components.TaskViewFactory
 import com.google.android.ground.ui.datacollection.tasks.AbstractTaskFragment
-import com.google.android.ground.ui.map.MapFragment
 import dagger.hilt.android.AndroidEntryPoint
 import javax.inject.Inject
+import javax.inject.Provider
 
 @AndroidEntryPoint
-class CaptureLocationTaskFragment : AbstractTaskFragment<CaptureLocationTaskViewModel>() {
-
-  @Inject lateinit var map: MapFragment
+class CaptureLocationTaskFragment @Inject constructor() :
+  AbstractTaskFragment<CaptureLocationTaskViewModel>() {
+  @Inject
+  lateinit var captureLocationTaskMapFragmentProvider: Provider<CaptureLocationTaskMapFragment>
 
   override fun onCreateTaskView(inflater: LayoutInflater): TaskView =
     TaskViewFactory.createWithCombinedHeader(inflater, R.drawable.outline_pin_drop)
@@ -41,20 +44,22 @@ class CaptureLocationTaskFragment : AbstractTaskFragment<CaptureLocationTaskView
     // NOTE(#2493): Multiplying by a random prime to allow for some mathematical uniqueness.
     // Otherwise, the sequentially generated ID might conflict with an ID produced by Google Maps.
     val rowLayout = LinearLayout(requireContext()).apply { id = View.generateViewId() * 11149 }
+    val fragment = captureLocationTaskMapFragmentProvider.get()
+    val args = Bundle()
+    args.putString(TASK_ID_FRAGMENT_ARG_KEY, taskId)
+    fragment.arguments = args
     parentFragmentManager
       .beginTransaction()
-      .add(
-        rowLayout.id,
-        CaptureLocationTaskMapFragment.newInstance(viewModel, map),
-        CaptureLocationTaskMapFragment::class.java.simpleName,
-      )
+      .add(rowLayout.id, fragment, CaptureLocationTaskMapFragment::class.java.simpleName)
       .commit()
     return rowLayout
   }
 
   override fun onTaskResume() {
     // Ensure that the location lock is enabled, if it hasn't been.
-    viewModel.enableLocationLock()
+    if (isVisible) {
+      viewModel.enableLocationLock()
+    }
   }
 
   override fun onCreateActionButtons() {

diff --git a/.../google/android/ground/ui/datacollection/tasks/location/CaptureLocationTaskMapFragment.kt b/.../google/android/ground/ui/datacollection/tasks/location/CaptureLocationTaskMapFragment.kt
@@ -24,13 +24,19 @@ import com.google.android.ground.ui.common.AbstractMapFragmentWithControls
 import com.google.android.ground.ui.common.BaseMapViewModel
 import com.google.android.ground.ui.map.MapFragment
 import dagger.hilt.android.AndroidEntryPoint
+import javax.inject.Inject
 import kotlinx.coroutines.launch
 
 @AndroidEntryPoint
-class CaptureLocationTaskMapFragment(private val viewModel: CaptureLocationTaskViewModel) :
-  AbstractMapFragmentWithControls() {
+class CaptureLocationTaskMapFragment @Inject constructor() : AbstractMapFragmentWithControls() {
 
   private lateinit var mapViewModel: CaptureLocationTaskMapViewModel
+  private val viewModel: CaptureLocationTaskViewModel by lazy {
+    // Access to this viewModel is lazy for testing. This is because the NavHostController could
+    // not be initialized before the Fragment under test is created, leading to
+    // hiltNavGraphViewModels() to fail when called on launch.
+    dataCollectionViewModel.getTaskViewModel(taskId) as CaptureLocationTaskViewModel
+  }
 
   override fun onCreate(savedInstanceState: Bundle?) {
     super.onCreate(savedInstanceState)
@@ -52,12 +58,8 @@ class CaptureLocationTaskMapFragment(private val viewModel: CaptureLocationTaskV
   }
 
   override fun onMapReady(map: MapFragment) {
+    super.onMapReady(map)
     binding.locationLockBtn.isClickable = false
     viewLifecycleOwner.lifecycleScope.launch { viewModel.onMapReady(mapViewModel) }
   }
-
-  companion object {
-    fun newInstance(viewModel: CaptureLocationTaskViewModel, map: MapFragment) =
-      CaptureLocationTaskMapFragment(viewModel).apply { this.map = map }
-  }
 }
diff --git a/.../main/java/com/google/android/ground/ui/datacollection/tasks/point/DropPinTaskFragment.kt b/.../main/java/com/google/android/ground/ui/datacollection/tasks/point/DropPinTaskFragment.kt
@@ -15,6 +15,7 @@
  */
 package com.google.android.ground.ui.datacollection.tasks.point
 
+import android.os.Bundle
 import android.view.LayoutInflater
 import android.view.View
 import android.view.ViewGroup
@@ -25,20 +26,20 @@ import androidx.compose.ui.platform.ComposeView
 import com.google.android.ground.R
 import com.google.android.ground.model.submission.isNotNullOrEmpty
 import com.google.android.ground.model.submission.isNullOrEmpty
+import com.google.android.ground.ui.common.AbstractMapFragmentWithControls.Companion.TASK_ID_FRAGMENT_ARG_KEY
 import com.google.android.ground.ui.datacollection.components.ButtonAction
 import com.google.android.ground.ui.datacollection.components.InstructionsDialog
 import com.google.android.ground.ui.datacollection.components.TaskView
 import com.google.android.ground.ui.datacollection.components.TaskViewFactory
 import com.google.android.ground.ui.datacollection.tasks.AbstractTaskFragment
-import com.google.android.ground.ui.map.MapFragment
 import com.google.android.ground.ui.theme.AppTheme
 import dagger.hilt.android.AndroidEntryPoint
 import javax.inject.Inject
+import javax.inject.Provider
 
 @AndroidEntryPoint
-class DropPinTaskFragment : AbstractTaskFragment<DropPinTaskViewModel>() {
-
-  @Inject lateinit var map: MapFragment
+class DropPinTaskFragment @Inject constructor() : AbstractTaskFragment<DropPinTaskViewModel>() {
+  @Inject lateinit var dropPinTaskMapFragmentProvider: Provider<DropPinTaskMapFragment>
 
   override fun onCreateTaskView(inflater: LayoutInflater): TaskView =
     TaskViewFactory.createWithCombinedHeader(inflater, R.drawable.outline_pin_drop)
@@ -47,9 +48,13 @@ class DropPinTaskFragment : AbstractTaskFragment<DropPinTaskViewModel>() {
     // NOTE(#2493): Multiplying by a random prime to allow for some mathematical "uniqueness".
     // Otherwise, the sequentially generated ID might conflict with an ID produced by Google Maps.
     val rowLayout = LinearLayout(requireContext()).apply { id = View.generateViewId() * 11617 }
+    val fragment = dropPinTaskMapFragmentProvider.get()
+    val args = Bundle()
+    args.putString(TASK_ID_FRAGMENT_ARG_KEY, taskId)
+    fragment.arguments = args
     parentFragmentManager
       .beginTransaction()
-      .add(rowLayout.id, DropPinTaskMapFragment.newInstance(viewModel, map), "Drop a pin fragment")
+      .add(rowLayout.id, fragment, "Drop a pin fragment")
       .commit()
     return rowLayout
   }

diff --git a/...in/java/com/google/android/ground/ui/datacollection/tasks/point/DropPinTaskMapFragment.kt b/...in/java/com/google/android/ground/ui/datacollection/tasks/point/DropPinTaskMapFragment.kt
@@ -21,12 +21,18 @@ import com.google.android.ground.ui.common.BaseMapViewModel
 import com.google.android.ground.ui.map.CameraPosition
 import com.google.android.ground.ui.map.MapFragment
 import dagger.hilt.android.AndroidEntryPoint
+import javax.inject.Inject
 
 @AndroidEntryPoint
-class DropPinTaskMapFragment(private val viewModel: DropPinTaskViewModel) :
-  AbstractMapFragmentWithControls() {
+class DropPinTaskMapFragment @Inject constructor() : AbstractMapFragmentWithControls() {
 
   private lateinit var mapViewModel: BaseMapViewModel
+  private val viewModel: DropPinTaskViewModel by lazy {
+    // Access to this viewModel is lazy for testing. This is because the NavHostController could
+    // not be initialized before the Fragment under test is created, leading to
+    // hiltNavGraphViewModels() to fail when called on launch.
+    dataCollectionViewModel.getTaskViewModel(taskId) as DropPinTaskViewModel
+  }
 
   override fun onCreate(savedInstanceState: Bundle?) {
     super.onCreate(savedInstanceState)
@@ -36,16 +42,12 @@ class DropPinTaskMapFragment(private val viewModel: DropPinTaskViewModel) :
   override fun getMapViewModel(): BaseMapViewModel = mapViewModel
 
   override fun onMapReady(map: MapFragment) {
+    super.onMapReady(map)
     viewModel.features.observe(this) { map.setFeatures(it) }
   }
 
   override fun onMapCameraMoved(position: CameraPosition) {
     super.onMapCameraMoved(position)
     viewModel.updateCameraPosition(position)
   }
-
-  companion object {
-    fun newInstance(viewModel: DropPinTaskViewModel, map: MapFragment) =
-      DropPinTaskMapFragment(viewModel).apply { this.map = map }
-  }
 }
diff --git a/...in/java/com/google/android/ground/ui/datacollection/tasks/polygon/DrawAreaTaskFragment.kt b/...in/java/com/google/android/ground/ui/datacollection/tasks/polygon/DrawAreaTaskFragment.kt
@@ -27,6 +27,7 @@ import androidx.lifecycle.lifecycleScope
 import com.google.android.ground.R
 import com.google.android.ground.model.geometry.LineString
 import com.google.android.ground.model.geometry.LineString.Companion.lineStringOf
+import com.google.android.ground.ui.common.AbstractMapFragmentWithControls.Companion.TASK_ID_FRAGMENT_ARG_KEY
 import com.google.android.ground.ui.datacollection.components.ButtonAction
 import com.google.android.ground.ui.datacollection.components.InstructionsDialog
 import com.google.android.ground.ui.datacollection.components.TaskButton
@@ -60,7 +61,7 @@ class DrawAreaTaskFragment @Inject constructor() : AbstractTaskFragment<DrawArea
     val rowLayout = LinearLayout(requireContext()).apply { id = View.generateViewId() * 11411 }
     drawAreaTaskMapFragment = drawAreaTaskMapFragmentProvider.get()
     val args = Bundle()
-    args.putString(DrawAreaTaskMapFragment.TASK_ID_FRAGMENT_ARG_KEY, taskId)
+    args.putString(TASK_ID_FRAGMENT_ARG_KEY, taskId)
     drawAreaTaskMapFragment.arguments = args
     parentFragmentManager
       .beginTransaction()

diff --git a/...java/com/google/android/ground/ui/datacollection/tasks/polygon/DrawAreaTaskMapFragment.kt b/...java/com/google/android/ground/ui/datacollection/tasks/polygon/DrawAreaTaskMapFragment.kt
@@ -16,12 +16,9 @@
 package com.google.android.ground.ui.datacollection.tasks.polygon
 
 import android.os.Bundle
-import androidx.hilt.navigation.fragment.hiltNavGraphViewModels
 import androidx.lifecycle.lifecycleScope
-import com.google.android.ground.R
 import com.google.android.ground.ui.common.AbstractMapFragmentWithControls
 import com.google.android.ground.ui.common.BaseMapViewModel
-import com.google.android.ground.ui.datacollection.DataCollectionViewModel
 import com.google.android.ground.ui.map.CameraPosition
 import com.google.android.ground.ui.map.Feature
 import com.google.android.ground.ui.map.MapFragment
@@ -33,13 +30,11 @@ import kotlinx.coroutines.launch
 class DrawAreaTaskMapFragment @Inject constructor() : AbstractMapFragmentWithControls() {
 
   private lateinit var mapViewModel: BaseMapViewModel
-  private val dataCollectionViewModel: DataCollectionViewModel by
-    hiltNavGraphViewModels(R.id.data_collection)
+
   private val viewModel: DrawAreaTaskViewModel by lazy {
     // Access to this viewModel is lazy for testing. This is because the NavHostController could
     // not be initialized before the Fragment under test is created, leading to
     // hiltNavGraphViewModels() to fail when called on launch.
-    val taskId = arguments?.getString(TASK_ID_FRAGMENT_ARG_KEY) ?: error("null taskId fragment arg")
     dataCollectionViewModel.getTaskViewModel(taskId) as DrawAreaTaskViewModel
   }
 
@@ -69,8 +64,4 @@ class DrawAreaTaskMapFragment @Inject constructor() : AbstractMapFragmentWithCon
       }
     }
   }
-
-  companion object {
-    const val TASK_ID_FRAGMENT_ARG_KEY = "taskId"
-  }
 }