Skip to content

‐Fix for #5540 Unable to reopen a map…

Jump to bottom
David Wright edited this page Dec 1, 2024 · 1 revision

Improve quick behaviour in SelectOneFromMapWidget

Fixes for #5540

What has been done to verify that this works as intended?

Tested manually using form select-quick.xml defined in select-quick.xlsx.

Auto-advance now behaves exactly as for SelectOneMinimalWidget and widget can be re-opened.

Why is this the best possible solution? Were any other approaches considered?

There are two independent aspects to this issue.

...and then skip to the next question...

In SelectOneFromMapWidget, the Context as AdvanceToNextListener is now alerted in setData on the lines of SelectOneMinimalWidget. WidgetFactory is adjusted to pass in isQuick.

(Aligning the behaviour exactly with SelectOneMinimalWidget was an interesting exercise in null safety, necessitated by a tiny detail in the internals of Selection.)

...they should be able to change their selection...

SelectionMapFragment implements quick by setting skipSummary so that onFeatureClicked jumps direct to setFragmentResult in FragmentManager; onFragmentResult in SelectOneFromMapDialogFragment then tidies up and calls dismiss.

When the user tries to reopen the widget as described in the issue, onFeatureClicked is called again from updateItems to display (if skipSummary is not set) the previouslySelectedItem, so (since skipSummary is set) SelectOneFromMapDialogFragment quits immediately.

Making skipSummary set previouslySelectedItem to null stops this happening.

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

No impact on tests; SelectOneFromMapWidgetTest, SelectOneFromMapDialogFragmentTest and SelectionMapFragmentTest all pass unmodified (none of them currently tests for quick).

Do we need any specific form for testing your changes? If so, please attach one.

Form as above

Does this change require updates to documentation? If so, please file an issue here and include the link below.

No.

Before submitting this PR, please make sure you have:

  • run ./gradlew checkAll and confirmed all checks still pass OR confirm CircleCI build passes and run ./gradlew connectedDebugAndroidTest locally.
  • verified that any code or assets from external sources are properly credited in comments and/or in the about file.
  • verified that any new UI elements use theme colors. UI Components Style guidelines

Having now explored this issue in detail, it turns out that appearance quick leads a double life.

Most obviously, it enables auto-advance via AdvanceToNextListener in several of the classes returned in WidgetFactory from getSelectOneWidget, though not in SelectOneFromMapWidget.

In SelectOneFromMapWidget by contrast, quick is used to skip the summary toast, with a side effect of preventing reopening that might be either a bug or a feature.

Looking at the code for SelectOneFromMapWidget, SelectOneFromMapDialogFragment and SelectionMapFragment (in particular onFeatureClicked), skipping the toast is clearly a considered feature.

And the non-reopening behaviour has a certain logic - there's no reason to reopen the map unless you want to make a new selection, in which case just press Remove response first.

So do we want to treat this issue as worth further attention?

To see this in detail,

  1. Load form maps-quick.xml defined by maps-quick.xlsx.
  2. Enter data into items A, B, C in each of groups no-quick and with-quick.
  3. Note that
    • none of the items in group with-quick interprets quick as auto-advance.
    • item C in group no-quick responds to selection with a confirmation toast; it can be reopened.
    • item C in group with-quick responds to selection by saving and exiting without a confirmation toast; it cannot be reopened.

Appearance map quick not implemented for select_one

I've raised this issue to formalise the subsidiary one raised in #5540.

ODK Collect version

As for #5540 - 2022.4.4, 2023.1.2, the master version db8f7ec

Android version

As for #5540 - 10, 13; also 7

Device used

As for #5540 - Redmi 9T, Galaxy A32; also Samsung SM719

Problem description

From #5540:

...When the quick appearance is added to select one from map question, the form doesn’t advance immediately to the next question once a selection is made.

Steps to reproduce the problem

  1. Load select-one-quick.xml - defined in select-one-quick.xlsx
  2. In group no-map, verify quick (auto-advance) behaviour in widgets B, C, D.
  3. In group with-map, verify lack of auto-advance in widgets B, C, D.

Expected behavior

Auto-advance behaviour should be identical in groups no-map and with-map.

Other information

I have a fix ready for PR

It's complicated, see Design notes below

Design notes

Fresh selection

Clone this wiki locally