refactor of hanger

app/browser/reducers/sitesReducer.js

@@ -51,7 +51,7 @@ const sitesReducer = (state, action, immutableAction) => {
       const isSyncEnabled = syncEnabled()
       if (Immutable.List.isList(action.siteDetail)) {
         action.siteDetail.forEach((s) => {
-          state = siteUtil.addSite(state, s, action.tag, undefined, action.skipSync)
+          state = siteUtil.addSite(state, s, action.tag, action.skipSync)
           if (isSyncEnabled) {
             state = syncUtil.updateSiteCache(state, s)
           }
@@ -61,18 +61,9 @@ const sitesReducer = (state, action, immutableAction) => {
         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)
-
-          if (sourceKey != null) {
-            state = siteUtil.moveSite(state,
-              sourceKey, destinationKey, false, false, true)
-          }
-        }
+        state = siteUtil.addSite(state, action.siteDetail, action.tag, action.skipSync)
         if (isSyncEnabled) {
-          state = syncUtil.updateSiteCache(state, action.destinationDetail || action.siteDetail)
+          state = syncUtil.updateSiteCache(state, action.siteDetail)
         }
       }
       state = updateActiveTabBookmarked(state)

app/common/lib/bookmarkUtil.js

@@ -2,17 +2,16 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-function bookmarkHangerHeading (detail, isFolder, shouldShowLocation) {
+function bookmarkHangerHeading (editMode, isFolder) {
   if (isFolder) {
-    return shouldShowLocation
+    return editMode
       ? 'bookmarkFolderEditing'
       : 'bookmarkFolderAdding'
   }
-  return shouldShowLocation
-    ? (!detail || !detail.has('location'))
-      ? 'bookmarkCreateNew'
-      : 'bookmarkEdit'
-    : 'bookmarkAdded'
+
+  return editMode
+    ? 'bookmarkEdit'
+    : 'bookmarkCreateNew'
 }
 
 function displayBookmarkName (detail) {

app/renderer/components/bookmarks/addEditBookmarkHanger.js

@@ -3,6 +3,7 @@
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 const React = require('react')
+const Immutable = require('immutable')
 
 // Components
 const ReduxComponent = require('../reduxComponent')
@@ -60,10 +61,9 @@ class AddEditBookmarkHanger extends React.Component {
 
   componentDidMount () {
     // Automatically save if this is triggered by the url star
-    if (!this.props.isModal && !this.props.shouldShowLocation) {
-      this.onSave(false)
+    if (!this.props.isModal && !this.props.isFolder) {
+      this.addBookmark()
     }
-    this.bookmarkName.value = displayBookmarkName(this.props.currentDetail)
     this.setDefaultFocus()
   }
 
@@ -87,27 +87,27 @@ class AddEditBookmarkHanger extends React.Component {
   }
 
   onNameChange (e) {
-    let currentDetail = this.props.currentDetail
+    let bookmarkDetail = this.props.bookmarkDetail
     let name = e.target.value
     if (typeof name === 'string' && UrlUtil.isURL(name)) {
       const punycodeUrl = UrlUtil.getPunycodeUrl(name)
       if (punycodeUrl.replace(/\/$/, '') !== name) {
         name = punycodeUrl
       }
     }
-    if (currentDetail.get('title') === name && name) {
-      currentDetail = currentDetail.delete('customTitle')
+    if (bookmarkDetail.get('title') === name && name) {
+      bookmarkDetail = bookmarkDetail.delete('customTitle')
     } else {
       // '' is reserved for the default customTitle value of synced bookmarks,
       // so replace '' with 0 if the user is deleting the customTitle.
       // Note that non-string bookmark titles fail isBookmarkNameValid so they
       // are not saved.
-      currentDetail = currentDetail.set('customTitle', name || 0)
+      bookmarkDetail = bookmarkDetail.set('customTitle', name || 0)
     }
     if (this.bookmarkName.value !== name) {
       this.bookmarkName.value = name
     }
-    windowActions.setBookmarkDetail(currentDetail, this.props.originalDetail, this.props.destinationDetail, this.props.shouldShowLocation, !this.props.isModal)
+    windowActions.setBookmarkDetail(bookmarkDetail, this.props.originalDetail, this.props.destinationDetail, this.props.shouldShowLocation, !this.props.isModal)
   }
 
   onLocationChange (e) {
@@ -118,13 +118,13 @@ class AddEditBookmarkHanger extends React.Component {
         location = punycodeUrl
       }
     }
-    const currentDetail = this.props.currentDetail.set('location', location)
-    windowActions.setBookmarkDetail(currentDetail, this.props.originalDetail, this.props.destinationDetail, this.props.shouldShowLocation, !this.props.isModal)
+    const bookmarkDetail = this.props.bookmarkDetail.set('location', location)
+    windowActions.setBookmarkDetail(bookmarkDetail, this.props.originalDetail, this.props.destinationDetail, this.props.shouldShowLocation, !this.props.isModal)
   }
 
   onParentFolderChange (e) {
-    const currentDetail = this.props.currentDetail.set('parentFolderId', Number(e.target.value))
-    windowActions.setBookmarkDetail(currentDetail, this.props.originalDetail, this.props.destinationDetail, undefined, !this.props.isModal)
+    const bookmarkDetail = this.props.bookmarkDetail.set('parentFolderId', Number(e.target.value))
+    windowActions.setBookmarkDetail(bookmarkDetail, this.props.originalDetail, this.props.destinationDetail, undefined, !this.props.isModal)
   }
 
   showToolbarOnFirstBookmark () {
@@ -133,23 +133,30 @@ class AddEditBookmarkHanger extends React.Component {
     }
   }
 
-  onSave (closeDialog = true) {
-    // First check if the title of the currentDetail is set
+  addBookmark () {
     if (!this.props.isBookmarkNameValid) {
       return false
     }
+
     this.showToolbarOnFirstBookmark()
-    const tag = this.props.isFolder ? siteTags.BOOKMARK_FOLDER : siteTags.BOOKMARK
-    appActions.addSite(this.props.currentDetail, tag, this.props.originalDetail, this.props.destinationDetail)
+    appActions.addSite(this.props.bookmarkDetail, siteTags.BOOKMARK)
+  }
 
-    if (closeDialog) {
-      this.onClose()
+  onSave () {
+    // First check if the title of the bookmarkDetail is set
+    if (!this.props.isBookmarkNameValid) {
+      return false
     }
+
+    this.showToolbarOnFirstBookmark()
+    // const tag = this.props.isFolder ? siteTags.BOOKMARK_FOLDER : siteTags.BOOKMARK
+    // appActions.addSite(this.props.bookmarkDetail, tag, this.props.originalDetail, this.props.destinationDetail)
+    this.onClose()
   }
 
   onRemoveBookmark () {
     const tag = this.props.isFolder ? siteTags.BOOKMARK_FOLDER : siteTags.BOOKMARK
-    appActions.removeSite(this.props.currentDetail, tag)
+    appActions.removeSite(this.props.bookmarkDetail, tag)
     this.onClose()
   }
 
@@ -161,29 +168,26 @@ class AddEditBookmarkHanger extends React.Component {
   mergeProps (state, ownProps) {
     const currentWindow = state.get('currentWindow')
     const bookmarkDetail = currentWindow.get('bookmarkDetail')
-    const currentDetail = bookmarkDetail.get('currentDetail')
-    const originalDetail = bookmarkDetail.get('originalDetail')
+    const siteDetail = bookmarkDetail.get('siteDetail', Immutable.Map())
+    const editMode = bookmarkDetail.has('editKey')
 
     const props = {}
     // used in renderer
     props.isModal = ownProps.isModal
     props.withHomeButton = getSetting(settings.SHOW_HOME_BUTTON)
     // Fake a folderId property so that the bookmark is considered a bookmark folder.
     // This is ImmutableJS so it doesn't actually set a value, it just returns a new one.
-    props.isFolder = siteUtil.isFolder(currentDetail.set('folderId', 0))
-    props.shouldShowLocation = bookmarkDetail.get('shouldShowLocation')
-    props.heading = bookmarkHangerHeading(originalDetail, props.isFolder, props.shouldShowLocation)
-    props.location = currentDetail.get('location')
-    props.parentFolderId = currentDetail.get('parentFolderId')
-    props.hasOriginalDetail = !!originalDetail
-    props.displayBookmarkName = displayBookmarkName(currentDetail)
-    props.isBookmarkNameValid = isBookmarkNameValid(currentDetail, props.isFolder)
-    props.folders = siteUtil.getFolders(state.get('sites'), currentDetail.get('folderId')) // TODO (nejc) improve, primitives only
+    props.isFolder = siteUtil.isFolder(siteDetail.set('folderId', 0))
+    props.heading = bookmarkHangerHeading(editMode, props.isFolder)
+    props.location = siteDetail.get('location')
+    props.parentFolderId = siteDetail.get('parentFolderId')
+    props.hasOriginalDetail = editMode
+    props.displayBookmarkName = displayBookmarkName(siteDetail)
+    props.isBookmarkNameValid = isBookmarkNameValid(siteDetail, props.isFolder)
+    props.folders = siteUtil.getFolders(state.get('sites'), siteDetail.get('folderId')) // TODO (nejc) improve, primitives only
 
     // used in functions
-    props.currentDetail = currentDetail // TODO (nejc) improve, primitives only
-    props.originalDetail = originalDetail // TODO (nejc) improve, primitives only
-    props.destinationDetail = bookmarkDetail.get('destinationDetail') // TODO (nejc) improve, primitives only
+    props.bookmarkDetail = siteDetail // TODO (nejc) improve, primitives only
     props.hasBookmarks = state.get('sites').some(
       (site) => siteUtil.isBookmark(site) || siteUtil.isFolder(site)
     )
@@ -229,12 +233,13 @@ class AddEditBookmarkHanger extends React.Component {
                   spellCheck='false'
                   onKeyDown={this.onKeyDown}
                   onChange={this.onNameChange}
+                  value={this.props.displayBookmarkName}
                   ref={(bookmarkName) => { this.bookmarkName = bookmarkName }}
                 />
               </div>
             </section>
             {
-              !this.props.isFolder && this.props.shouldShowLocation
+              !this.props.isFolder
               ? <section className={css(styles.bookmarkHanger__marginRow)}>
                 <div className={css(
                   commonFormStyles.inputWrapper,

docs/appActions.md

@@ -171,7 +171,7 @@ Notifies that a tab has been closed
 
 
 
-### addSite(siteDetail, tag, originalSiteDetail, destinationIsParent, skipSync) 
+### addSite(siteDetail, tag, skipSync) 
 
 Adds a site to the site list
 
@@ -181,11 +181,6 @@ Adds a site to the site list
 
 **tag**: `string`, A tag to associate with the site. e.g. bookmarks.
 
-**originalSiteDetail**: `string`, If specified, the original site detail to edit / overwrite.
-
-**destinationIsParent**: `boolean`, Whether or not the destinationDetail should be considered the new parent.
-  The details of the old entries will be modified if this is set, otherwise only the tag will be added.
-
 **skipSync**: `boolean`, Set true if a site isn't eligible for Sync (e.g. if addSite was triggered by Sync)
 
 

docs/windowActions.md

@@ -376,6 +376,20 @@ If set, also indicates that add/edit is shown
 
 
 
+### addBookmark() 
+
+Used for displaying bookmark hanger
+when adding bookmark site or folder
+
+
+
+### editBookmark() 
+
+Used for displaying bookmark hanger
+when editing bookmark site or folder
+
+
+
 ### setContextMenuDetail(detail) 
 
 Dispatches a message to set context menu detail.

js/actions/appActions.js

@@ -237,18 +237,13 @@ const appActions = {
    * Adds a site to the site list
    * @param {Object} siteDetail - Properties of the site in question, can also be an array of siteDetail
    * @param {string} tag - A tag to associate with the site. e.g. bookmarks.
-   * @param {string} originalSiteDetail - If specified, the original site detail to edit / overwrite.
-   * @param {boolean} destinationIsParent - Whether or not the destinationDetail should be considered the new parent.
-   *   The details of the old entries will be modified if this is set, otherwise only the tag will be added.
    * @param {boolean} skipSync - Set true if a site isn't eligible for Sync (e.g. if addSite was triggered by Sync)
    */
-  addSite: function (siteDetail, tag, originalSiteDetail, destinationDetail, skipSync) {
+  addSite: function (siteDetail, tag, skipSync) {
     dispatch({
       actionType: appConstants.APP_ADD_SITE,
       siteDetail,
       tag,
-      originalSiteDetail,
-      destinationDetail,
       skipSync
     })
   },

js/actions/windowActions.js

@@ -470,6 +470,7 @@ const windowActions = {
    * @param {boolean} shouldShowLocation - Whether or not to show the URL input
    * @param {boolean} isBookmarkHanger - true if triggered from star icon in nav bar
    */
+  // TODO remove
   setBookmarkDetail: function (currentDetail, originalDetail, destinationDetail, shouldShowLocation, isBookmarkHanger) {
     dispatch({
       actionType: windowConstants.WINDOW_SET_BOOKMARK_DETAIL,
@@ -481,6 +482,30 @@ const windowActions = {
     })
   },
 
+  /**
+   * Used for displaying bookmark hanger
+   * when adding bookmark site or folder
+   */
+  addBookmark: function (siteDetail) {
+    dispatch({
+      actionType: windowConstants.WINDOW_ON_ADD_BOOKMARK,
+      siteDetail
+    })
+  },
+
+  /**
+   * Used for displaying bookmark hanger
+   * when editing bookmark site or folder
+   */
+  editBookmark: function (siteDetail, editKey, isHanger) {
+    dispatch({
+      actionType: windowConstants.WINDOW_ON_EDIT_BOOKMARK,
+      siteDetail,
+      editKey,
+      isHanger
+    })
+  },
+
   /**
    * Dispatches a message to set context menu detail.
    * If set, also indicates that the context menu is shown.

js/constants/windowConstants.js

@@ -98,7 +98,9 @@ const windowConstants = {
   WINDOW_SHOULD_OPEN_DEV_TOOLS: _,
   WINDOW_SET_ALL_AUDIO_MUTED: _,
   WINDOW_ON_GO_BACK_LONG: _,
-  WINDOW_ON_GO_FORWARD_LONG: _
+  WINDOW_ON_GO_FORWARD_LONG: _,
+  WINDOW_ON_ADD_BOOKMARK: _,
+  WINDOW_ON_EDIT_BOOKMARK: _
 }
 
 module.exports = mapValuesByKeys(windowConstants)

js/contextMenus.js

@@ -45,7 +45,6 @@ const isLinux = process.platform === 'linux'
 
 /**
  * Obtains an add bookmark menu item
- * @param {object} Detail of the bookmark to initialize with
  */
 const addBookmarkMenuItem = (label, siteDetail, closestDestinationDetail, isParent) => {
   return {
@@ -58,7 +57,9 @@ const addBookmarkMenuItem = (label, siteDetail, closestDestinationDetail, isPare
       if (siteDetail.constructor !== Immutable.Map) {
         siteDetail = Immutable.fromJS(siteDetail)
       }
+
       siteDetail = siteDetail.set('location', urlUtil.getLocationIfPDF(siteDetail.get('location')))
+      windowActions.addBookmark(siteDetail)
       windowActions.setBookmarkDetail(siteDetail, siteDetail, closestDestinationDetail, true)
     }
   }