Skip to content

Commit

Permalink
Removed Navigator from HomeScreenFragment (#2896)
Browse files Browse the repository at this point in the history
* added navigation check

* added lifecycle state

* coroutineScope fix

* navigate termsOfService to surveySelector

* remove navigator

* tests fixes

* test fix

* removed viewlifecycle scope

* remove navigator from homeScreenFrag

* remove navigator

* test fix
  • Loading branch information
anandwana001 authored Dec 5, 2024
1 parent 8349e55 commit 5bd180d
Show file tree
Hide file tree
Showing 9 changed files with 88 additions and 213 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package com.google.android.ground.ui.common
import android.os.Bundle
import android.view.View
import android.widget.Toast
import androidx.navigation.fragment.findNavController
import com.google.android.ground.R
import com.google.android.ground.coroutines.DefaultDispatcher
import com.google.android.ground.model.geometry.Coordinates
Expand All @@ -39,7 +40,6 @@ import timber.log.Timber
abstract class AbstractMapContainerFragment : AbstractFragment() {

@Inject lateinit var map: MapFragment
@Inject lateinit var navigator: Navigator
@Inject lateinit var geocodingManager: GeocodingManager
@Inject @DefaultDispatcher lateinit var defaultDispatcher: CoroutineDispatcher

Expand Down Expand Up @@ -101,7 +101,7 @@ abstract class AbstractMapContainerFragment : AbstractFragment() {
/** Opens a dialog for selecting a [MapType] for the basemap layer. */
fun showMapTypeSelectorDialog() {
val types = map.supportedMapTypes.toTypedArray()
navigator.navigate(MapTypeDialogFragmentDirections.showMapTypeDialogFragment(types))
findNavController().navigate(MapTypeDialogFragmentDirections.showMapTypeDialogFragment(types))
}

private fun onLocationLockStateChange(result: Result<Boolean>, map: MapFragment) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,13 @@ import androidx.core.view.doOnLayout
import androidx.hilt.navigation.fragment.hiltNavGraphViewModels
import androidx.interpolator.view.animation.FastOutSlowInInterpolator
import androidx.lifecycle.lifecycleScope
import androidx.navigation.fragment.findNavController
import androidx.viewpager2.widget.ViewPager2
import com.google.android.ground.R
import com.google.android.ground.databinding.DataCollectionFragBinding
import com.google.android.ground.model.task.Task
import com.google.android.ground.ui.common.AbstractFragment
import com.google.android.ground.ui.common.BackPressListener
import com.google.android.ground.ui.common.Navigator
import com.google.android.ground.ui.home.HomeScreenFragmentDirections
import com.google.android.ground.ui.theme.AppTheme
import dagger.hilt.android.AndroidEntryPoint
Expand All @@ -49,7 +49,6 @@ import kotlinx.coroutines.launch
/** Fragment allowing the user to collect data to complete a task. */
@AndroidEntryPoint
class DataCollectionFragment : AbstractFragment(), BackPressListener {
@Inject lateinit var navigator: Navigator
@Inject lateinit var viewPagerAdapterFactory: DataCollectionViewPagerAdapterFactory

val viewModel: DataCollectionViewModel by hiltNavGraphViewModels(R.id.data_collection)
Expand All @@ -75,7 +74,7 @@ class DataCollectionFragment : AbstractFragment(), BackPressListener {
binding.dataCollectionToolbar.setNavigationOnClickListener {
isNavigatingUp = true
viewModel.clearDraft()
navigator.navigateUp()
findNavController().navigateUp()
}

return binding.root
Expand Down Expand Up @@ -171,7 +170,7 @@ class DataCollectionFragment : AbstractFragment(), BackPressListener {
AppTheme {
DataSubmissionConfirmationDialog {
openAlertDialog.value = false
navigator.navigate(HomeScreenFragmentDirections.showHomeScreen())
findNavController().navigate(HomeScreenFragmentDirections.showHomeScreen())
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ import com.google.android.ground.repository.UserMediaRepository
import com.google.android.ground.system.PermissionDeniedException
import com.google.android.ground.system.PermissionsManager
import com.google.android.ground.ui.common.EphemeralPopups
import com.google.android.ground.ui.common.Navigator
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
Expand All @@ -62,7 +61,6 @@ class PhotoTaskFragment : AbstractTaskFragment<PhotoTaskViewModel>() {
@Inject @MainScope lateinit var mainScope: CoroutineScope
@Inject lateinit var permissionsManager: PermissionsManager
@Inject lateinit var popups: EphemeralPopups
@Inject lateinit var navigator: Navigator

// Registers a callback to execute after a user captures a photo from the on-device camera.
private var capturePhotoLauncher: ActivityResultLauncher<Uri> =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import androidx.core.view.GravityCompat
import androidx.core.view.WindowInsetsCompat
import androidx.drawerlayout.widget.DrawerLayout
import androidx.lifecycle.lifecycleScope
import androidx.navigation.fragment.findNavController
import com.google.android.ground.BuildConfig
import com.google.android.ground.MainViewModel
import com.google.android.ground.R
Expand All @@ -38,7 +39,6 @@ import com.google.android.ground.repository.UserRepository
import com.google.android.ground.ui.common.AbstractFragment
import com.google.android.ground.ui.common.BackPressListener
import com.google.android.ground.ui.common.EphemeralPopups
import com.google.android.ground.ui.common.Navigator
import com.google.android.ground.ui.theme.AppTheme
import com.google.android.ground.util.systemInsets
import com.google.android.material.imageview.ShapeableImageView
Expand All @@ -60,7 +60,6 @@ class HomeScreenFragment :
// make this more intuitive.

@Inject lateinit var ephemeralPopups: EphemeralPopups
@Inject lateinit var navigator: Navigator
@Inject lateinit var userRepository: UserRepository
private lateinit var binding: HomeScreenFragBinding
private lateinit var homeScreenViewModel: HomeScreenViewModel
Expand Down Expand Up @@ -95,7 +94,10 @@ class HomeScreenFragment :
binding.navView.setNavigationItemSelectedListener(this)
val navHeader = binding.navView.getHeaderView(0)
navHeader.findViewById<TextView>(R.id.switch_survey_button).setOnClickListener {
homeScreenViewModel.showSurveySelector()
findNavController()
.navigate(
HomeScreenFragmentDirections.actionHomeScreenFragmentToSurveySelectorFragment(false)
)
}
viewLifecycleOwner.lifecycleScope.launch { user = userRepository.getAuthenticatedUser() }
navHeader.findViewById<ShapeableImageView>(R.id.user_image).setOnClickListener {
Expand All @@ -105,15 +107,16 @@ class HomeScreenFragment :
// Re-open data collection screen if any drafts are present
viewLifecycleOwner.lifecycleScope.launch {
homeScreenViewModel.getDraftSubmission()?.let { draft ->
navigator.navigate(
HomeScreenFragmentDirections.actionHomeScreenFragmentToDataCollectionFragment(
draft.loiId,
draft.loiName ?: "",
draft.jobId,
true,
SubmissionDeltasConverter.toString(draft.deltas),
findNavController()
.navigate(
HomeScreenFragmentDirections.actionHomeScreenFragmentToDataCollectionFragment(
draft.loiId,
draft.loiName ?: "",
draft.jobId,
true,
SubmissionDeltasConverter.toString(draft.deltas),
)
)
)

ephemeralPopups
.InfoPopup(binding.root, R.string.draft_restored, EphemeralPopups.PopupDuration.SHORT)
Expand Down Expand Up @@ -160,11 +163,26 @@ class HomeScreenFragment :

override fun onNavigationItemSelected(item: MenuItem): Boolean {
when (item.itemId) {
R.id.sync_status -> homeScreenViewModel.showSyncStatus()
R.id.nav_offline_areas -> homeScreenViewModel.showOfflineAreas()
R.id.nav_settings -> homeScreenViewModel.showSettings()
R.id.about -> homeScreenViewModel.showAbout()
R.id.terms_of_service -> homeScreenViewModel.showTermsOfService()
R.id.sync_status -> {
findNavController().navigate(HomeScreenFragmentDirections.showSyncStatus())
}
R.id.nav_offline_areas -> {
lifecycleScope.launch {
if (homeScreenViewModel.getOfflineAreas().isEmpty())
findNavController().navigate(HomeScreenFragmentDirections.showOfflineAreaSelector())
else findNavController().navigate(HomeScreenFragmentDirections.showOfflineAreas())
}
}
R.id.nav_settings -> {
findNavController()
.navigate(HomeScreenFragmentDirections.actionHomeScreenFragmentToSettingsActivity())
}
R.id.about -> {
findNavController().navigate(HomeScreenFragmentDirections.showAbout())
}
R.id.terms_of_service -> {
findNavController().navigate(HomeScreenFragmentDirections.showTermsOfService(true))
}
}
closeDrawer()
return true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import com.google.android.ground.repository.SubmissionRepository
import com.google.android.ground.repository.SurveyRepository
import com.google.android.ground.repository.UserRepository
import com.google.android.ground.ui.common.AbstractViewModel
import com.google.android.ground.ui.common.Navigator
import com.google.android.ground.ui.common.SharedViewModel
import javax.inject.Inject
import kotlinx.coroutines.flow.MutableSharedFlow
Expand All @@ -43,7 +42,6 @@ class HomeScreenViewModel
@Inject
internal constructor(
private val localValueStore: LocalValueStore,
private val navigator: Navigator,
private val offlineAreaRepository: OfflineAreaRepository,
private val submissionRepository: SubmissionRepository,
private val mutationRepository: MutationRepository,
Expand Down Expand Up @@ -103,38 +101,7 @@ internal constructor(
viewModelScope.launch { _openDrawerRequests.emit(Unit) }
}

fun showSurveySelector() {
navigator.navigate(
HomeScreenFragmentDirections.actionHomeScreenFragmentToSurveySelectorFragment(false)
)
}

private suspend fun getOfflineAreas() = offlineAreaRepository.offlineAreas().first()

fun showOfflineAreas() {
viewModelScope.launch {
navigator.navigate(
if (getOfflineAreas().isEmpty()) HomeScreenFragmentDirections.showOfflineAreaSelector()
else HomeScreenFragmentDirections.showOfflineAreas()
)
}
}

fun showSettings() {
navigator.navigate(HomeScreenFragmentDirections.actionHomeScreenFragmentToSettingsActivity())
}

fun showSyncStatus() {
navigator.navigate(HomeScreenFragmentDirections.showSyncStatus())
}

fun showAbout() {
navigator.navigate(HomeScreenFragmentDirections.showAbout())
}

fun showTermsOfService() {
navigator.navigate(HomeScreenFragmentDirections.showTermsOfService(true))
}
suspend fun getOfflineAreas() = offlineAreaRepository.offlineAreas().first()

fun signOut() {
viewModelScope.launch { userRepository.signOut() }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.compose.ui.platform.ComposeView
import androidx.lifecycle.lifecycleScope
import androidx.navigation.fragment.findNavController
import androidx.recyclerview.widget.LinearLayoutManager
import androidx.recyclerview.widget.PagerSnapHelper
import androidx.recyclerview.widget.RecyclerView
Expand Down Expand Up @@ -295,25 +296,27 @@ class HomeScreenMapContainerFragment : AbstractMapContainerFragment() {
private fun navigateToDataCollectionFragment(cardUiData: MapCardUiData) {
when (cardUiData) {
is MapCardUiData.LoiCardUiData ->
navigator.navigate(
HomeScreenFragmentDirections.actionHomeScreenFragmentToDataCollectionFragment(
cardUiData.loi.id,
cardUiData.loi.properties[LOI_NAME_PROPERTY] as? String?,
cardUiData.loi.job.id,
false,
null,
findNavController()
.navigate(
HomeScreenFragmentDirections.actionHomeScreenFragmentToDataCollectionFragment(
cardUiData.loi.id,
cardUiData.loi.properties[LOI_NAME_PROPERTY] as? String?,
cardUiData.loi.job.id,
false,
null,
)
)
)
is MapCardUiData.AddLoiCardUiData ->
navigator.navigate(
HomeScreenFragmentDirections.actionHomeScreenFragmentToDataCollectionFragment(
null,
null,
cardUiData.job.id,
false,
null,
findNavController()
.navigate(
HomeScreenFragmentDirections.actionHomeScreenFragmentToDataCollectionFragment(
null,
null,
cardUiData.job.id,
false,
null,
)
)
)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import androidx.compose.ui.test.junit4.createAndroidComposeRule
import androidx.compose.ui.test.onNodeWithText
import androidx.compose.ui.test.performClick
import androidx.drawerlayout.widget.DrawerLayout
import androidx.navigation.NavDirections
import androidx.navigation.NavController
import androidx.test.espresso.Espresso.onView
import androidx.test.espresso.action.ViewActions.click
import androidx.test.espresso.action.ViewActions.swipeUp
Expand All @@ -37,12 +37,11 @@ import androidx.test.espresso.matcher.ViewMatchers.withId
import androidx.test.espresso.matcher.ViewMatchers.withText
import com.google.android.ground.BaseHiltTest
import com.google.android.ground.R
import com.google.android.ground.launchFragmentInHiltContainer
import com.google.android.ground.launchFragmentWithNavController
import com.google.android.ground.model.Survey
import com.google.android.ground.persistence.local.stores.LocalSurveyStore
import com.google.android.ground.repository.SurveyRepository
import com.google.android.ground.testMaybeNavigateTo
import com.google.android.ground.ui.common.Navigator
import com.google.common.truth.Truth.assertThat
import com.sharedtest.FakeData
import com.squareup.picasso.Picasso
import dagger.hilt.android.testing.HiltAndroidTest
Expand All @@ -62,11 +61,15 @@ abstract class AbstractHomeScreenFragmentTest : BaseHiltTest() {
@Inject lateinit var localSurveyStore: LocalSurveyStore
lateinit var fragment: HomeScreenFragment
private var initializedPicasso = false
protected lateinit var navController: NavController

@Before
override fun setUp() {
super.setUp()
launchFragmentInHiltContainer<HomeScreenFragment> {
launchFragmentWithNavController<HomeScreenFragment>(
destId = R.id.home_screen_fragment,
navControllerCallback = { navController = it },
) {
fragment = this as HomeScreenFragment
initPicasso(fragment.requireContext())
}
Expand Down Expand Up @@ -239,14 +242,11 @@ class HomeScreenFragmentTest : AbstractHomeScreenFragmentTest() {
@RunWith(ParameterizedRobolectricTestRunner::class)
class NavigationDrawerItemClickTest(
private val menuItemLabel: String,
private val expectedNavDirection: Int?,
private val survey: Survey,
private val expectedNavDirection: NavDirections?,
private val shouldDrawerCloseAfterClick: Boolean,
private val testLabel: String,
) : AbstractHomeScreenFragmentTest() {

@Inject lateinit var navigator: Navigator

@Inject lateinit var surveyRepository: SurveyRepository

@Test
Expand All @@ -257,8 +257,12 @@ class NavigationDrawerItemClickTest(

openDrawer()

testMaybeNavigateTo(navigator.getNavigateRequests(), expectedNavDirection) {
onView(withText(menuItemLabel)).check(matches(isEnabled())).perform(click())
onView(withId(R.id.drawer_layout)).perform(swipeUp())

onView(withText(menuItemLabel)).check(matches(isEnabled())).perform(click())

if (expectedNavDirection != null) {
assertThat(navController.currentDestination?.id).isEqualTo(expectedNavDirection)
}

if (shouldDrawerCloseAfterClick) {
Expand All @@ -272,17 +276,15 @@ class NavigationDrawerItemClickTest(
private val TEST_SURVEY = FakeData.SURVEY.copy()

@JvmStatic
@ParameterizedRobolectricTestRunner.Parameters(name = "{4}")
@ParameterizedRobolectricTestRunner.Parameters(name = "{3}")
fun data() =
listOf(
// TODO(#2385): Restore tests deleted in #2382.
arrayOf(
"Data sync status",
TEST_SURVEY,
HomeScreenFragmentDirections.showSyncStatus(),
true,
"Clicking 'Data sync status' should navigate to fragment",
)
arrayOf("Data sync status", R.id.sync_status_fragment, TEST_SURVEY, true),
arrayOf("Terms of service", R.id.terms_of_service_fragment, TEST_SURVEY, true),
arrayOf("About", R.id.aboutFragment, TEST_SURVEY, true),
arrayOf("Offline map imagery", R.id.offline_area_selector_fragment, TEST_SURVEY, true),
arrayOf("Settings", R.id.settings_activity, TEST_SURVEY, true),
)
}
}
Loading

0 comments on commit 5bd180d

Please sign in to comment.