Feature/contacts history #29
Conversation
Add new dependencies for 2 pipes
There was a problem hiding this comment.
Hey @MurhafSousli, thank you for this and for opening tickets for the missing bits! I know this feature has been a sort of a pain and a lot of investigation was needed. Before testing this PR deeper, there are a few files that have changes and they look to me as auto-generated files. Some of these files, IMHO, shouldn't be changed (most of package.json content, config.xml, etc.) for this feature. My comments try to point you to some of these changes. Can you please have a look them and let me know what you think?
config.xml
Outdated
| <plugin name="cordova-plugin-device" spec="1.1.4"/> | ||
| <plugin name="cordova-plugin-splashscreen" spec="~4.0.1"/> | ||
| <name>WiFiCalling</name> | ||
| <description>An awesome Ionic/Cordova app.</description> |
There was a problem hiding this comment.
As you are updating this file, can you please also set the proper description, team and so on?
package.json
Outdated
| @@ -1,8 +1,8 @@ | |||
| { | |||
| "name": "wifi-calling", | |||
| "version": "0.0.1", | |||
| "author": "EyeSeeTea Team", | |||
| "homepage": "http://eyeseetea.com/", | |||
| "author": "Ionic Framework", | |||
There was a problem hiding this comment.
Can you please keep the current author and homepage?
package.json
Outdated
| "@ionic-native/core": "3.12.1", | ||
| "@ionic-native/splash-screen": "3.12.1", | ||
| "@ionic-native/status-bar": "3.12.1", | ||
| "@angular/animations": "^4.1.3", |
There was a problem hiding this comment.
I don't think we should downgrade the version unless there is a reason to do so, right?
There was a problem hiding this comment.
Right, 4.1.3 is the angular version generated by the current latest ionic cli. I had many difficulties getting our project to work with the emulator, it was throwing non-sense errors. finally I got it to work, downgrading angular version was the fix. these issues could be caused by npm 5 or ionic. I can't remember them in details! but there are couple of opened issues related to it. like this one
package.json
Outdated
| "cordova-plugin-contacts": "^2.3.1", | ||
| "cordova-plugin-device": "^1.1.4", | ||
| "cordova-plugin-inappbrowser": "^1.7.1", | ||
| "cordova-plugin-splashscreen": "^4.0.3", |
There was a problem hiding this comment.
Is splashscreen actually needed btw? Can you review we actually need all this bunch of libs?
There was a problem hiding this comment.
splashscreenis used to show welcome/loading page just before the app loads. I think it is important.inappbrowser"is suppoused to" let you launch other apps or web pages from the app. if it works as expected, we will use it to launch the native history app (call log). depends on thiscordova-plugin-devicefor device information, it also has the onDeviceReady event which we use.
Currently we are using all of them.
src/index.html
Outdated
| <!-- The bundle js is generated during the build process --> | ||
| <!-- The vendor js is generated during the build process | ||
| It contains all of the dependencies in node_modules --> | ||
| <script src="build/vendor.js"></script> |
There was a problem hiding this comment.
This was generated by ionic, it is part of the new update
| @@ -7,6 +7,5 @@ | |||
| }, | |||
| "rulesDirectory": [ | |||
| "node_modules/tslint-eslint-rules/dist/rules" | |||
| ], | |||
| "extends": "tslint-ionic-rules" | |||
| ] | |||
There was a problem hiding this comment.
Nope, it is also a part of the update. things are constantly being changed by ionic so it might be added again. I am just going with the framework as it goes. It's not stable as I thought!
|
Thanks @MurhafSousli ! I just merged it with development and some minor fixes. |
closes #4, (this PR extends the store PR #18)