Include sync status in Hamburg menu #3382
Conversation
static/js/controllers/inbox.js
Outdated
| $scope.replicationStatus.lastCompleted = now; | ||
| } | ||
| $scope.replicationStatus.current = update.state; | ||
| $scope.replicationStatus.icon = 'fa-' + SYNC_ICON[update.status]; |
There was a problem hiding this comment.
Is it just me or is this adding too many "fa-"? Remove the prefix here.
There was a problem hiding this comment.
Fa-fa-fa-fa-fa-fa-fa-fa-fa-far better
templates/partials/header.html
Outdated
| <li role="separator" class="divider" ng-if="!replicationStatus.disabled"></li> | ||
| <li role="presentation disabled" ng-show="!replicationStatus.disabled && replicationStatus.current !== 'unknown'"> | ||
| <a> | ||
| Last sync <i class="fa fa-fw fa-refresh"></i><br> |
templates/partials/header.html
Outdated
| <li role="presentation disabled" ng-if="!replicationStatus.disabled" class="sync-status"> | ||
| <a ng-class="replicationStatus.current"> | ||
| <i class="fa fa-fw" ng-class="replicationStatus.icon"></i> | ||
| <span>{{'sync.status.' + replicationStatus.current | translate}}</span> |
There was a problem hiding this comment.
Don't concatenate keys - it makes it really hard to find if they're in use. Instead make replicationStatus.current have the full key, or have a replicationStatus.key.
templates/partials/header.html
Outdated
| <li role="presentation disabled" ng-if="!replicationStatus.disabled" class="sync-status"> | ||
| <a ng-class="replicationStatus.current"> | ||
| <i class="fa fa-fw" ng-class="replicationStatus.icon"></i> | ||
| <span>{{'sync.status.' + replicationStatus.current | translate}}</span> |
There was a problem hiding this comment.
Use <span translate>{{replicationStatus.current}}</span> - it reduces our watch count.
There was a problem hiding this comment.
👍 Good to know someone's watching the watches.
translations/messages-en.properties
Outdated
| contact.profile.delivery_code.unknown = Delivery | ||
| sync.status.in_progress=Currently syncing… |
There was a problem hiding this comment.
I think you need spaces around the = operator, but I could be wrong...
There was a problem hiding this comment.
Not sure they're required functionally, but definitely are for consistency. Added.
static/js/controllers/inbox.js
Outdated
| var delay = last ? (now - last) / 1000 : 'unknown'; | ||
| $log.info('Replicate ' + status.direction + ' server successful with ' + delay + ' seconds since the previous successful replication.'); | ||
|
|
||
| var now; |
There was a problem hiding this comment.
Surely now must always be Date.now()? I think it's possible it'll be undefined at line 84. Just initialise it here, or inline it everywhere and remove the now var altogether.
There was a problem hiding this comment.
Not possible it will be undefined at line 84, but I agree it's hard to follow the code. Seems logical to me to keep the value of now the same if assigning it in two different places - someone may think it's logical at a later date to compare the values at lastSuccess and lastCompleted, and my be surprised if they differ.
I'll move the assignment in line with the variable declaration.
static/css/inbox.less
Outdated
| @@ -245,6 +245,15 @@ body { | |||
| } | |||
| } | |||
|
|
|||
| .sync-status { | |||
| .in_progress, .not_required { | |||
| color: green; | |||
There was a problem hiding this comment.
Always try and use existing variables for colours because then they'll be consistent with our palette rather than every commit adding a new colour. Also if we decide to change what colour should represent "ok" then we only need to change it in one place. The current variables are defined in variables.less. Consider using @ok-color.
static/css/inbox.less
Outdated
| color: green; | ||
| } | ||
| .required { | ||
| color: red; |
There was a problem hiding this comment.
Same as above - try @error-color
translations/messages-en.properties
Outdated
| contact.profile.delivery_code.unknown = Delivery | ||
| sync.status.in_progress=Currently syncing… | ||
| sync.status.not_required=All reports synced |
There was a problem hiding this comment.
Should this be "reports" or "docs"? I think docs is more technically correct but makes less sense to users maybe?
There was a problem hiding this comment.
Good question. I'm not sure if there was specific thinking behind this wording from the design team or not.
I agree that docs is technically more correct, but perhaps reports is more relevant to a user - he'll may be worrying whether the things he has submitted via the Reports tab have been sent to the server. Although... the Reports tab is normally hidden, and he'll generally filling out reports via Contacts or Tasks...
N.B. currently the only use of the word docs in user-facing text is in the About tab's db debug info, and soon to be in the boot screen...
Closes #3357