Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement initial integration with enketo-core to display complex emb… #563

Closed
wants to merge 9 commits into from

Conversation

shankari
Copy link
Contributor

@shankari shankari commented May 2, 2019

…edded surveys

The form is displayed inline in the main app, so it can be manipulated much
more easily. In particular:

  • both the form and the model are javascript objects passed in to a function so
    can be manipulated at will to customize the survey for a particular user.
  • The result is also retrieved in the form, so results can be reflected
    immediately in the UX

The form is currently displayed as a view in the metrics tab, with its own
state. I considered making it a popover instead, but wasn't sure that the
smaller width would work for the survey. I might experiment with that in a
future change.

The main changes to the app itself are to the main.js to define the new state,
and a new survey launcher enketo_launch.js that is the controller for the new
state.

The rest of the code is to check in the enketo javascript libraries. It is
theoretically possible to install them via npm install and then use rollup
or webpack to create the bundle. But we don't really have a gruntfile or
other build mechanism to invoke the rollup/webpack, and ES6 imports don't
necessarily play nice with angular 1. For example, when I tried using import { Form } from enketo-core`, angular complained about the import.

Also, the resulting bundle file did not include an export for Form since
rollup inlines functions where possible. I had to manually add
window.FormModule to the javascript file to allow it to be used from the rest
of the app.

Given all this, I decided to check in enketo-core as a manual_lib.

This change can be tested by applying the following patch, although it is
pretty kludgy. Long-term, individual projects need to determine where they want
to launch the test from.

Also includes a couple of sample JSON files for testing.

The input to the library is the enketo paper formatted JSON files.

They can be obtained using the following process:

  • Create the form using kobotoolbox https://github.com/kobotoolbox/ or https://build.opendatakit.org/ This will let you download an xlsform
  • Convert xlsform -> xml form using http://xlsform.opendatakit.org/
  • Convert xml form -> enketo paper format form + model using
    enketo-transformer. The curl option works fine - I used it with requests
  • Copy the json files into www/json
  • Launch them by passing the JSON file as a parameter while changing state, as in metrics.js below
diff --git a/www/js/metrics.js b/www/js/metrics.js
index d80e81d7..e7e4a3d3 100644
--- a/www/js/metrics.js
+++ b/www/js/metrics.js
@@ -35,6 +35,13 @@ angular.module('emission.main.metrics',['nvd3', 'emission.services', 'ionic-date
         $scope.onCurrentTrip();
     });

+  $scope.testSurvey = function() {
+    $state.go("root.main.enketosurvey", {
+        form_location: "json/enketo-survey-sample-full.json"
+    });
+  }
+
+
     // If we want to share this function (see the pun?) between the control screen and the dashboard, we need to put it into a service/factory.
     // But it is not clear to me why it needs to be in the profile screen...
     var prepopulateMessage = {
diff --git a/www/templates/main-metrics.html b/www/templates/main-metrics.html
index 124e95d9..923e0d05 100644
--- a/www/templates/main-metrics.html
+++ b/www/templates/main-metrics.html
@@ -10,6 +10,9 @@
       <button class="button button-icon" ng-click="doRefresh()">
         <i class="icon ion-refresh"></i>
       </button>
+      <button class="button button-icon" ng-click="testSurvey()">
+        TEST SURVEY
+      </button>
   </ion-nav-buttons>
   <ion-content class="has-header">
     <div style="height: 40px; width: 100%; padding: 10px;" ng-if="uictrl.showVis"> <!-- visualization control -->

…edded surveys

The form is displayed inline in the main app, so it can be manipulated much
more easily. In particular:
- both the form and the model are javascript objects passed in to a function so
  can be manipulated at will to customize the survey for a particular user.
- The result is also retrieved in the form, so results can be reflected
  immediately in the UX

The form is currently displayed as a view in the metrics tab, with its own
state.  I considered making it a popover instead, but wasn't sure that the
smaller width would work for the survey. I might experiment with that in a
future change.

The main changes to the app itself are to the main.js to define the new state,
and a new survey launcher `enketo_launch.js` that is the controller for the new
state.

The rest of the code is to check in the enketo javascript libraries. It is
theoretically possible to install them via `npm install` and then use `rollup`
or `webpack` to create the bundle. But we don't really have a gruntfile or
other build mechanism to invoke the rollup/webpack, and ES6 imports don't
necessarily play nice with angular 1. For example, when I tried using `import {
Form } from `enketo-core`, angular complained about the import.

