Skip to content

Convert GeoPointMapActivity to Fragment#7253

Merged
grzesiek2010 merged 20 commits into
getodk:masterfrom
seadowg:geopoint-fragment
Jun 16, 2026
Merged

Convert GeoPointMapActivity to Fragment#7253
grzesiek2010 merged 20 commits into
getodk:masterfrom
seadowg:geopoint-fragment

Conversation

@seadowg

@seadowg seadowg commented Jun 10, 2026

Copy link
Copy Markdown
Member

Work towards #7118

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

We ideally want to be showing the geo point map UI within FormFillingActivity so that we don't have to deal with passing reference geometry between Activity objects. Like with geopoly questions, I've converted it to a DialogFragment.

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?

This significantly restructures the way geopoint with maps/placement-maps works, so we'll want to test this feature thoroughly. No changes to how maps work has been made, so this only needs to be tested with one source/style.

Before submitting this PR, please make sure you have:

  • added or modified tests for any new or changed behavior
  • run ./gradlew connectedAndroidTest (or ./gradlew testLab) and confirmed all checks still pass
  • added a comment above any new strings describing it for translators
  • added any new strings with date formatting to DateFormatsTest
  • 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

@seadowg
seadowg force-pushed the geopoint-fragment branch from 89e5e17 to eb1223a Compare June 10, 2026 13:33
@seadowg seadowg changed the title Convert GeoPoinMapActivity to Fragment Convert GeoPointMapActivity to Fragment Jun 10, 2026
@seadowg seadowg changed the title Convert GeoPointMapActivity to Fragment Convert GeoPointMapActivity to Fragment Jun 10, 2026
@dbemke

dbemke commented Jun 11, 2026

Copy link
Copy Markdown

Tested with success!

Verified on a device with Android 10

Verified cases:

  • geopoints with and without map appearance in all sources of maps
  • saving and removing points, discarding changes
  • saved geopoints in the form map
  • checking real location points
  • offline layers and map styles

@WKobus

WKobus commented Jun 11, 2026

Copy link
Copy Markdown

Tested with success

Verified on Android 16

@seadowg
seadowg marked this pull request as ready for review June 12, 2026 09:21
@seadowg
seadowg requested a review from grzesiek2010 June 12, 2026 09:21
Comment thread androidshared/src/main/java/org/odk/collect/androidshared/system/BundleExt.kt Outdated
Comment thread androidshared/src/main/java/org/odk/collect/androidshared/system/BundleExt.kt Outdated
Comment thread geo/src/main/java/org/odk/collect/geo/geopoint/GeoPointMapFragment.kt Outdated
Comment thread geo/src/main/java/org/odk/collect/geo/geopoint/GeoPointMapFragment.kt Outdated
import org.odk.collect.testshared.getOrAwaitValue

@RunWith(AndroidJUnit4::class)
class GeoPointMapDialogFragmentTest {

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.

GeoPointMapDialogFragmentTest only covers the input configuration (configures GeoPointMapFragment with answer/draggable/readOnly), but none of the result-handling logic in onCreateFragment/onAnswer is tested. Compared to the sibling GeoPolyDialogFragmentTest there are some cases missing.

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.

Agreed, but I don't think we've lost any coverage compared to the GeoPointMapActivityTest right? I'd rather follow up with back-filling here.

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.

Agreed it's not a regression vs GeoPointMapActivityTest. But the result-handling in onCreateFragment (the REQUEST_GEOPOINT listener: onAnswer vs dismiss, emptynull) is new logic that's untested, and
GeoPolyDialogFragmentTest already covers the equivalent. Fine to back-fill in a separate PR.

@seadowg
seadowg requested a review from grzesiek2010 June 15, 2026 11:44
@seadowg
seadowg requested a review from grzesiek2010 June 15, 2026 15:29
@seadowg
seadowg requested a review from grzesiek2010 June 16, 2026 07:34
@grzesiek2010
grzesiek2010 merged commit b87f3d8 into getodk:master Jun 16, 2026
8 checks passed
@seadowg
seadowg deleted the geopoint-fragment branch June 16, 2026 08:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants