Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed using the quick appearance in SelectOneFromMapWidget #5914

Merged
merged 8 commits into from
Jan 26, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
import org.javarosa.form.api.FormEntryCaption;
import org.javarosa.form.api.FormEntryPrompt;
import org.odk.collect.android.R;
import org.odk.collect.android.activities.FormFillingActivity;
import org.odk.collect.android.application.Collect;
import org.odk.collect.android.audio.AudioHelper;
import org.odk.collect.android.exception.ExternalParamsException;
Expand Down Expand Up @@ -196,7 +197,8 @@ public ODKView(
viewLifecycle,
new FileRequesterImpl(intentLauncher, externalAppIntentProvider, formController),
new StringRequesterImpl(intentLauncher, externalAppIntentProvider, formController),
formController
formController,
(FormFillingActivity) context
);

widgets = new ArrayList<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import org.odk.collect.android.formentry.questions.QuestionDetails;
import org.odk.collect.android.geo.MapConfiguratorProvider;
import org.odk.collect.android.javarosawrapper.FormController;
import org.odk.collect.android.listeners.AdvanceToNextListener;
import org.odk.collect.android.storage.StoragePathProvider;
import org.odk.collect.android.utilities.Appearances;
import org.odk.collect.android.utilities.ExternalWebPageHelper;
Expand Down Expand Up @@ -89,6 +90,7 @@ public class WidgetFactory {
private final FileRequester fileRequester;
private final StringRequester stringRequester;
private final FormController formController;
private final AdvanceToNextListener advanceToNextListener;

public WidgetFactory(Activity activity,
boolean readOnlyOverride,
Expand All @@ -103,7 +105,9 @@ public WidgetFactory(Activity activity,
LifecycleOwner viewLifecycle,
FileRequester fileRequester,
StringRequester stringRequester,
FormController formController) {
FormController formController,
AdvanceToNextListener advanceToNextListener
) {
this.activity = activity;
this.readOnlyOverride = readOnlyOverride;
this.useExternalRecorder = useExternalRecorder;
Expand All @@ -118,6 +122,7 @@ public WidgetFactory(Activity activity,
this.fileRequester = fileRequester;
this.stringRequester = stringRequester;
this.formController = formController;
this.advanceToNextListener = advanceToNextListener;
}

public QuestionWidget createWidgetFromPrompt(FormEntryPrompt prompt, PermissionsProvider permissionsProvider) {
Expand Down Expand Up @@ -315,7 +320,7 @@ private QuestionWidget getSelectOneWidget(String appearance, QuestionDetails que
} else if (appearance.contains(Appearances.IMAGE_MAP)) {
questionWidget = new SelectOneImageMapWidget(activity, questionDetails, isQuick, formEntryViewModel);
} else if (appearance.contains(Appearances.MAP)) {
questionWidget = new SelectOneFromMapWidget(activity, questionDetails);
questionWidget = new SelectOneFromMapWidget(activity, questionDetails, isQuick, advanceToNextListener);
} else {
questionWidget = new SelectOneWidget(activity, questionDetails, isQuick, formController, formEntryViewModel);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import org.javarosa.core.model.data.helper.Selection
import org.javarosa.form.api.FormEntryPrompt
import org.odk.collect.android.databinding.SelectOneFromMapWidgetAnswerBinding
import org.odk.collect.android.formentry.questions.QuestionDetails
import org.odk.collect.android.listeners.AdvanceToNextListener
import org.odk.collect.android.widgets.QuestionWidget
import org.odk.collect.android.widgets.interfaces.WidgetDataReceiver
import org.odk.collect.android.widgets.items.SelectOneFromMapDialogFragment.Companion.ARG_FORM_INDEX
Expand All @@ -22,8 +23,12 @@ import org.odk.collect.androidshared.ui.DialogFragmentUtils
import org.odk.collect.permissions.PermissionListener

@SuppressLint("ViewConstructor")
class SelectOneFromMapWidget(context: Context, questionDetails: QuestionDetails) :
QuestionWidget(context, questionDetails), WidgetDataReceiver {
class SelectOneFromMapWidget(
context: Context,
questionDetails: QuestionDetails,
private val autoAdvance: Boolean,
private val autoAdvanceListener: AdvanceToNextListener
) : QuestionWidget(context, questionDetails), WidgetDataReceiver {

init {
render()
Expand Down Expand Up @@ -79,6 +84,9 @@ class SelectOneFromMapWidget(context: Context, questionDetails: QuestionDetails)
override fun setData(answer: Any?) {
updateAnswer(answer as SelectOneData)
widgetValueChanged()
if (autoAdvance) {
autoAdvanceListener.advance()
}
}

private fun updateAnswer(answer: SelectOneData?) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public class WidgetFactoryTest {
@Before
public void setup() {
Activity activity = CollectHelpers.buildThemedActivity(WidgetTestActivity.class).get();
widgetFactory = new WidgetFactory(activity, false, false, null, null, null, null, mock(FormEntryViewModel.class), null, null, null, null, null, null);
widgetFactory = new WidgetFactory(activity, false, false, null, null, null, null, mock(FormEntryViewModel.class), null, null, null, null, null, null, mock());
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,13 @@ import org.junit.runner.RunWith
import org.mockito.kotlin.doReturn
import org.mockito.kotlin.mock
import org.mockito.kotlin.verify
import org.mockito.kotlin.verifyNoInteractions
import org.mockito.kotlin.whenever
import org.odk.collect.android.fakes.FakePermissionsProvider
import org.odk.collect.android.formentry.FormEntryViewModel
import org.odk.collect.android.formentry.questions.QuestionDetails
import org.odk.collect.android.injection.config.AppDependencyModule
import org.odk.collect.android.listeners.AdvanceToNextListener
import org.odk.collect.android.preferences.GuidanceHint
import org.odk.collect.android.support.CollectHelpers
import org.odk.collect.android.support.MockFormEntryPromptBuilder
Expand Down Expand Up @@ -81,7 +83,9 @@ class SelectOneFromMapWidgetTest {
val settings = settingsProvider.getUnprotectedSettings()
val widget = SelectOneFromMapWidget(
activityController.get(),
QuestionDetails(promptWithAnswer(null))
QuestionDetails(promptWithAnswer(null)),
false,
mock()
)

assertThat(
Expand All @@ -106,7 +110,7 @@ class SelectOneFromMapWidgetTest {

val prompt = promptWithAnswer(null)
val widget =
SelectOneFromMapWidget(activity, QuestionDetails(prompt))
SelectOneFromMapWidget(activity, QuestionDetails(prompt), false, mock())
whenever(formEntryViewModel.getQuestionPrompt(prompt.index)).doReturn(prompt)

widget.binding.button.performClick()
Expand All @@ -127,7 +131,9 @@ class SelectOneFromMapWidgetTest {
fun `clicking button when location permissions denied does nothing`() {
val widget = SelectOneFromMapWidget(
activityController.get(),
QuestionDetails(promptWithAnswer(null))
QuestionDetails(promptWithAnswer(null)),
false,
mock()
)

permissionsProvider.setPermissionGranted(false)
Expand All @@ -149,7 +155,7 @@ class SelectOneFromMapWidgetTest {
.withAnswer(SelectOneData(choices[0].selection()))
.build()

val widget = SelectOneFromMapWidget(activityController.get(), QuestionDetails(prompt))
val widget = SelectOneFromMapWidget(activityController.get(), QuestionDetails(prompt), false, mock())
assertThat(widget.binding.answer.text, equalTo("A"))
}

Expand All @@ -158,7 +164,9 @@ class SelectOneFromMapWidgetTest {
val settings = settingsProvider.getUnprotectedSettings()
val widget = SelectOneFromMapWidget(
activityController.get(),
QuestionDetails(promptWithAnswer(null))
QuestionDetails(promptWithAnswer(null)),
false,
mock()
)

assertThat(
Expand All @@ -174,7 +182,9 @@ class SelectOneFromMapWidgetTest {

val widget = SelectOneFromMapWidget(
activityController.get(),
QuestionDetails(promptWithAnswer(answer))
QuestionDetails(promptWithAnswer(answer)),
false,
mock()
)
assertThat(widget.answer, equalTo(answer))
}
Expand All @@ -186,7 +196,9 @@ class SelectOneFromMapWidgetTest {

val widget = SelectOneFromMapWidget(
activityController.get(),
QuestionDetails(promptWithAnswer(answer))
QuestionDetails(promptWithAnswer(answer)),
false,
mock()
)
widget.clearAnswer()
assertThat(widget.answer, equalTo(null))
Expand All @@ -199,7 +211,9 @@ class SelectOneFromMapWidgetTest {

val widget = SelectOneFromMapWidget(
activityController.get(),
QuestionDetails(promptWithAnswer(answer))
QuestionDetails(promptWithAnswer(answer)),
false,
mock()
)

val mockValueChangedListener = mockValueChangedListener(widget)
Expand All @@ -216,7 +230,7 @@ class SelectOneFromMapWidgetTest {
.withAnswer(SelectOneData(choices[0].selection()))
.build()

val widget = SelectOneFromMapWidget(activityController.get(), QuestionDetails(prompt))
val widget = SelectOneFromMapWidget(activityController.get(), QuestionDetails(prompt), false, mock())

widget.clearAnswer()
assertThat(widget.binding.answer.text, equalTo(""))
Expand All @@ -226,7 +240,9 @@ class SelectOneFromMapWidgetTest {
fun `setData sets answer`() {
val widget = SelectOneFromMapWidget(
activityController.get(),
QuestionDetails(promptWithAnswer(null))
QuestionDetails(promptWithAnswer(null)),
false,
mock()
)

val selectChoice = selectChoice(value = "a", index = 101)
Expand All @@ -244,7 +260,7 @@ class SelectOneFromMapWidgetTest {
mapOf(choices[0] to "A", choices[1] to "B")
)
.build()
val widget = SelectOneFromMapWidget(activityController.get(), QuestionDetails(prompt))
val widget = SelectOneFromMapWidget(activityController.get(), QuestionDetails(prompt), false, mock())

widget.setData(SelectOneData(choices[1].selection()))
assertThat(widget.binding.answer.text, equalTo("B"))
Expand All @@ -258,7 +274,7 @@ class SelectOneFromMapWidgetTest {
.withSelectChoiceText(mapOf(choices[0] to "A"))
.build()

val widget = SelectOneFromMapWidget(activityController.get(), QuestionDetails(prompt))
val widget = SelectOneFromMapWidget(activityController.get(), QuestionDetails(prompt), false, mock())

val mockValueChangedListener = mockValueChangedListener(widget)
widget.setData(SelectOneData(choices[0].selection()))
Expand Down Expand Up @@ -286,7 +302,7 @@ class SelectOneFromMapWidgetTest {
.build()

val widget =
SelectOneFromMapWidget(activity, QuestionDetails(prompt))
SelectOneFromMapWidget(activity, QuestionDetails(prompt), false, mock())
widget.setData(SelectOneData(choices[1].selection()))

whenever(formEntryViewModel.getQuestionPrompt(prompt.index)).doReturn(prompt)
Expand All @@ -303,4 +319,36 @@ class SelectOneFromMapWidgetTest {
equalTo(choices[1].index)
)
}

@Test
fun `setData calls AdvanceToNextListener when the 'quick' appearance is used`() {
val listener = mock<AdvanceToNextListener>()
val widget = SelectOneFromMapWidget(
activityController.get(),
QuestionDetails(promptWithAnswer(null)),
true,
listener
)

val selectChoice = selectChoice(value = "a", index = 101)
val answer = SelectOneData(selectChoice.selection())
widget.setData(answer)
verify(listener).advance()
}

@Test
fun `setData does not call AdvanceToNextListener when the 'quick' appearance is not used`() {
val listener = mock<AdvanceToNextListener>()
val widget = SelectOneFromMapWidget(
activityController.get(),
QuestionDetails(promptWithAnswer(null)),
false,
listener
)

val selectChoice = selectChoice(value = "a", index = 101)
val answer = SelectOneData(selectChoice.selection())
widget.setData(answer)
verifyNoInteractions(listener)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import org.odk.collect.androidshared.ui.FragmentFactoryBuilder
import org.odk.collect.androidshared.ui.ToastUtils
import org.odk.collect.androidshared.ui.multiclicksafe.setMultiClickSafeOnClickListener
import org.odk.collect.geo.GeoDependencyComponentProvider
import org.odk.collect.geo.R
import org.odk.collect.geo.ReferenceLayerSettingsNavigator
import org.odk.collect.geo.databinding.SelectionMapLayoutBinding
import org.odk.collect.maps.MapFragment
Expand Down Expand Up @@ -197,7 +196,7 @@ class SelectionMapFragment(

map.setGpsLocationEnabled(true)

map.setFeatureClickListener(::onFeatureClicked)
map.setFeatureClickListener(::onFeatureSelected)
map.setClickListener { onClick() }

selectionMapData.getMappableItems().observe(viewLifecycleOwner) {
Expand Down Expand Up @@ -265,7 +264,7 @@ class SelectionMapFragment(
}
}

private fun onFeatureClicked(featureId: Int, maintainZoom: Boolean = true) {
private fun onFeatureSelected(featureId: Int, maintainZoom: Boolean = true, selectedByUser: Boolean = true) {
val item = itemsByFeatureId[featureId]
val selectedItem = selectedItemViewModel.getSelectedItem()

Expand All @@ -274,7 +273,14 @@ class SelectionMapFragment(
resetIcon(selectedItem)
}

if (!skipSummary) {
if (skipSummary && selectedByUser) {
parentFragmentManager.setFragmentResult(
REQUEST_SELECT_ITEM,
Bundle().also {
it.putLong(RESULT_SELECTED_ITEM, item.id)
}
)
} else {
if (item.points.size > 1) {
map.zoomToBoundingBox(item.points, 0.8, true)
} else {
Expand Down Expand Up @@ -305,13 +311,6 @@ class SelectionMapFragment(
)

selectedItemViewModel.setSelectedItem(item)
} else {
parentFragmentManager.setFragmentResult(
REQUEST_SELECT_ITEM,
Bundle().also {
it.putLong(RESULT_SELECTED_ITEM, item.id)
}
)
}
}
}
Expand All @@ -334,10 +333,10 @@ class SelectionMapFragment(
if (selectedItem != null) {
val featureId = featureIdsByItemId[selectedItem.id]
if (featureId != null) {
onFeatureClicked(featureId)
onFeatureSelected(featureId, selectedByUser = false)
}
} else if (previouslySelectedItem != null) {
onFeatureClicked(previouslySelectedItem, maintainZoom = false)
onFeatureSelected(previouslySelectedItem, maintainZoom = false, selectedByUser = false)
} else if (!map.hasCenter()) {
if (zoomToFitItems && points.isNotEmpty()) {
map.zoomToBoundingBox(points, 0.8, false)
Expand Down
Loading