Uiwork: Renamed Files and Deleted Extraneous Icon Pictures #471
Conversation
|
Can one of the admins verify this patch? |
| .controller('RecommendationsCtrl', function(CommHelper, $state, $ionicLoading, $scope, $rootScope, $ionicModal, nzTour, | ||
| $window, $http, $ionicPopup, $timeout, storage, ReferralHandler, ReferHelper, Logger, $cordovaInAppBrowser, SurveyLaunch) { | ||
| $scope.cSF = 1000; | ||
| var map = document.getElementById('mapModal'); |
There was a problem hiding this comment.
this is not angular style.
Please look at how to do this properly in angular.
www/js/splash/startprefs.js
Outdated
| @@ -230,8 +230,8 @@ angular.module('emission.splash.startprefs', ['emission.plugin.logger', | |||
| return startprefs.getPendingOnboardingState().then(function(result){ | |||
| if (result == null) { | |||
| var temp = ReferralHandler.getReferralNavigation(); | |||
| if (temp == 'goals') { | |||
There was a problem hiding this comment.
I would suggest reverting the changes to this file.
I have a large pending restructure of the file (#470) and you are going to get a merge conflict.
There was a problem hiding this comment.
And this code is to deal with referrals, which is not really an issue for you right now.
| "connectUrl": "http://52.91.65.149:8080", | ||
| "android": { | ||
| "auth": { | ||
| "method": "prompted-auth", |
There was a problem hiding this comment.
I really have four main changes:
- revert the changes to startprefs
- revert the zephyr rename
- remove the icons which are still not removed (see
www/img/primarylogo.png) - move the css stuff to a css file
Ideally, you would also rewrite the recommedation to be more angular-like, but I am not going to insist on that at this time.
| @@ -87,7 +87,7 @@ angular.module('emission.splash.startprefs', ['emission.plugin.logger', | |||
| startprefs.isConsented = function() { | |||
| logger.log("curr_consented = "+$rootScope.curr_consented+ | |||
| "isIntroDone = " + startprefs.isIntroDone()); | |||
| if (startprefs.isIntroDone() && | |||
There was a problem hiding this comment.
This file also has a bunch of extraneous changes that are not relevant to any functionality like this one.
www/templates/main-metrics.html
Outdated
| @@ -18,7 +18,7 @@ | |||
| <div ng-class="chartButtonClass()" ng-click="showChart()">Chart</div> | |||
| </div> | |||
| </div> | |||
| <div ng-if="uictrl.showChart" style="width:50%; float:right; padding-top: 10px;"> | |||
| <div ng-if="uictrl.shxowChart" style="width:50%; float:right; padding-top: 10px;"> | |||
| </button> | ||
| </ion-nav-buttons> | ||
| <style> | ||
| .recommendbox { |
There was a problem hiding this comment.
This needs to move into css files instead of being inline. Again, please check the recommendations for angular1
| <p>The drinks are ok but the service is kind of slow. | ||
| <p><img src="img/small/small_4.png" width="50" /> - Sam, Boston | ||
| </div> | ||
| </right> |
There was a problem hiding this comment.
Where is the corresponding <right>?
|
Also, why do I see three pull requests here? Aren't they for the same change? |
There was a problem hiding this comment.
You still have the following pending changes:
- really revert all changes to startprefs. It should not show up in the list of modified files.
- revert the change to the zephyr connection config
- move the CSS out
- remove the icons which are still not removed (see www/img/regular_0.png, www/img/regular_1.png, www/img/regular_2.png, ....) or justify why they need to remain
www/js/splash/startprefs.js
Outdated
| @@ -230,8 +230,8 @@ angular.module('emission.splash.startprefs', ['emission.plugin.logger', | |||
| return startprefs.getPendingOnboardingState().then(function(result){ | |||
| if (result == null) { | |||
| var temp = ReferralHandler.getReferralNavigation(); | |||
| if (temp == 'recommendations') { | |||
| return 'root.main.recommendations'; | |||
| if (temp == 'goals') { | |||
There was a problem hiding this comment.
@trevor-wu This revert is not complete. The file still has the other extraneous changes, and they will cause merge conflicts. Please use git to revert the changes instead of reverting manually. This pull request is now really messy - you may also want to create a new branch, include only the relevant changes and create a new pull request from it.
|
@trevor-wu can you please add a comment when you are done with all changes and I should re-review? Thanks! |
|
@trevor-wu Also, can you please close either this PR or #468 ? |
renamed main-goals.html to main-recommendations.html
renamed goals.js to recommendations.js
deleted pictures that definitely won't be used