review comments 2

brave · Sep 11, 2017 · 1c51823 · 1c51823
1 parent 524b614
commit 1c51823
Show file tree

Hide file tree

Showing 6 changed files with 29 additions and 12 deletions.
diff --git a/app/browser/api/textCalc.js b/app/browser/api/textCalc.js
@@ -17,14 +17,18 @@ const {makeImmutable} = require('../../common/state/immutableUtil')
 // Styles
 const globalStyles = require('../../renderer/components/styles/global')
 
-const fontSize = parseInt(globalStyles.spacing.bookmarksItemFontSize)
+const fontSize = globalStyles.spacing.bookmarksItemFontSize
 const fontFamily = globalStyles.defaultFontFamily
 
 const calcText = (item, type) => {
  const title = type === siteTags.BOOKMARK
  ? item.get('title') || item.get('location')
  : item.get('title')
 
+ if (title && title.length === 0) {
+ return
+ }
+
  const param = `
  (function() {
  let ctx = document.createElement('canvas').getContext('2d')
@@ -59,10 +63,15 @@ const calcText = (item, type) => {
  }
  })
 }
+
 const calcTextList = (list) => {
  const take = 500
  list = makeImmutable(list)
 
+ if (list.size === 0) {
+ return
+ }
+
  let paramList = JSON.stringify(list.take(take))
  .replace(/'/g, '!')
  .replace(/\\"/g, '!')
@@ -103,7 +112,7 @@ const calcTextList = (list) => {
 
  tabs.executeScriptInBackground(param, (err, url, result) => {
  if (err) {
- console.log('Error in executeScriptInBackground (textCalcUtil.js)')
+ console.error('Error in executeScriptInBackground (textCalcUtil.js)')
  }
 
  if (result[0]) {

diff --git a/app/browser/reducers/bookmarkFoldersReducer.js b/app/browser/reducers/bookmarkFoldersReducer.js
@@ -32,12 +32,14 @@ const bookmarkFoldersReducer = (state, action, immutableAction) => {
  }
 
  if (Immutable.List.isList(folder)) {
+ let folderList = Immutable.List()
  action.get('folderDetails', Immutable.List()).forEach((folder) => {
  const folderDetails = bookmarkFolderUtil.buildFolder(folder, bookmarkFoldersState.getFolders(state))
  state = bookmarkFoldersState.addFolder(state, folderDetails, closestKey)
  state = syncUtil.updateObjectCache(state, folderDetails, STATE_SITES.BOOKMARK_FOLDERS)
- textCalc.calcText(folderDetails, siteTags.BOOKMARK_FOLDER)
+ folderList = folderList.push(folderDetails)
  })
+ textCalc.calcTextList(folderList)
  } else {
  const folderDetails = bookmarkFolderUtil.buildFolder(folder, bookmarkFoldersState.getFolders(state))
  state = bookmarkFoldersState.addFolder(state, folderDetails, closestKey)

diff --git a/app/browser/reducers/bookmarksReducer.js b/app/browser/reducers/bookmarksReducer.js
@@ -36,12 +36,14 @@ const bookmarksReducer = (state, action, immutableAction) => {
  }
 
  if (Immutable.List.isList(bookmark)) {
+ let bookmarkList = Immutable.List()
  action.get('siteDetail', Immutable.List()).forEach((bookmark) => {
  const bookmarkDetail = bookmarkUtil.buildBookmark(state, bookmark)
  state = bookmarksState.addBookmark(state, bookmarkDetail, closestKey)
  state = syncUtil.updateObjectCache(state, bookmarkDetail, STATE_SITES.BOOKMARKS)
- textCalc.calcText(bookmarkDetail, siteTags.BOOKMARK)
+ bookmarkList = bookmarkList.push(bookmarkDetail)
  })
+ textCalc.calcTextList(bookmarkList)
  } else {
  const bookmarkDetail = bookmarkUtil.buildBookmark(state, bookmark)
  state = bookmarksState.addBookmark(state, bookmarkDetail, closestKey)

diff --git a/app/browser/tabs.js b/app/browser/tabs.js
@@ -350,8 +350,10 @@ const runScript = (win, debug, script, cb) => {
  cb(err, url, result)
  if (!debug) {
  backgroundProcessTimer = setTimeout(() => {
- win.close()
- backgroundProcess = null
+ if (backgroundProcess) {
+ win.close()
+ backgroundProcess = null
+ }
  }, 2 * 60 * 1000) // 2 min
  }
  })

diff --git a/test/unit/app/browser/reducers/bookmarkFoldersReducerTest.js b/test/unit/app/browser/reducers/bookmarkFoldersReducerTest.js
@@ -103,7 +103,8 @@ describe('bookmarkFoldersReducer unit test', function () {
  })
 
  const fakeTextCalc = {
- calcText: () => true
+ calcText: () => true,
+ calcTextList: () => true
  }
 
  before(function () {
@@ -182,7 +183,7 @@ describe('bookmarkFoldersReducer unit test', function () {
 
  it('folder data is list (multiple folders)', function () {
  spy = sinon.spy(bookmarkFoldersState, 'addFolder')
- spyCalc = sinon.spy(fakeTextCalc, 'calcText')
+ spyCalc = sinon.spy(fakeTextCalc, 'calcTextList')
  const newState = bookmarkFoldersReducer(state, {
  actionType: appConstants.APP_ADD_BOOKMARK_FOLDER,
  folderDetails: [
@@ -253,7 +254,7 @@ describe('bookmarkFoldersReducer unit test', function () {
  ]
  }))
  assert.equal(spy.callCount, 3)
- assert.equal(spyCalc.callCount, 3)
+ assert.equal(spyCalc.callCount, 1)
  assert.deepEqual(newState.toJS(), expectedState.toJS())
  })
  })

diff --git a/test/unit/app/browser/reducers/bookmarksReducerTest.js b/test/unit/app/browser/reducers/bookmarksReducerTest.js
@@ -124,7 +124,8 @@ describe('bookmarksReducer unit test', function () {
  })
 
  const fakeTextCalc = {
- calcText: () => true
+ calcText: () => true,
+ calcTextList: () => true
  }
 
  before(function () {
@@ -231,7 +232,7 @@ describe('bookmarksReducer unit test', function () {
 
  it('bookmark data is list (multiple bookmarks)', function () {
  spy = sinon.spy(bookmarksState, 'addBookmark')
- spyCalc = sinon.spy(fakeTextCalc, 'calcText')
+ spyCalc = sinon.spy(fakeTextCalc, 'calcTextList')
  const newState = bookmarksReducer(state, {
  actionType: appConstants.APP_ADD_BOOKMARK,
  siteDetail: [
@@ -300,7 +301,7 @@ describe('bookmarksReducer unit test', function () {
  ]
  }))
  assert.equal(spy.callCount, 2)
- assert.equal(spyCalc.callCount, 2)
+ assert.equal(spyCalc.callCount, 1)
  assert.deepEqual(newState.toJS(), expectedState.toJS())
  })
  })