Also, the resulting bundle file did not include an export for `Form` since
rollup inlines functions where possible. I had to manually add
`window.FormModule` to the javascript file to allow it to be used from the rest
of the app.

Given all this, I decided to check in enketo-core as a `manual_lib`.

This change can be tested by applying the following patch, although it is
pretty kludgy. Long-term, individual projects need to determine where they want
to launch the test from.

Also includes a couple of sample JSON files for testing.

The input to the library is the enketo paper formatted JSON files.

They can be obtained using the following process:
- Create the form using kobotoolbox https://github.com/kobotoolbox/ or https://build.opendatakit.org/ This will let you download an xlsform
- Convert xlsform -> xml form using http://xlsform.opendatakit.org/
- Convert xml form -> enketo paper format form + model using
  enketo-transformer. The curl option works fine - I used it with requests
- Copy the json files into www/json
- Launch them by passing the JSON file as a parameter while changing state, as in `metrics.js` below

```
diff --git a/www/js/metrics.js b/www/js/metrics.js
index d80e81d7..e7e4a3d3 100644
--- a/www/js/metrics.js
+++ b/www/js/metrics.js
@@ -35,6 +35,13 @@ angular.module('emission.main.metrics',['nvd3', 'emission.services', 'ionic-date
         $scope.onCurrentTrip();
     });

+  $scope.testSurvey = function() {
+    $state.go("root.main.enketosurvey", {
+        form_location: "json/enketo-survey-sample-full.json"
+    });
+  }
+
+
     // If we want to share this function (see the pun?) between the control screen and the dashboard, we need to put it into a service/factory.
     // But it is not clear to me why it needs to be in the profile screen...
     var prepopulateMessage = {
diff --git a/www/templates/main-metrics.html b/www/templates/main-metrics.html
index 124e95d9..923e0d05 100644
--- a/www/templates/main-metrics.html
+++ b/www/templates/main-metrics.html
@@ -10,6 +10,9 @@
       <button class="button button-icon" ng-click="doRefresh()">
         <i class="icon ion-refresh"></i>
       </button>
+      <button class="button button-icon" ng-click="testSurvey()">
+        TEST SURVEY
+      </button>
   </ion-nav-buttons>
   <ion-content class="has-header">
     <div style="height: 40px; width: 100%; padding: 10px;" ng-if="uictrl.showVis"> <!-- visualization control -->
```
@shankari
Copy link
Contributor Author

shankari commented May 2, 2019

This fixes e-mission/e-mission-docs#376

@shankari
Copy link
Contributor Author

shankari commented May 2, 2019

Note that the embedded surveys share a limitation with all enketo smart paper based surveys on recent android and iOS. It looks like for the geopicker widget, maps are not displayed for picking a location - only the lat/lng fields can be typed into. This is true of the embedded survey, the IAB and chrome on android.

I have filed enketo/enketo#209 for upstream to address it. While try other hacks if they can't fill it. I am not quite sure whether you really need the geowidget, given that you already have trajectories and places available for the user.

  • If you use forms that don't have a geo component, you are probably fine
  • If you can wait until the underlying bug is resolved, you are also fine

If not, you may want to use forms that don't have a geo/map component.

@shankari
Copy link
Contributor Author

shankari commented May 2, 2019

See attached video of the current functionality
embedded_form_demo.mp4.gz

This required some deep digging into the bowels of both enketo and ionic code,
but it works now. Please see
e-mission/e-mission-docs#376 for way more detail than
can fit here :)

