Publisher #1
Publisher #1
Conversation
There was a problem hiding this comment.
few things wrote mention:
1.moment libary can help in popup file
2.console.logs all over the place, we need it in production?
3.Object.defineProperty i dont see any updated code use that syntax
also Something.property - can be trouble, dont use it.
i suggested few solutions in the pr.
| "build:chrome-production": "webpack -p --config chrome/webpack.config.prod.js", | ||
| "watch:chrome": "webpack --config chrome/webpack.config.js -w", | ||
| "build": "npm run clean && npm run build:chrome", | ||
| "build:production": "npm run build:angular-production && npm run build:chrome-production && npm run pack", |
| "watch:chrome": "webpack --config chrome/webpack.config.js -w", | ||
| "build": "npm run clean && npm run build:chrome", | ||
| "build:production": "npm run build:angular-production && npm run build:chrome-production && npm run pack", | ||
| "pack": "cd angular/dist && bestzip ../../extension-build.zip *" |
There was a problem hiding this comment.
i don't have or see any angular folder
| if(!localStorage.ignoredSites) { | ||
| localStorage.ignoredSites = JSON.stringify([]); | ||
| } | ||
| console.log("Load Config object"); |
There was a problem hiding this comment.
we always want to show logs? maybe it should be removed in production.
| console.log("Load Config object"); | ||
| } | ||
|
|
||
| Config.timeDisplayFormatEnum = { |
There was a problem hiding this comment.
in line 6 you export a function and now you adding properties?
| MINUTES: 1 | ||
| } | ||
|
|
||
| Config.prototype.addIgnoredSite = function(site) { |
There was a problem hiding this comment.
why to use prototype? its unsafe , not the convention.
|
|
||
| this._currentSite = null; | ||
| this._siteRegexp = /^(\w+:\/\/[^\/]+).*$/; | ||
| this._startTime = null; |
| let site = "unknown"; | ||
|
|
||
| let time = 0; | ||
| if(tab.url.indexOf("www.facebook.com") > -1) { |
| time = sites["https://www.facebook.com"]; | ||
| } | ||
|
|
||
| if(tab.url.indexOf("www.youtube.com") > -1) { |
| chrome.runtime.onMessage.addListener(function (request, sender, sendResponse) { | ||
| if(request.action === "redirect") { | ||
| redirectPage(request.tabToRedirect); | ||
| } |
There was a problem hiding this comment.
need refactor to updated code like rest
| /** | ||
| * Responsible for detecting focus change from tabs and windows. | ||
| */ | ||
| export default function Tracker(config, sites) { |
There was a problem hiding this comment.
same as mentioned in config, use Class or functions with default export.
the code is yet not production ready rather it about 90% there, it lift the main job