Implement Toast Service #350
Conversation
| .hideDelay(hideDelay || self.hideDelay) | ||
| .textContent(message); | ||
| if (typeName) { | ||
| toast.theme(self.TypeName[type]); |
There was a problem hiding this comment.
Did you intend to use typename here vs self.TypeName[type]?
|
|
||
| self.show = function(message, type, hideDelay, stopTranslate) { | ||
| if (type > self.loggingType) { | ||
| return; |
There was a problem hiding this comment.
Maybe we should log an error here before returning?
There was a problem hiding this comment.
Yeh it may be worth doing this. I will get it to log all messages to the log file, if enabled.
| 'use strict'; | ||
|
|
||
| angular.module('arkclient.services') | ||
| .service('configService', ['$mdToast', 'gettextCatalog', ConfigService]); |
There was a problem hiding this comment.
Looks like some of these services aren't being used
There was a problem hiding this comment.
Ah yes, I removed the parameters from the constructor, but not here!
| * ToastService | ||
| * @constructor | ||
| */ | ||
| function ToastService(configService, $mdToast, gettextCatalog) { |
There was a problem hiding this comment.
This is passed in. I agree, I'd use camelcase normally but wanted to keep it consistent with the value on line 6/7 (as well as other places like account.controller.js):
angular.module('arkclient.services')
.service('toastService', ['configService', '$mdToast', 'gettextCatalog', ToastService]);
| return; | ||
| } | ||
| self.fileStream.write( | ||
| '[' + new Date().toISOString() + '] ' + (typeName ? typeName.toUpperCase() + ': ' : '') + message + '\n' |
There was a problem hiding this comment.
Would look a bit cleaner to use string interpolation here
|
Thanks @spresnal! I've made all your changes aside from the camel case comment, if you'd like to take a look at my response :) I agree with the hideDelay. I just kept it consistent with current functionality for now with the option of changing in the future if needed. Would be good to get the opinion of @fix and/or @luciorubeens, as to whether it's something we need to worry about. |
|
Great! +10 to ConfigService and ToastService 👍🏻 |
@fix @luciorubeens please take a look and see if this is implemented the way you were looking for. As I said in the issue, I've replaced all existing toasts to use the Service, but have set them up as either SUCCESS or ERROR. Please see below as I've added colours for each toast - GitHub has blown the images up so they're not that blurry in practice.
Config can be changed in
client/app/config/config.js:logFile- true or false - appends tologs/ark.logif enabled.level- what log messages to show/log. Below is the mapping - this value shows itself and above (E.g. 1 shows ERROR and SUCCESS messages)defaultHideDelay- how long until the toast disappears. Some places have been overridden, but the majority will use this valueLogging Enum
ERROR
SUCCESS
WARN
LOG
DEBUG
Resolves #344