At this point, the survey is displayed in the e-mission app in the same way as
in a mobile browser, but returns values to the app on validation.
Not sure why this didn't show up in the devapp, but I guess the devapp process rewrites the index to be more promiscuous. The real apps do honor the CSP, so we couldn't load some logos and map tiles properly.
e-mission/e-mission-docs#376 (comment)

These fixes were discussed in
e-mission/e-mission-docs#376 (comment)
and fortunately, they work!
@shankari
Copy link
Contributor Author

shankari commented May 3, 2019

@asiripanich, I have merged these fixes to https://e-mission.eecs.berkeley.edu/#/client_setup?new_client=martanetworksp18&clear_usercache=true&clear_local_storage=true so you can get a sense of what the surveys may look like before they are merged to your channel.

And it was good that I did, because I found and fixed e781d99 which was not visible in the devapp.

To test the form, you can click on the "TEST SURVEY" button at the top of the metrics tab.

The "TEST SURVEY" button is clearly just a placeholder. Now that you have this example, you might want to think about what surveys you want, and where you want to launch them from, so that @atton16 can implement the changes specific to your use case.

@shankari
Copy link
Contributor Author

shankari commented May 3, 2019

Also, @asiripanich once you are done with deciding how to use this for your use case, can you please contribute documentation on how to do so? Please ensure that this is integrated into the existing documentation and not a separate throwaway file as in e-mission/e-mission-docs#47

This is a third kind of data input, so it will probably go in here, potentially with a link to more details as for the external surveys.
https://github.com/e-mission/e-mission-docs/blob/master/docs/e-mission-both/supporting_user_inputs.md

@atton16
Copy link
Contributor

atton16 commented May 4, 2019

I merged the changes from shankari:xlsform_survey_support and it appears that the css from enketo-core broke a lot of internal app UI (tested on iOS).
i.e.

  • There is a shift up in the overall UI about the size of the status bar (screenshot 1 & 2).
  • There is a change in the looks of the input field on the date picker (screenshot 1).
  • There is a change in caret button next to the input field on the date picker (screenshot 1).
  • There is a change on arrow buttons of the date scroller (screenshot 2).

Screenshot 1
Screen Shot 2019-05-04 at 18 54 59

Screenshot 2
Screen Shot 2019-05-04 at 19 01 06

However, I have not yet test the core functionality of the new enketosurvey feature.

I will get back as soon as I get some results.

@atton16
Copy link
Contributor

atton16 commented May 4, 2019

Yay!! The survey UI is displayed. However, I am experiencing scroll lock on the survey page.

I will have a look around to see what went wrong.

Unfortunately, the internal app UI is still broken.

Screen Shot 2019-05-04 at 19 15 05

Screen Shot 2019-05-04 at 19 15 09

@atton16
Copy link
Contributor

atton16 commented May 4, 2019

@shankari Can you please give me a tips on how to do auto reload during the development?? It is difficult to force close the app and reconnect with the UI server every time I modified the code.

@atton16
Copy link
Contributor

atton16 commented May 4, 2019

Ran some tests on Android Emulator. I got these:

  • UI appears to be broken as well
  • The survey launched successfully
  • The survey does not exhibit scroll lock behaviour

@shankari
Copy link
Contributor Author

shankari commented May 4, 2019

There is a shift up in the overall UI about the size of the status bar (screenshot 1 & 2).
There is a change in the looks of the input field on the date picker (screenshot 1).
There is a change in caret button next to the input field on the date picker (screenshot 1).
There is a change on arrow buttons of the date scroller (screenshot 2).

@atton16 all of these are changes to the visual appearance, not to the functionality. I suspect they are related to the new css tags that come with the survey - just like how all the links are now red instead of blue. It should be pretty easy to change them by editing the survey css to only apply to elements within the survey div. I generally don't mess with the css, since theoretically every channel can modify their own L&F.

As I said in e-mission/e-mission-docs#376 (comment)

