Skip to content
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ jobs:
if [[ -n "$GOOGLE_MAPS_API_KEY" ]]; then \
./check-size.sh 23117869
else
./check-size.sh 13841204
./check-size.sh 13853041
fi

- run:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import androidx.activity.ComponentActivity;
import androidx.lifecycle.MutableLiveData;
import androidx.test.core.app.ApplicationProvider;
import androidx.test.ext.junit.runners.AndroidJUnit4;

import org.javarosa.form.api.FormEntryPrompt;
Expand Down Expand Up @@ -31,6 +32,7 @@ public class InternalRecordingRequesterTest {

@Before
public void setup() {
ApplicationProvider.getApplicationContext().setTheme(com.google.android.material.R.style.Theme_MaterialComponents);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this change needed? If I take it out, then there's a failure, but it seems completely unrelated to everything else here.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This happens because androidx.compose.ui:ui-test-manifest provides its own test manifest, which changes the default app theme used during tests. Robolectric picks up that manifest even for non-Compose tests, and the new default theme is not AppCompat/Material.

ComponentActivity activity = Robolectric.buildActivity(ComponentActivity.class).get();
when(audioRecorder.getCurrentSession()).thenReturn(new MutableLiveData<>(null));

Expand Down
8 changes: 8 additions & 0 deletions geo/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ plugins {
alias(libs.plugins.androidLibrary)
alias(libs.plugins.kotlinAndroid)
alias(libs.plugins.kotlinKapt)
alias(libs.plugins.composeCompiler)
}

apply(from = "../config/quality.gradle")
Expand Down Expand Up @@ -69,6 +70,11 @@ dependencies {
exclude(group = "org.hamcrest", module = "hamcrest-all")
}

implementation(libs.androidXComposeMaterial)
implementation(libs.androidXComposeMaterialIcons)
implementation(libs.androidXComposePreview)
debugImplementation(libs.androidXComposeTooling)

debugImplementation(project(":fragments-test"))

testImplementation(project(":androidtest"))
Expand All @@ -81,4 +87,6 @@ dependencies {
testImplementation(libs.robolectric)
testImplementation(libs.androidxTestEspressoCore)
testImplementation(libs.androidxArchCoreTesting)
testImplementation(libs.androidXComposeUiTestJunit4)
debugImplementation(libs.androidXComposeUiTestManifest)
}
77 changes: 37 additions & 40 deletions geo/src/main/java/org/odk/collect/geo/geopoly/GeoPolyFragment.kt
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import org.odk.collect.maps.PolygonDescription
import org.odk.collect.maps.layers.OfflineMapLayersPickerBottomSheetDialogFragment
import org.odk.collect.maps.layers.ReferenceLayerRepository
import org.odk.collect.settings.SettingsProvider
import org.odk.collect.strings.R.string
import org.odk.collect.webpage.WebPageService
import javax.inject.Inject

Expand Down Expand Up @@ -75,15 +76,7 @@ class GeoPolyFragment @JvmOverloads constructor(
private var map: MapFragment? = null
private var featureId = -1 // will be a positive featureId once map is ready
private var originalPoly: List<MapPoint>? = null

private var inputActive = false // whether we are ready for the user to add points
private var recordingEnabled =
false // whether points are taken from GPS readings (if not, placed by tapping)
private var recordingAutomatic =
false // whether GPS readings are taken at regular intervals (if not, only when user-directed)

private var intervalIndex: Int = DEFAULT_INTERVAL_INDEX

private var accuracyThresholdIndex: Int = DEFAULT_ACCURACY_THRESHOLD_INDEX

private val onBackPressedCallback: OnBackPressedCallback =
Expand Down Expand Up @@ -143,9 +136,6 @@ class GeoPolyFragment @JvmOverloads constructor(
previousState = savedInstanceState

if (savedInstanceState != null) {
inputActive = savedInstanceState.getBoolean(INPUT_ACTIVE_KEY, false)
recordingEnabled = savedInstanceState.getBoolean(RECORDING_ENABLED_KEY, false)
recordingAutomatic = savedInstanceState.getBoolean(RECORDING_AUTOMATIC_KEY, false)
intervalIndex = savedInstanceState.getInt(INTERVAL_INDEX_KEY, DEFAULT_INTERVAL_INDEX)
accuracyThresholdIndex = savedInstanceState.getInt(
ACCURACY_THRESHOLD_INDEX_KEY, DEFAULT_ACCURACY_THRESHOLD_INDEX
Expand All @@ -172,20 +162,17 @@ class GeoPolyFragment @JvmOverloads constructor(
}
return
}
state.putBoolean(INPUT_ACTIVE_KEY, inputActive)
state.putBoolean(RECORDING_ENABLED_KEY, recordingEnabled)
state.putBoolean(RECORDING_AUTOMATIC_KEY, recordingAutomatic)
state.putInt(INTERVAL_INDEX_KEY, intervalIndex)
state.putInt(ACCURACY_THRESHOLD_INDEX_KEY, accuracyThresholdIndex)
}

fun initMap(newMapFragment: MapFragment?, binding: GeopolyLayoutBinding) {
map = newMapFragment

binding.info.setOnClickListener { showInfoDialog(false) }
binding.clear.setOnClickListener { showClearDialog() }
binding.pause.setOnClickListener {
viewModel.stopRecording()
inputActive = false
updateUi()
}

Expand Down Expand Up @@ -247,7 +234,14 @@ class GeoPolyFragment @JvmOverloads constructor(
}
}

val snackbar = SnackbarUtils.make(requireView(), "", Snackbar.LENGTH_INDEFINITE)
val snackbar = SnackbarUtils.make(
requireView(),
"",
Snackbar.LENGTH_INDEFINITE,
action = SnackbarUtils.Action(getString(string.how_to_modify)) {
showInfoDialog(true)
}
)
val viewData = viewModel.points.asLiveData().zip(invalidMessage)
viewData.observe(viewLifecycleOwner) { (points, invalidMessage) ->
val isValid = invalidMessage == null
Expand Down Expand Up @@ -354,8 +348,8 @@ class GeoPolyFragment @JvmOverloads constructor(
}

override fun startInput() {
inputActive = true
if (recordingEnabled && recordingAutomatic) {
viewModel.enableInput()
if (viewModel.recordingMode == GeoPolyViewModel.RecordingMode.AUTOMATIC) {
locationTracker.warm(map!!.getGpsLocation()?.toLocation())
viewModel.startRecording(
ACCURACY_THRESHOLD_OPTIONS[accuracyThresholdIndex],
Expand All @@ -366,15 +360,18 @@ class GeoPolyFragment @JvmOverloads constructor(
}

override fun updateRecordingMode(id: Int) {
recordingEnabled = id != R.id.placement_mode
recordingAutomatic = id == R.id.automatic_mode
when (id) {
R.id.placement_mode -> viewModel.setRecordingMode(GeoPolyViewModel.RecordingMode.PLACEMENT)
R.id.manual_mode -> viewModel.setRecordingMode(GeoPolyViewModel.RecordingMode.MANUAL)
R.id.automatic_mode -> viewModel.setRecordingMode(GeoPolyViewModel.RecordingMode.AUTOMATIC)
}
}

override fun getCheckedId(): Int {
return if (recordingEnabled) {
if (recordingAutomatic) R.id.automatic_mode else R.id.manual_mode
} else {
R.id.placement_mode
return when (viewModel.recordingMode) {
GeoPolyViewModel.RecordingMode.PLACEMENT -> R.id.placement_mode
GeoPolyViewModel.RecordingMode.MANUAL -> R.id.manual_mode
GeoPolyViewModel.RecordingMode.AUTOMATIC -> R.id.automatic_mode
}
}

Expand Down Expand Up @@ -410,21 +407,21 @@ class GeoPolyFragment @JvmOverloads constructor(
}

private fun onClick(point: MapPoint) {
if (inputActive && !recordingEnabled) {
if (viewModel.inputActive && viewModel.recordingMode == GeoPolyViewModel.RecordingMode.PLACEMENT) {
viewModel.add(point)
}
}

private fun onGpsLocationReady(map: MapFragment) {
// Don't zoom to current location if a user is manually entering points
if (requireActivity().window.isActive && (!inputActive || recordingEnabled)) {
if (requireActivity().window.isActive && (!viewModel.inputActive || viewModel.recordingMode != GeoPolyViewModel.RecordingMode.PLACEMENT)) {
map.zoomToCurrentLocation(map.getGpsLocation())
}
updateUi()
}

private fun onGpsLocation(point: MapPoint?) {
if (inputActive && recordingEnabled) {
if (viewModel.inputActive && viewModel.recordingMode != GeoPolyViewModel.RecordingMode.PLACEMENT) {
map!!.setCenter(point, false)
}
updateUi()
Expand All @@ -447,7 +444,7 @@ class GeoPolyFragment @JvmOverloads constructor(
get() {
val meters: Int =
ACCURACY_THRESHOLD_OPTIONS[accuracyThresholdIndex]
return recordingEnabled && recordingAutomatic && meters > 0
return viewModel.recordingMode == GeoPolyViewModel.RecordingMode.AUTOMATIC && meters > 0
}

private fun removeLastPoint() {
Expand All @@ -457,7 +454,7 @@ class GeoPolyFragment @JvmOverloads constructor(
}

private fun clear() {
inputActive = false
viewModel.disableInput()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need this? The tests pass without it and I think whenever we call clear we've already had to call GeoPolyViewModel#stopRecording right?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think whenever we call clear we've already had to call GeoPolyViewModel#stopRecording right?

No, it’s not guaranteed. I didn’t think much about whether it’s mandatory here or not I just tried to mirror the old implementation. So if we used to set the flag to false here, I’m doing the same now.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha. I can maybe just pull it out in the next PR if it's not needed.

viewModel.update(emptyList())
}

Expand All @@ -469,14 +466,14 @@ class GeoPolyFragment @JvmOverloads constructor(
val location = map!!.getGpsLocation()

// Visibility state
binding.play.isVisible = !inputActive
binding.pause.isVisible = inputActive
binding.recordButton.isVisible = inputActive && recordingEnabled && !recordingAutomatic
binding.play.isVisible = !viewModel.inputActive
binding.pause.isVisible = viewModel.inputActive
binding.recordButton.isVisible = viewModel.inputActive && viewModel.recordingMode == GeoPolyViewModel.RecordingMode.MANUAL

// Enabled state
binding.zoom.isEnabled = location != null
binding.backspace.isEnabled = numPoints > 0
binding.clear.isEnabled = !inputActive && numPoints > 0
binding.clear.isEnabled = !viewModel.inputActive && numPoints > 0

if (readOnly) {
binding.play.isEnabled = false
Expand All @@ -501,16 +498,16 @@ class GeoPolyFragment @JvmOverloads constructor(
}
}

binding.collectionStatus.text = if (!inputActive) {
binding.collectionStatus.text = if (!viewModel.inputActive) {
getString(org.odk.collect.strings.R.string.collection_status_paused, numPoints)
} else {
if (!recordingEnabled) {
if (viewModel.recordingMode == GeoPolyViewModel.RecordingMode.PLACEMENT) {
getString(
org.odk.collect.strings.R.string.collection_status_placement,
numPoints
)
} else {
if (!recordingAutomatic) {
if (viewModel.recordingMode == GeoPolyViewModel.RecordingMode.MANUAL) {
getString(
org.odk.collect.strings.R.string.collection_status_manual,
numPoints
Expand Down Expand Up @@ -552,6 +549,10 @@ class GeoPolyFragment @JvmOverloads constructor(
}
}

private fun showInfoDialog(fromSnackbar: Boolean) {
InfoDialog.show(requireContext(), viewModel, fromSnackbar)
}

private fun showClearDialog() {
if (!viewModel.points.value.isEmpty()) {
MaterialAlertDialogBuilder(requireContext())
Expand All @@ -578,10 +579,6 @@ class GeoPolyFragment @JvmOverloads constructor(
const val REQUEST_GEOPOLY: String = "geopoly"
const val RESULT_GEOPOLY: String = "geopoly"
const val RESULT_GEOPOLY_CHANGE: String = "geopoly_change"

const val INPUT_ACTIVE_KEY: String = "input_active"
const val RECORDING_ENABLED_KEY: String = "recording_enabled"
const val RECORDING_AUTOMATIC_KEY: String = "recording_automatic"
const val INTERVAL_INDEX_KEY: String = "interval_index"
const val ACCURACY_THRESHOLD_INDEX_KEY: String = "accuracy_threshold_index"
val INTERVAL_OPTIONS = intArrayOf(
Expand Down
23 changes: 23 additions & 0 deletions geo/src/main/java/org/odk/collect/geo/geopoly/GeoPolyViewModel.kt
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,16 @@ class GeoPolyViewModel(
private val scheduler: Scheduler
) : ViewModel() {

enum class RecordingMode {
PLACEMENT, MANUAL, AUTOMATIC
}

var recordingMode: RecordingMode = RecordingMode.PLACEMENT
private set

var inputActive: Boolean = false
private set

private val _points = MutableStateFlow(
if (points.isNotEmpty()) {
if (outputMode == OutputMode.GEOSHAPE) {
Expand Down Expand Up @@ -69,10 +79,23 @@ class GeoPolyViewModel(
}

fun stopRecording() {
disableInput()
recording?.cancel()
locationTracker.stop()
}

fun setRecordingMode(mode: RecordingMode) {
recordingMode = mode
}

fun enableInput() {
inputActive = true
}

fun disableInput() {
inputActive = false
}

override fun onCleared() {
stopRecording()
}
Expand Down
Loading