XLSForm / pyxform

A Python package to create XForms for ODK Collect.
BSD 2-Clause "Simplified" License
82 stars 137 forks source link

New Feature: expose xforms-value-changed event with odk:setgeopoint action #716

Closed tiritea closed 2 weeks ago

tiritea commented 2 months ago

Problem description

Desire to expose the existing ODK Collect and Enketo XForm action that permits saving a background geopoint - aka odk:setgeopoint action - in response to an xforms-value-changed event.

This functionality is documented in ODK XForms spec here, which includes an example of the desired output XML: https://getodk.github.io/xforms-spec/#setting-a-static-value-when-a-nodes-value-changes

Expected behavior

Per Forum discussion, the consensus appears to be adding a new XLSForm question type, perhaps called either bg-geopoint or background-geopoint (similar to how start-geopoint is used to identify a odk-instance-first-load event-triggered odk:setgeopoint action). This new question type would REQUIRE a non-null ref trigger entry in the form, much like existing setvalue-based triggers, to indicate to source question that triggers the event. Like setvalue, this trigger would be required. Unlike a regular setvalue trigger, however, the desired XForm action would be odk:setgeopoint (not setvalue) and there does NOT require an associated calculation. Indeed, the calculation column SHOULD (or MUST?) be null for this new question type, otherwise it might be unclear what the consequence of the trigger will be

Other information

There is a lengthy discussion on ODK Forum here: https://forum.getodk.org/t/spec-proposal-expose-xforms-value-changed-event-with-odk-setgeopoint-action-in-xlsform/48655

tiritea commented 2 months ago

Per ODK XForm spec, the desired output is:

<input ref="/data/my_text">
    <odk:setgeopoint event="xforms-value-changed" ref="/data/my_current_location" />
</input>

This is envisioned to appear in XLSForm as something like:

type                | name                | trigger    | calculation
--------------------+---------------------+------------+------------
background-geopoint | my_current_location | ${my_text} |
tiritea commented 2 months ago

Per Forum discussion, there are unresolved issue around both the existing setvalue-based xforms-value-changed events - that apply equally to setgeopoint-based xforms-value-changed events - regarding the relative nesting of the trigger and target in the XML hierarchy, specifically when repeat groups are involved. These issues may result in the addition of new checks to pyxform for existing triggers which also need to be applied to this new question type, in order to flag to the user semantic form issues where the specified trigger and target are 'incompatible'.

tinok commented 1 month ago

@tiritea Should the unresolved issues be moved to a separate issue to keep this feature extension narrower?

tiritea commented 1 month ago

Agree, I dont think this ticket should be used to address the more general case of what permutations of trigger and target controls are permitted for xform-value-changed events, not the least of which because they probably apply equally to both setvalue and odk:setgeopoint actions. This ticket is merely to expose something in XLSForm that will map onto the existing XForm support for this feature that exists today in Collect and Enketo, warts and all.

Restricting what permutations of xform-value-changed trigger+target are supported - and what the prescribed behavior should be in these contexts - is a somewhat different issue, which probably needs further careful consideration before adding explicit checks in pyxform to enforce.

tiritea commented 1 month ago

This is what I believe the new feature will look like in a very simple XLSForm:

Screenshot 2024-09-12 at 10 26 24 AM Simple_background_geo.xlsx

and this is what I think it should get translated to in the resulting XForm:

Screenshot 2024-09-12 at 10 27 31 AM Simple_background_geo.xml.zip

@lognaturel @lindsay-stevens can you perhaps confirm this is also your understanding? [nothwithstanding finalizing the new name to be "background-geopoint" vs "bg-geopoint" vs ...]

lognaturel commented 1 month ago

Yes, that looks right to me.

tiritea commented 2 weeks ago

Thanks for merging @RuthShryock's changes!

Next obvious, and somewhat expected, question - when might v2.1.2 be coming out with it? :-)

Or should we plan on backporting? Thnx.