Also, I assume that you and @atton16 can make the design of the survey template be beautiful, consistent with the app, etc and launch it from the places that you want to.

Yay!! The survey UI is displayed. However, I am experiencing scroll lock on the survey page.

Not sure what you mean by this. Can you clarify? "x doesn't work" is not as helpful as "when I do x, y happens". Since this is functionality, if it is indeed broken, I will undertake to fix this.

@shankari Can you please give me a tips on how to do auto reload during the development?? It is difficult to force close the app and reconnect with the UI server every time I modified the code.

Again, can you provide more details of how/why this is not working? It works just fine for me (see attached video).

footprint_bootprint_change_reload.mp4.gz

@shankari
Copy link
Contributor Author

shankari commented May 4, 2019

Yay!! The survey UI is displayed. However, I am experiencing scroll lock on the survey page.

I tried this out on iOS and I see what you mean. Fortunately, stackoverflow comes to the rescue.
https://stackoverflow.com/a/37231467

It works now on both (see attached video).
scroll_works_on_both_yay.mp4.gz

This fixes
e-mission#563 (comment)
using a suggestion from https://stackoverflow.com/a/37231467

Fix was trivial, we just had to add `overflow-scroll="true"` to the `ion-content`
Fix documented in e-mission#563 (comment)
@shankari
Copy link
Contributor Author

shankari commented May 5, 2019

Just to clarify what I mean by css, since this has historically not been a very smooth process.

It should be pretty easy to change them by editing the survey css to only apply to elements within the survey div.

With the following diff to the css

diff --git a/www/manual_lib/enketo/css/formhub.css b/www/manual_lib/enketo/css/formhub.css
index 9a37841e..17d4aeb4 100644
--- a/www/manual_lib/enketo/css/formhub.css
+++ b/www/manual_lib/enketo/css/formhub.css
@@ -32,7 +32,7 @@ textarea {
   font-size: inherit;
   line-height: inherit; }

