Imporves perf of bookmark hanger

Resolves #9674 Auditors: @bsclifton Test Plan: - have a lot of bookmarks (1k +) - try to add a bookmark with clicking on the star
brave · Jul 11, 2017 · 21e2b13 · 21e2b13
1 parent 03ae74b
commit 21e2b13
Show file tree

Hide file tree

Showing 26 changed files with 1,050 additions and 458 deletions.
diff --git a/app/browser/menu.js b/app/browser/menu.js
@@ -677,6 +677,8 @@ const doAction = (state, action) => {
       }
       break
     case appConstants.APP_ADD_SITE:
+    case appConstants.APP_ADD_BOOKMARK:
+    case appConstants.APP_EDIT_BOOKMARK:
       {
         if (action.tag === siteTags.BOOKMARK || action.tag === siteTags.BOOKMARK_FOLDER) {
           createMenu(state)

diff --git a/app/browser/reducers/sitesReducer.js b/app/browser/reducers/sitesReducer.js
@@ -54,35 +54,57 @@ const sitesReducer = (state, action, immutableAction) => {
         break
       }
     case appConstants.APP_ADD_SITE:
-      const isSyncEnabled = syncEnabled()
-      if (Immutable.List.isList(action.siteDetail)) {
-        action.siteDetail.forEach((s) => {
-          state = siteUtil.addSite(state, s, action.tag, undefined, action.skipSync)
+      {
+        const isSyncEnabled = syncEnabled()
+        if (Immutable.List.isList(action.siteDetail)) {
+          action.siteDetail.forEach((s) => {
+            state = siteUtil.addSite(state, s, action.tag, action.skipSync)
+            if (isSyncEnabled) {
+              state = syncUtil.updateSiteCache(state, s)
+            }
+          })
+        } else {
+          let sites = state.get('sites')
+          if (!action.siteDetail.get('folderId') && siteUtil.isFolder(action.siteDetail)) {
+            action.siteDetail = action.siteDetail.set('folderId', siteUtil.getNextFolderId(sites))
+          }
+          state = siteUtil.addSite(state, action.siteDetail, action.tag, action.skipSync)
           if (isSyncEnabled) {
-            state = syncUtil.updateSiteCache(state, s)
+            state = syncUtil.updateSiteCache(state, action.siteDetail)
           }
-        })
-      } else {
-        let sites = state.get('sites')
-        if (!action.siteDetail.get('folderId') && siteUtil.isFolder(action.siteDetail)) {
-          action.siteDetail = action.siteDetail.set('folderId', siteUtil.getNextFolderId(sites))
         }
-        state = siteUtil.addSite(state, action.siteDetail, action.tag, action.originalSiteDetail, action.skipSync)
-        if (action.destinationDetail) {
-          const sourceKey = siteUtil.getSiteKey(action.siteDetail)
-          const destinationKey = siteUtil.getSiteKey(action.destinationDetail)
+        break
+      }
+    case appConstants.APP_ADD_BOOKMARK:
+    case appConstants.APP_EDIT_BOOKMARK:
+      {
+        const isSyncEnabled = syncEnabled()
+        const sites = state.get('sites')
+        const closestKey = action.closestKey
+        let site = action.siteDetail
 
-          if (sourceKey != null) {
-            state = siteUtil.moveSite(state,
-              sourceKey, destinationKey, false, false, true)
-          }
+        if (site == null || action.tag == null) {
+          break
         }
+
+        if (!site.get('folderId') && action.tag === siteTags.BOOKMARK_FOLDER) {
+          site = site.set('folderId', siteUtil.getNextFolderId(sites))
+        }
+
+        state = siteUtil.addSite(state, site, action.tag, action.editKey)
+
+        if (closestKey != null) {
+          const sourceKey = siteUtil.getSiteKey(site)
+          state = siteUtil.moveSite(state, sourceKey, closestKey, false, false, true)
+        }
+
         if (isSyncEnabled) {
-          state = syncUtil.updateSiteCache(state, action.destinationDetail || action.siteDetail)
+          state = syncUtil.updateSiteCache(state, site)
         }
+
+        state = updateActiveTabBookmarked(state)
+        break
       }
-      state = updateActiveTabBookmarked(state)
-      break
     case appConstants.APP_REMOVE_SITE:
       const removeSiteSyncCallback = action.skipSync ? undefined : syncActions.removeSite
       state = siteUtil.removeSite(state, action.siteDetail, action.tag, true, removeSiteSyncCallback)

diff --git a/app/browser/reducers/urlBarSuggestionsReducer.js b/app/browser/reducers/urlBarSuggestionsReducer.js
@@ -14,6 +14,8 @@ const tabState = require('../../common/state/tabState')
 const urlBarSuggestionsReducer = (state, action) => {
   switch (action.actionType) {
     case appConstants.APP_ADD_SITE:
+    case appConstants.APP_ADD_BOOKMARK:
+    case appConstants.APP_EDIT_BOOKMARK:
       if (Immutable.List.isList(action.siteDetail)) {
         action.siteDetail.forEach((s) => {
           add(s)

diff --git a/app/common/lib/bookmarkUtil.js b/app/common/lib/bookmarkUtil.js
@@ -16,17 +16,20 @@ const {calculateTextWidth} = require('../../../js/lib/textCalculator')
 const {iconSize} = require('../../../js/constants/config')
 const {getSetting} = require('../../../js/settings')
 
-const bookmarkHangerHeading = (detail, isFolder, shouldShowLocation) => {
+function bookmarkHangerHeading (editMode, isFolder, isAdded) {
   if (isFolder) {
-    return shouldShowLocation
+    return editMode
       ? 'bookmarkFolderEditing'
       : 'bookmarkFolderAdding'
   }
-  return shouldShowLocation
-    ? (!detail || !detail.has('location'))
-      ? 'bookmarkCreateNew'
-      : 'bookmarkEdit'
-    : 'bookmarkAdded'
+
+  if (isAdded) {
+    return 'bookmarkAdded'
+  }
+
+  return editMode
+    ? 'bookmarkEdit'
+    : 'bookmarkCreateNew'
 }
 
 const displayBookmarkName = (detail) => {
@@ -37,11 +40,10 @@ const displayBookmarkName = (detail) => {
   return detail.get('title') || ''
 }
 
-const isBookmarkNameValid = (detail, isFolder) => {
-  const title = detail.get('title') || detail.get('customTitle')
-  const location = detail.get('location')
+const isBookmarkNameValid = (title, location, isFolder, customTitle) => {
+  const newTitle = title || customTitle
   return isFolder
-    ? (title != null && title !== 0) && title.trim().length > 0
+    ? (newTitle != null && newTitle !== 0) && newTitle.trim().length > 0
     : location != null && location.trim().length > 0
 }