search

medic / cht-conf-test-harness

A test harness for configurations of Community Health Toolkit applications
6 stars 3 forks source link

Fix filling out `time` questions #250

Closed jkuester closed 7 months ago

jkuester commented 7 months ago

Fixes https://github.com/medic/cht-conf-test-harness/issues/249

I think this was probably a bug introduced in 3.0 (with the new Enekto changes) since I cannot re-create the behavior in 2.4.3. My guess is that Enekto added some extra logic to its time widget around localization (and apparently changed the type value). I did a side-by-side comparison between 2.4.3 and master and confirmed that a form question that had the type time in 2.4.3 was now coming back with the type text in master.

So, I have gone ahead and removed our special handling for the time type in form-filler.js and instead updated the text type handling to support filling out the time widget. This code is closely related to the datetime widget. As a helpful reference, here is the form HTML you get for a datetime widget:

<div class="datetimepicker widget">
  <div class="date">
    <input class="ignore" type="text" placeholder="yyyy-mm-dd">
  </div>
  <div class="timepicker">
    <input class="ignore timepicker-default" type="text" placeholder="hh:mm">
    <button type="button" class="btn-icon-only btn-reset" aria-label="reset">
      <i class="icon icon-refresh"> </i>
    </button>
  </div>
</div>

And here is the HTML you get for just a time widget:

<div class="widget timepicker">
  <input class="ignore timepicker-default" type="text" placeholder="hh:mm">
  <button type="button" class="btn-icon-only btn-reset" aria-label="reset">
    <i class="icon icon-refresh"> </i>
  </button>
</div>
kennsippell commented 7 months ago

Thanks for taking this on. Reminder to npm version patch before merging

jkuester commented 7 months ago

@kennsippell what is the release process here? I pushed the version change to the package.json as suggested, but I did not actually push the git tag (I assume we want the tag on master and not on 249_time_input).

Should I merge this PR and then someone (aka you) will handle actually tagging the release and publishing it on NPM?

kennsippell commented 7 months ago

@jkuester I've merged and published. Pretty ghetto. I literally just run npm publish. Pursuing improvements in https://github.com/medic/cht-conf-test-harness/issues/237 but no progress in a long while.

medic-ci commented 6 months ago

:tada: This PR is included in version 1.0.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket: