Port over the UI fixes related to the plugin interface changes #641
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
shankari
commented
Apr 19, 2020
shankari
commented
- small build improvement
In order to be compatible with restrictions in Android Pie. https://android-developers.googleblog.com/2018/04/protecting-users-with-tls-by-default-in.html Both of them support https already so there is no reason to not switch
- which explains which permissions we need - why we need them, and - which prompt to select
Since after upgrading to [email protected], the build Just Works even with the new build system
To clarify which button people should press, specific to the version. This fixes e-mission/e-mission-docs#483
The switch to WKWebView forced the use of CORS. There is no workaround. https://ionicframework.com/docs/v3/wkwebview/#cors We didn't encounter this in covid-19 repo since we made all calls through the built-in native plugin that automatically adds the user token and sends out the call. In emission, though, we need make aggregate calls that should not have a user token. We had been using XHR for this (through the angular `$http` server) and all those calls broke. While we could make the appropriate change on the server side, that would not be consistent with our long term goal (e-mission/e-mission-docs#408, e-mission/e-mission-docs#288) So for now, I use the alternative documented here https://ionicframework.com/docs/v3/wkwebview/#i-cant-implement-cors Concretely: - add the native plugin - add a new method `CommHelper.getAggregateData` that wraps it in a promise - change all instances of `$http.post` -> `CommHelper.getAggregateData` - use the configured connectUrl instead of hardcoding - remove the hardcoded URL from index.html Bonus fix: - Dealt with error in the heatmap if there was no data and thus, no bounds by checking to see if the bounds were valid Testing done: Navigated through the app screens until all the aggregate calls were invoked (see below). No errors in the console. ``` [Log] getting aggregate data without user authentication from http://localhost:8080/result/metrics/timestamp with arguments {"freq":"D","start_time":1586131200,"end_time":1587254400,"metric_list":["duration","median_speed","count","distance"],"is_return_aggregate":true} (cordova.js, line 1540) [Log] getting aggregate data without user authentication from http://localhost:8080/result/heatmap/incidents/timestamp with arguments {"start_time":1587187062,"end_time":1587359861,"sel_region":null} (cordova.js, line 1540) [Log] getting aggregate data without user authentication from http://localhost:8080/result/heatmap/pop.route/local_date with arguments {"modes":null,"from_local_date":{"year":2020,"month":4,"day":17,"hour":22},"to_local_date":{"year":2020,"month":4,"day":18,"hour":22},"sel_region":null} (cordova.js, line 1540) [Log] getting aggregate data without user authentication from http://localhost:8080/result/heatmap/incidents/local_date with arguments {"modes":null,"from_local_date":{"year":2020,"month":4,"day":17,"hour":22},"to_local_date":{"year":2020,"month":4,"day":18,"hour":22},"sel_region":null} (cordova.js, line 1540) ```
This involved two fixes:
- recreating the config on resume so that the end date is reset (`list.js`), and
- setting "today" to the correct date when the datepicker is opened
(`ionic-datepicker.bundle.min.js`)
I have also added build instructions to my fork of the ionic-datepicker repo,
but in case I delete the fork later, the instructions are:
Install node 10.19.0 ([gulp-sass will fail for more recent version of node](nodejs/node-gyp#1763))
```
node -v
v10.19.0
```
Install and check gulp version
```
npm install
npm list | grep gulp
├─┬ [email protected]
```
Build
```
npx gulp build
```
Testing done:
Followed the steps in e-mission/e-mission-docs#531 (comment)
Both end date and today are updated correctly
Fix 'today' in the datepicker
+ Add a list of used Open Source Licenses
+ remove used modules
- one from bower.json
- one from manual_lib
+ from index.html
Fix license
at the bottom of `package.json`
Add a couple of additional dependencies
…into port_ui_fixes
Major changes:
- the interface is already promisified, so we don't need to wrap the calls in promises
- some of the functions return slightly different values, supporting them now
- the updatecheck returns a build number that we display for greater debuggability
- Need to add the ionic hosts back to the CSP
Testing done:
- Ran the app with these changes
- Clicked on the link at
https://e-mission.eecs.berkeley.edu/#/client_setup?new_client=urap2017emotion
- UI was updated to the emotion UI
After the update, we had the following errors:
```
[Error] Failed to load resource: the server responded with a status of 404 () (others.js, line 0)
[Error] Ionic Deploy: Unable to check for updates – TypeError: deploy.init is not a function. (In 'deploy.init(config, resolve, reject)', 'deploy.init' is undefined) — updatecheck.js:38
TypeError: deploy.init is not a function. (In 'deploy.init(config, resolve, reject)', 'deploy.init' is undefined) — updatecheck.js:38(anonymous function) — updatecheck.js:38Promise(anonymous function) — updatecheck.js:33(anonymous function) — updatecheck.js:163(anonymous function) — updatecheck.js:204processQueue — ionic.bundle.js:29127(anonymous function) — ionic.bundle.js:29143$digest — ionic.bundle.js:30211(anonymous function) — ionic.bundle.js:30434completeOutstandingRequest — ionic.bundle.js:19194(anonymous function) — ionic.bundle.js:19470
(anonymous function) (cordova.js:1540)
(anonymous function) (updatecheck.js:199)
promiseReactionJob
[Error] Refused to load unsafe:ionic://localhost/img/neutralbear.gif because it does not appear in the img-src directive of the Content Security Policy.
```
These are because the channel has not yet been updated to use the new UI
changes with the new updatecheck code and the `ionic:` whitelist. Some of these
(like `others.js`) may also just be obsolete.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.