Skip to content

Conversation

@garethbowen
Copy link
Collaborator

Closes #591

I have verified this PR works in these browsers (latest versions):

  • Chrome
  • Firefox
  • Safari (macOS)
  • Safari (iOS)
  • Chrome for Android
  • Not applicable

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

  • Unit testing

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

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?

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

The included test form uses the function: packages/common/src/fixtures/geolocation/geofence.xml

What's changed

@changeset-bot
Copy link

changeset-bot bot commented Dec 22, 2025

🦋 Changeset detected

Latest commit: 33f6134

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@getodk/xpath Minor
@getodk/web-forms Minor
@getodk/xforms-engine Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Collaborator

@latin-panda latin-panda left a comment

Choose a reason for hiding this comment

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

Looks good! I have a question mainly to confirm the feature's behavior, and a suggestion to add a couple of test cases that make that clear.

approving to unblock

* @param polygon the closed list of geoshape coordinates defining the polygon 'fence'.
* @return true if the location is inside the polygon; false otherwise.
*
* Adapted from https://wrfranklin.org/Research/Short_Notes/pnpoly.html:
Copy link
Collaborator

Choose a reason for hiding this comment

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

💛

};

[
{ expression: '0.5 0.5 0 0', expected: true }, // inside
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add a case where:

  • The point is one of the vertices
  • The point is on a segment (directly on the polygon line) but not a vertex

* return c;
* }
*/
const calculateIsPointInGPSPolygon = (point: Geopoint, polygon: Geotrace) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

In the specs

Returns True if the specified point is inside the specified geoshape, False otherwise.

Is it safe to assume that it excludes points that are vertices or points on the segment (perimeter line)?
I tested this, and it returns false.

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.

Add geofence function

3 participants