-a {
+article a {
   color: #ce4f07; }
   a:hover, a:focus {
     color: #843304; }
@@ -121,7 +121,7 @@ input, select, textarea {
 .affix {
   position: fixed; }

-.icon, .enketo-geopoint-marker, .glyphicon-chevron-up, .glyphicon-chevron-down {
+article .icon, .enketo-geopoint-marker, .glyphicon-chevron-up, .glyphicon-chevron-down {
   width: 15px;
   height: 15px;
   display: inline-block;

The links are back to blue and the arrows next to the datepicker look better. Again, this is a L&F issue, not a functionality issue. I encourage you to make similar changes as part of styling the survey and submit the fixes to master.

TIA for contributing!

Links are blue again Arrows look fine again
Simulator Screen Shot - iPhone 7 - 2019-05-04 at 18 38 08 Simulator Screen Shot - iPhone 7 - 2019-05-04 at 18 38 30

@atton16
Copy link
Contributor

atton16 commented May 5, 2019

Again, can you provide more details of how/why this is not working? It works just fine for me (see attached video).

footprint_bootprint_change_reload.mp4.gz

Oh, I can see that your e-mission-devapp periodically calls /__api__/autoreload. Mine dev app never fires that api call. First I thought it was the iOS version of the app as I se you are running the dev app on android. As I tried on the android emulator, it got me the same result. No api call to /__api__/autoreload.

The step looks simple enough with npm run serve on e-mission-phone then run the dev app and connect to e-mission-phone server.

I have been doing a Modify-Force Exit-Launch-Connect loop for months and thought that this is normal.

I am not certain what I might have missed at this point.

@atton16
Copy link
Contributor

atton16 commented May 5, 2019

Here I left the app running for a while and there is zero attempt to try autoreload`.

Screen Shot 2019-05-05 at 19 23 27

@shankari
Copy link
Contributor Author

shankari commented May 5, 2019

@atton16 can we pull out the devapp questions into their own issue since they seem a bit involved and are not really related to this PR?

@atton16
Copy link
Contributor

atton16 commented May 6, 2019

@atton16 can we pull out the devapp questions into their own issue since they seem a bit involved and are not really related to this PR?

Not sure what is wrong but I cannot open a new issue on this repo.

Is is okay if I open a new issue on e-mission/e-mission-docs ?

@shankari
Copy link
Contributor Author

shankari commented May 6, 2019

Issues: Since this repository is part of a larger project, all issues are tracked in the central docs repository. If you have a question, as suggested by the open source guide, please file an issue instead of sending an email. Since issues are public, other contributors can try to answer the question and benefit from the answer.

@atton16
Copy link
Contributor

atton16 commented May 6, 2019

For the broken UI,

after inserting two enketo-related css files, I did try to put all selectors under enketo-plugin class as the activation class in hope that this would help fix all UI-related issues. For more, Check this gist

It turns out that it resolved all of the UI issues except the total shift up in iOS devices.

I will try something else and come back again.

@shankari
Copy link
Contributor Author

shankari commented May 6, 2019

It turns out that it resolved all of the UI issues except the total shift up in iOS devices.

how bad is the total shift up in iOS devices?
I am looking at the screenshot from #563 (comment) and I don't see any substantial differences.

@shankari
Copy link
Contributor Author

shankari commented May 6, 2019

@atton16 I am testing enketo/enketo#235 with iOS now and I see the displacement even on that branch, which does not have any of the enketo changes. Are you sure this is not just something in master - i.e. are you sure that it did not have this displacement before?

@shankari
Copy link
Contributor Author

shankari commented May 7, 2019

@atton16 I take that back, the list view looks fine on enketo/enketo#235. And actually, on this branch as well. See screenshots below. The arrows are still messed up because I don't have your changes, but the screen is not displaced. IIRC, when I saw the displaced screen, there was an error during page load, which complicated the rendering; you may want to look at your debugger to see if you can find anything.

Branch for mode and purpose This branch
Simulator Screen Shot - iPhone 7 - 2019-05-06 at 18 16 24 Simulator Screen Shot - iPhone 7 - 2019-05-06 at 18 19 29

@shankari
Copy link
Contributor Author

shankari commented May 7, 2019

Can you send out a PR or a patch with your css changes? I can apply them to my branch, see if I still see the displacement, and if not, merge to master. Then you can always just recreate your branch from master if you are not able to get the spacing right.

@shankari
Copy link
Contributor Author

shankari commented May 7, 2019

after inserting two enketo-related css files, I did try to put all selectors under enketo-plugin class as the activation class in hope that this would help fix all UI-related issues. For more, Check this gist

Wait, the gist has you making changes to the print view

    <link href="manual_lib/enketo/css/formhub-print.css" media="print" rel="stylesheet">

As far as I know, formhub-print.css should not even be used because it is only for media=print. In fact, I first used formhub-print.css accidentally by using media="all" instead of media="print" and it failed very badly.

@atton16
Copy link
Contributor

atton16 commented May 14, 2019

Can you send out a PR or a patch with your css changes? I can apply them to my branch, see if I still see the displacement, and if not, merge to master. Then you can always just recreate your branch from master if you are not able to get the spacing right.

Done enketo/enketo#308

@shankari
Copy link
Contributor Author

shankari commented May 14, 2019

I merged the css changes from enketo/enketo#308 and can still reproduce the issue with the displacement. But, and this is interesting, the displacement happens only when the survey is launched. See attached video - when the app is first launched, the UX is not displaced. Then when the survey is launched, there's the additional line at the bottom.

ios_displacement_video.mp4.gz

@shankari
Copy link
Contributor Author

Comparing the extent of the body tag in the inspector, it is pretty clear that the tag is "moved up" after the survey is displayed.

Normal body Body moved up
Simulator Screen Shot - iPhone 7 - 2019-05-14 at 11 37 24 Simulator Screen Shot - iPhone 7 - 2019-05-14 at 11 36 21

@shankari
Copy link
Contributor Author

I tried to compare the html before and after loading the survey and could not find substantial differences. I then loaded the form in a browser, and noticed that the page scrolled to hide the page navigation at the top. I wonder if this displacement is happening in javascript...

@shankari
Copy link
Contributor Author

ok so I added multiple breakpoints in the javascript, and the line that causes the displacement is

            $scope.form = new $window.FormModule( formSelector, data, {});
--->     var loadErrors = $scope.form.init();
            if (loadErrors.length > 0) {
                $ionicPopup.alert({template: "loadErrors: " + loadErrors.join(",")});
            }
Before init After init
Simulator Screen Shot - iPhone 7 - 2019-05-14 at 12 43 03 Simulator Screen Shot - iPhone 7 - 2019-05-14 at 12 39 18

@shankari
Copy link
Contributor Author

After some more breakpointing and digging deeper into the rabbit hole, I narrowed it down to this line. We scroll the page until pageEl is in view. That scrolling breaks the body, maybe because this is already in a separate scroll view for the ion-view?
https://github.com/enketo/enketo-core/blob/master/src/js/page.js#L302

@shankari
Copy link
Contributor Author

This is the pageEl that we are trying to scroll into view.
Unsure why this breaks things.
Screen Shot 2019-05-14 at 1 04 25 PM

It turned out to be due to the enketo paging module scrolling to show the page
element that needs the focus. Since we are using ionic scrolling, it looks like
it already scrolls to show the top element, which works.

Please see the next 5-6 comments after
e-mission#563 (comment)

This doesn't _seem_ to break anything, but let me know if it does!
@shankari
Copy link
Contributor Author

Commenting out that line - e.g.

diff --git a/www/manual_lib/enketo/enketo-bundle.js b/www/manual_lib/enketo/enketo-bundle.js
index 866b681a..731541aa 100644
--- a/www/manual_lib/enketo/enketo-bundle.js
+++ b/www/manual_lib/enketo/enketo-bundle.js
@@ -23761,7 +23761,7 @@ var enketocore = (function () {
                    .eq( 0 )
                    .trigger( 'fakefocus' );

-               pageEl.scrollIntoView();
+               // pageEl.scrollIntoView();
            },
            _toggleButtons( index ) {
                const i = index || this._getCurrentIndex(),

fixes the issue

@shankari
Copy link
Contributor Author

Follow on to af94688
This time, it is to scroll to the location with errors
@asiripanich
Copy link
Member

@shankari the survey looks good. Scrolling is no longer an issue. However, I found a UI bug that I haven't been able to replicate. The bug caused the back button to Dashboard disappear.

@shankari
Copy link
Contributor Author

shankari commented May 16, 2019

The bug caused the back button to Dashboard disappear.

@asiripanich

Hm, was that in the devapp or in the test channel? If it is in the devapp, I am pretty sure I know the reason [1]. If in the test channel, yeah would be good to remember what you did to reproduce. $angularStateProvider (which is what we use for tabs) is pretty stable.

Also, I currently have the survey as a second view in the dashboard tab since I wanted to get a Proof of Concept working. It can definitely be a second view in a different tab (e.g. diary/profile) or in a tab of its own. Are you even going to have the dashboard in your version of the app?

I think we need to figure that out along with where the survey should be launched from, the contents of the survey,....

[1] if you edit a file and cause the app to reload when you are on the survey page, then the survey is the first page loaded in that view, and so there is no there is no back button to the "previous" dashboard page.

@asiripanich
Copy link
Member

@shankari I have just realised that you didn't merge this into main! what was your reason? :) after all these great efforts you put in.

@shankari
Copy link
Contributor Author

@asiripanich I didn't merge it into master because it is not complete.
It displays the survey but it doesn't load/store results.

@atton16 picked this up and implemented additional changes on top.
However, there was never a point at which we had only the framework and no UI.
Since full surveys are not our core use case, I didn't not merge that into master as well.

I think that merging the survey framework code into master is a great idea, but it needs to be cleaned up and made modular.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants