cht-core
cht-core copied to clipboard
Support making countdown-widgets `required` in forms
Is your feature request related to a problem? Please describe.
Currently, when including the countdown-widget
in a form, there is no way to ensure that the widget was actually used when filling out the form. Users can simply skip the widget and continue to fill out the form without waiting for the timer to run.
Describe the solution you'd like
It seems like the cleanest way to proceed would be to have the required
value be respected for fields with the countdown-timer
appearance. This way, a form designer could control whether or not the user has to use the timer by setting the required
expression (and, by default, the behavior would remain the same as now where a user is not required to wait on the timer).
Additional context This is a feature request from a partner.
Implementing this could be tricky since currently the countdown-widget
operates as a note
field in the form. The note
fields actually cannot be required
in recent versions of Enekto (since objectively it does not really make sense to require a note). So, we may need to support using the countdown-timer
appearance on a different type of question besides note
. Options off the to of my head are: boolean
field that is set to true
when the timer completes, or integer
field that contains the number of seconds remaining on the timer.
@jkuester The request is for the preceding question to be restricted / dependent on the successful completion of the timer after 60 seconds / one minute. Below is the video explaining the current implementation in detail and the requested change.
I just wanted to capture a few more details about this request based on today's conversations!
The main pain point that this feature would address is ensuring data accuracy in forms that involve a timer, For example, when capturing a value for Breaths per Minute, we can only get an accurate count of breaths when they are tracked for the full minute. Requiring the Breaths per Minute question does ensure that we get a value, but being able to also require the timer to be run would help to ensure that the provided value was accurate!
Also, it seems that modeling the countdown timer as a boolean
question makes the most sense! (An integer
value would be more appropriate for a theoretical stopwatch widget.)
@Kymoraa and Abigael, here are some additional design considerations I had after taking a closer look at the code:
- I think we should look at leveraging
trigger
/acknowledge
questions as the ones we support having thecountdown-timer
appearance since I think those will be the most simple to answer with true/false - Obviously we don't want to actually be able to see the checkbox from the question in the form (we just want to see the timer). My thinking is that we could maybe just hide the checkbox element similar to what we do in the db-object-widget to hide the default text input box
- Finally, for setting the value back into the
trigger
question when the timer completes, I believe this could happen in theTimerAnimation.animate
function (probably at the same spot where we play the audio beep). We would just need to somehow trigger a true value to get set into thetrigger
question similar to how the db-object-widget sets the selected value back into the initial input question.
Okay, now that @SheilaAbby has got us some working code to play around with, I think we have been able to shake out the majority of the complexities here and can land on a particular functionality. @garethbowen please let me know what you think (as this proposal does result in a change to how we would recommend representing timers in xforms)!
Design proposal
Currently, the count-down timer widget works by putting the countdown-timer
appearance onto a note
field. You can set your custom time on the timer via the default column in your xform. We cannot continue to use this approach, while at the same time adding support for the require because you cannot require a note
in Enketo.
My proposal is we deprecate note
timers (and do not support require for them). Instead we switch to supporting the countdown-timer
appearance on trigger
/acknowledge
questions. A custom time for the timer could be provided via a duration
parameter (note that parameter is a separate column in your xform that is used for providing configuration params for a question (e.g. you can set how many rows to display for a text
question)). For trigger
/acknowledge
timers, we would set the value of the question to OK
(equivalent to selecting the checkbox) once the timer has completed. This will allow the normal Enketo require functionality to operate on these questions.
In my opinion, a trigger
question with a countdown-timer
appearance, configured by a parameter is the least surprising (most appropriate) way model this functionality in Enketo. Basically we are just adding a visible delay to the "acknowledgement". I have tinkered a bit with using the code in countdown-widget with a trigger
question. It will not just be a drop-in replacement, but it should be feasible to implement in a clean way (will be a bit of extra logic to maintain the note
functionality alongside, but should not be a problem).
I think that's a big improvement! I have no problem deprecating the old widget definition and maintaining it till the next major (or beyond if necessary). I imagine the code will be reused for this anyway.
The new design looks much more idiomatic of Enketo forms and like you say it means we can leverage this elsewhere with require to give designers more flexibility.
Do you know if the parameter
parameter is supported by our build tools, eg: xls2xform?
Thanks @garethbowen!
Do you know if the parameter parameter is supported by our build tools, eg: xls2xform?
~Yes!~ (Edit: no, it turns out the supported params are hardcoded in pyxform.) I recently tested a full end-to-end of using parameters for a range
question (xform > cht-conf > cht-core > webapp > Enketo) and everything was converted/included as expected for the parameters!
Okay, now that @SheilaAbby has got us some working code to play around with, I think we have been able to shake out the majority of the complexities here and can land on a particular functionality. @garethbowen please let me know what you think (as this proposal does result in a change to how we would recommend representing timers in xforms)!
Design proposal
Currently, the count-down timer widget works by putting the
countdown-timer
appearance onto anote
field. You can set your custom time on the timer via the default column in your xform. We cannot continue to use this approach, while at the same time adding support for the require because you cannot require anote
in Enketo.My proposal is we deprecate
note
timers (and do not support require for them). Instead we switch to supporting thecountdown-timer
appearance ontrigger
/acknowledge
questions. A custom time for the timer could be provided via aduration
parameter (note that parameter is a separate column in your xform that is used for providing configuration params for a question (e.g. you can set how many rows to display for atext
question)). Fortrigger
/acknowledge
timers, we would set the value of the question toOK
(equivalent to selecting the checkbox) once the timer has completed. This will allow the normal Enketo require functionality to operate on these questions.In my opinion, a
trigger
question with acountdown-timer
appearance, configured by a parameter is the least surprising (most appropriate) way model this functionality in Enketo. Basically we are just adding a visible delay to the "acknowledgement". I have tinkered a bit with using the code in countdown-widget with atrigger
question. It will not just be a drop-in replacement, but it should be feasible to implement in a clean way (will be a bit of extra logic to maintain thenote
functionality alongside, but should not be a problem).
@jkuester Thank you for the valuable suggestions. I've attempted to integrate the timer widget with a trigger/acknowledge
question. However, we encounter a minor drawback since the trigger/acknowledge field cannot be set to 'hidden'; it appears on the form, requiring users to check the box. Once the box is checked, 'OK' becomes the timer value even though the timer is still running. Nonetheless, the js code can assign a different value (not a 'OK' value) to the same field when the timer completes, and this new value is used to constrain the timer, not the initial 'OK' check input by the user.
please find attached screenshot
Supporting the timer on a text question type won’t introduce an extra question on the form requiring a user to input a value. Surprisingly, I have observed it is possible to leave the default
column for custom timer durations, as it has been( and remove the FALSE value I set here https://github.com/medic/cht-core/pull/8826). The text input will receive an 'ok' string value when the timer completes running. The text field won't display an 'input' field on the form requiring a user to enter a value and properly displays the countdown widget.
see,
:+1: Yes, we have a bit more logic to add to get everything working like we need it. Currently, the textbox for the string/note question is actually being hidden by some CSS (which is why you do not see it for questions of that type). We should be able to do the same for that OK
radio button. However, we need a bit more changes to the logic for attaching the timer canvas to the page to avoid also hiding the timer too (even in your screenshot you can see that the timer canvas is actually a child of the OK radio...). I will share some more specific suggestions later today on the PR!
So, in today's episode of why we cannot have nice things... it turns out my previous comment about how parameters should work fine for us, is wrong.... Apparently, in Pyxform, all the supported parameters are actually hard-coded for each question type and only those values are loaded. So, we cannot make-up custom params for trigger
questions. :sob: There is an open Pyxform issue regarding making parameter handling more sophisticated. I have added a comment out there to see if they might support just passing through custom parameters. :shrug:
This does leave us in a bit of a dilemma here, though. Here are the options I have thought of:
- Just stick with using the default column to provide the custom duration. To make this work with trigger/require I think we could just add some code to the init of the countdown widget to get the custom duration value and then set the value for the question to
''
before the user starts filling out the form. I think this would be workable, but not very "idiomatic". - Update our custom fork of Pyxform to support the parameters we need. I would prefer to avoid additional custom pyxform code at all costs.
- Instead of the proper parameters column, we could instead support setting the
duration
into a new instance::parameters column. Pyxform would automatically load this data into the form xml instance model. If needed, we could update cht-conf to then copy the parameters down into the form body. The instance::parameters values could be comma separated properties just like the normal parameters column and the column could get re-used to set parameters on any kind of question we wanted.
#1
seems like the most feasible option for us to be able to immediately proceed here. (And has the bonus of being the closest to our current implementation of countdown-timer.) If ODK is open to supporting custom parameters in Pyxform, then I would rather not do the extra work of #2
or #3
just to change it again in the future when we get custom params. If ODK is not open to custom params (and does not have any other suggestions for a better way to go), then I think #3
is worth considering as a possible way forwards that might even be able to get us off of our custom pyxform fork... @garethbowen what do you think?
Actually got a response already from ODK out on the Pyxform thread! TLDR is that they do not think we should be trying to use the properties column like this. BUT, they pointed me to this doc for the instance/bind/body extension columns and this more or less reinforces exactly what I was suggesting in #3
(only now I know that this is actually the intended usage!).
I am going to dig deeper on this next week and try to put together a more detailed proposal!
Sounds like #3
is the way forward and definitely worth the investigation. Thanks @jkuester
Okay, @garethbowen, hopefully the last request for design review!
I think we should support a workflow where the xlsxform looks like:
-
type:
trigger
/acknowledge
(existing supported question type) -
appearance:
cht:countdown-timer
(custom appearance withcht
namespace) -
instance::cht::properties:
duration=100
(column supported in the spec as a way to add custom values to a form)
Pyxform will automatically load data from the instance::cht::properties column into the form xml (as it is currently doing for db-doc
). Then, in the cht-core code, we would need to update the generate-xform
service in the api code to unpack the attributes from the form xml and set them into the form HTML (generated by the xsl translation) as attributes of the input
element (<input cht:duration="100" ...
). This will allow them to be accessible to the custom Enketo widgets. (This similar to how the official properties column values get set onto the Enketo widgets. Basically, pyxform translates the properties values into the form xml body
section as attributes. Then, the enketo-transformer xsl copies the body attributes into the generated form HTML. The Enketo widgets read the attribute values to know the property values to use.)
This seems to me to be the best balance of following the standards, getting the functionality we need, and not being too difficult to implement/maintain. There are a couple questions this does raise:
- Should we use
instance::cht::properties
orbind::..
orbody::...
? All are supported by the spec. I am leaning towardsinstance::
because:- We already have
instance::db-doc
, etc. So, closer to what folks are used to seeing in forms. - The values from
instance::...
also get automatically loaded into the form model xml (by the xsl transition) and then are automatically included by Enketo in the form data returned when callingform.getDataStr()
. This could make it more simple to use these values for post-processing form data (e.g. this is how we currently are doing thedb-doc
processing). - From the spec, the body of the xform xml seems more related to display of the data. Things like timer duration seem to be more than just a "display" customization.
- On the other hand,
bind::...
may actually be a more semantically accurate place to store custom properties. As far as I can tell, theinstance
data is more related to the actual answers to the form questions (e.g. also includes default values) while thebind
data is focused on the properties that affect the logic of the question (e.g. type, relevance, etc). Happy to go this way instead if it seems more correct.
- We already have
- One alternative approach that seems worth pointing out is that instead of having a common
instance::cht::properties
column in the xlsxform, we could have different columns based on the actual property names (e.g.instance::cht::duration
). Then, instead of adding custom code intogenerate-xforms
to unpack the properties, we could just customize the xsl transition to automatically set the attributes onto the HTML. I currently prefer the singularinstance::cht::properties
column approach for the following reasons:- It avoids having a bunch of one-off columns in the xlsxform that are only used for one or two questions. Instead, many types of questions can have their parameters set in
instance::cht::properties
(could ultimately refactordb-doc
to reuse this column). Will reduce the excessive number of columns in some forms. - We are not far from being able to avoid having to use a custom xsl file for translating the form xml. I would rather avoid adding more complexity to that situation (even if the alternative is manual post-processing).
- It avoids having a bunch of one-off columns in the xlsxform that are only used for one or two questions. Instead, many types of questions can have their parameters set in
Let me know what you think or if you have any better ideas for how to approach this! Thanks!
If our implementation for db-doc didn't exist then I think bind
would be more correct, and I'm not too concerned about compatibility with db-doc if we feel bind is right. I assume bind
gets handled just the same as instance
in Enketo?
I prefer more columns with simple values (option 2). Defining properties is rare so far so I don't think it'll explode the number of columns in every xform. However I really don't like the idea of custom xsl. Can we use the same approach as 1 while using multiple columns like in 2?
If the answer is complicated (and I suspect it is) feel free to grab me for a call!
@jkuester @garethbowen Some great ideas here! But putting them into action seems a bit tricky to me! I've made some changes to the PR for using a text input question type. Please take a look when you can. https://github.com/medic/cht-core/pull/8826
Also, just a heads up, workflow devs can still use the 'default' column for setting custom timer durations....so basically am not setting any default value on the fields with the countdown timer appearance...but when the timer completes running, the field will be set to contain a 'true'
string value
@SheilaAbby Thanks for the patience here as we work through the best way to add support. It is true that the scope is shifting a bit here. My proposal, is that you and I work together to land this! You can keep focused on the countdown-timer widget code (I think we are pretty close on this code, just need to get some tests). And I will jump in and make the code changes needed on the api side to get the data we need from the xform. How does that sound?
Had a brief discussion with @garethbowen today and we landed on this design approach:
Xlsxform:
-
type:
trigger
/acknowledge
-
appearance:
countdown-timer
-
instance::cht::duration:
100
(We decided to use a custom column for each property, instead of combining them into a singleinstance::cht::properties
column. The main motivation here is not not over-architect things and avoid the complexity of having to parse multiple properties values from one attribute, especially when the benefits of a single-column approach seem dubious. (e.g. even if you have a bunch of properties to set for a question, it still might be easier for app devs to write/read if these values could go in separate columns, instead of having a ton of fields combined into a single column.))
api/generate-xform service:
- Parse the form XML looking for instance attributes that start with
cht::
- Copy the values of each of these into a corresponding attribute on the HTML
input
field for the question (<input cht:duration="100" ...
). - The goal is to do this generically without needing to hard-code
duration
. This will make it much easier in the future to add custom properties to other form widgets.
webapp/countdown-timer widget:
- Retain backwards compatibility with
note
times with the duration provided in the default column. - Update widget selector to also match on
trigger
inputs (also update internal jquery logic to interact with the proper elements in this case. - Add support for reading the default value from the
input
property:cht:duration
. - Update widget to set the
'OK'
trigger value for the question when the timer completes. This will enable the require logic to work as desired. - Update CSS to hide trigger element when the countdown-timer is active
@garethbowen I have run into a challenge with using a custom namespace for the xlsx column name (e.g. instance::cht:duration
). If you have a custom attribute column without the namespace (e.g. instance::duration
) this gets converted to something like <my_timer duration="10"/>
. However, with the custom namespace, you get something like <my_timer cht="{'duration': '5'}"/>
. Of course, things get really fun, then, when you start including quotes and stuff in your column values: cht="{'duration': '5', 'message': 'Uses\'s "both" kinds of quotes'}"
. None of these serializes directly to JSON. I am hoping you can tell me that this is some standard format for dictionary data and there is a library for easily serializing this into a JS object! (I have not been able to find anything searching around the web, but maybe I am missing something... :shrug: ) I am afraid it is just the string serialization of a Python dict (that perhaps, by design, is XML-compatible). But, once again, in that case I am not seeing any convenient way to de-serialize it to a JS object.
Assuming there is no convenient library available for de-serializing this data, it seems like our options are:
- Specify that we do not support quotes in our custom attribute columns (not a problem at this point since at this point we just need to support a number for
duration
). Then it should be pretty straightforward to convert the data to JSON. - Write some more advanced parsing code to properly de-serialize the data (and handle all the different quote combinations).
- Just don't use a custom namespace for our attribute columns. Instead we could say something like:
instance::cht_duration
. - Log a Pyxform ticket to see if they can change the data format to something more parsable.
#2
seems like a bad idea due to a high level of complexity so support an unnecessary usecase that we do not even have yet. #3
would be okay, but does not really follow the true spirit of the spec. I almost did #4
, but I realized that you cannot put JSON directly into XML attributes, so I was not sure exactly what the alternative approach should be here. If anyone has an idea of a better say data could be represented in the XML attribute (maybe just escaped JSON?) then I am happy to suggest it in Pyxform!
Okay. That's really painful!
I think the answer is some form of (3). I have no strong preference now about whether we use the cht_
convention or just drop it and use duration
which seems like the supported approach. The chance of a conflict is probably low enough we can get away with it, and we can handle a conflict if one does arise. If the prefix is useful as a flag for which attributes to handle in our custom code then it's fine to keep it.
With (4), I'm not sure if they'll want to support JSON in xml, but you should be able to namespace the attribute somehow, eg: instance::cht:duration
-> <my_timer cht:duration="10"/>
. I think that's worth an issue...
Logged a Pyxform issue! https://github.com/XLSForm/pyxform/issues/696
My vote is we wait a bit to see what the reaction is to the Pyxform suggestion. If they are open to changing that behavior, then maybe we can get away with #1
(or really, just hard-code support for duration for now and don't try and have generic support for all columns/value). Then, a new version of Pyxform would allow us to unlock more usages of custom columns and we could add more generic support at that point....
Update from the Pyxform issue! It turns out I just cannot read properly (even typoed several times in this thread). I thought the spec said to use instance::cht::duration
, but actually I should have been using instance::cht:duration
. :facepalm: Fixing my column name resolves the weird object serialization issues (so we can just load one value per attribute). So, we are back in business with the same originally intended generic approach to translating those attributes. @SheilaAbby I am hoping to have the server-side code ready to add to your branch by the end of this week!
Completed in https://github.com/medic/cht-core/commit/3e91ea65c0aa0d6799dcf1e3f7c6345d2cd2dd8b